Repository: incubator-freemarker Updated Branches: refs/heads/3 3d0fdc8c7 -> a612ce0db
Clarified buffering concerns regarding TemplateLoaderResult.getReader and getInputStream. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/a612ce0d Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/a612ce0d Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/a612ce0d Branch: refs/heads/3 Commit: a612ce0db53f8551d646bacf83f179d7b4acecb8 Parents: 3d0fdc8 Author: ddekany <[email protected]> Authored: Sat Feb 11 17:20:15 2017 +0100 Committer: ddekany <[email protected]> Committed: Sat Feb 11 17:20:15 2017 +0100 ---------------------------------------------------------------------- src/main/java/freemarker/cache/TemplateCache.java | 8 +++++--- .../freemarker/cache/TemplateLoadingResult.java | 16 ++++++++++++---- src/main/java/freemarker/template/Template.java | 10 ++++++++-- 3 files changed, 25 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/cache/TemplateCache.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/cache/TemplateCache.java b/src/main/java/freemarker/cache/TemplateCache.java index d545986..9439b78 100644 --- a/src/main/java/freemarker/cache/TemplateCache.java +++ b/src/main/java/freemarker/cache/TemplateCache.java @@ -628,12 +628,14 @@ public class TemplateCache { if (!inputStream.markSupported()) { inputStream = new BufferedInputStream(inputStream); } - inputStream.mark(Integer.MAX_VALUE); // [FM3] Mark should be released after the 1st FTL tag - templateSpecifiedEncodingHandler = new MarkReleaserTemplateSpecifiedEncodingHandler( - inputStream); + inputStream.mark(Integer.MAX_VALUE); // Mark is released after the 1st FTL tag + templateSpecifiedEncodingHandler = new MarkReleaserTemplateSpecifiedEncodingHandler(inputStream); } else { templateSpecifiedEncodingHandler = null; } + // Regarding buffering worries: On the Reader side we should only read in chunks (like through a + // BufferedReader), so there shouldn't be a problem if the InputStream is not buffered. (Also, at least + // on Oracle JDK and OpenJDK 7 the InputStreamReader itself has an internal ~8K buffer.) reader = new InputStreamReader(inputStream, initialEncoding); } else { throw new IllegalStateException("For a(n) " + templateLoaderResult.getClass().getName() http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/cache/TemplateLoadingResult.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/cache/TemplateLoadingResult.java b/src/main/java/freemarker/cache/TemplateLoadingResult.java index 87dcac9..3bc7f8d 100644 --- a/src/main/java/freemarker/cache/TemplateLoadingResult.java +++ b/src/main/java/freemarker/cache/TemplateLoadingResult.java @@ -18,6 +18,8 @@ */ package freemarker.cache; +import java.io.BufferedInputStream; +import java.io.BufferedReader; import java.io.InputStream; import java.io.Reader; import java.io.Serializable; @@ -56,7 +58,8 @@ public final class TemplateLoadingResult { * See {@link #getVersion()} for the meaning of this. Can be {@code null}, but use that only if the * backing storage mechanism doesn't know this information. * @param reader - * Gives the content of the template + * Gives the content of the template. It will be read in few thousand character chunks by FreeMarker, so + * generally it need not be a {@link BufferedReader}. * @param templateConfiguration * Usually {@code null}, as usually the backing storage mechanism doesn't store such information; * see {@link #getTemplateConfiguration()}. @@ -85,7 +88,8 @@ public final class TemplateLoadingResult { * See {@link #getVersion()} for the meaning of this. Can be {@code null}, but use that only if the * backing storage mechanism doesn't know this information. * @param inputStream - * Gives the content of the template + * Gives the content of the template. It will be read in few thousand byte chunks by FreeMarker, so + * generally it need not be a {@link BufferedInputStream}. * @param templateConfiguration * Usually {@code null}, as usually the backing storage mechanism doesn't store such information; see * {@link #getTemplateConfiguration()}. The most probable application is supplying the charset (encoding) @@ -124,8 +128,8 @@ public final class TemplateLoadingResult { * The return value is always the same instance, no mater when and how many times this method is called. * * <p> - * The return {@code InputStream} should use buffering if that's useful considering the backing storage mechanism. - * TODO Is it really needed? + * The returned {@code InputStream} will be read in few kilobyte chunks by FreeMarker, so generally it need not + * be a {@link BufferedInputStream}. * * @return {@code null} or a {@code InputStream} to read the template content; see method description for more. */ @@ -172,6 +176,10 @@ public final class TemplateLoadingResult { * responsibility of the caller (which is {@link TemplateCache} usually) to {@code close()} the {@link Reader}. The * return value is always the same instance, no mater when and how many times this method is called. * + * <p> + * The returned {@code Reader} will be read in few thousand character chunks by FreeMarker, so generally it need not + * be a {@link BufferedReader}. + * * @return {@code null} or a {@code Reader} to read the template content; see method description for more. */ public Reader getReader() { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/template/Template.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/Template.java b/src/main/java/freemarker/template/Template.java index ac9bde2..42b7ee9 100644 --- a/src/main/java/freemarker/template/Template.java +++ b/src/main/java/freemarker/template/Template.java @@ -80,6 +80,8 @@ import freemarker.template.utility.NullArgumentException; public class Template extends Configurable { public static final String DEFAULT_NAMESPACE_PREFIX = "D"; public static final String NO_NS_PREFIX = "N"; + + private static final int READER_BUFFER_SIZE = 8192; /** This is only non-null during parsing. It's used internally to make some information available through the * Template API-s earlier than the parsing was finished. */ @@ -168,7 +170,8 @@ public class Template extends Configurable { * @param reader * The character stream to read from. The {@link Reader} is <em>not</em> closed by this method (unlike * in FreeMarker 2.x.x), so be sure that it's closed somewhere. (Except of course, readers like - * {@link StringReader} need not be closed.) + * {@link StringReader} need not be closed.) The {@link Reader} need not be buffered, because this + * method ensures that it will be read in few kilobyte chunks, not byte by byte. * @param cfg * The Configuration object that this Template is associated with. If this is {@code null}, the "default" * {@link Configuration} object is used, which is highly discouraged, because it can easily lead to @@ -254,9 +257,12 @@ public class Template extends Configurable { try { ParserConfiguration actualParserConfiguration = getParserConfiguration(); + // Ensure that the parameter Reader is only read in bigger chunks, as we don't know if the it's buffered. + // In particular, inside the FreeMarker code, we assume that the stream stages need not be buffered. if (!(reader instanceof BufferedReader) && !(reader instanceof StringReader)) { - reader = new BufferedReader(reader, 0x1000); + reader = new BufferedReader(reader, READER_BUFFER_SIZE); } + ltbReader = new LineTableBuilder(reader, actualParserConfiguration); reader = ltbReader;
