Got rid of over-engineering in the handling of WrongEncodingException
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/0a8f9d1d Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/0a8f9d1d Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/0a8f9d1d Branch: refs/heads/3 Commit: 0a8f9d1d21217abe45799e1b4d4106ee6d82f55f Parents: 7d784b2 Author: ddekany <[email protected]> Authored: Thu Feb 23 20:30:05 2017 +0100 Committer: ddekany <[email protected]> Committed: Thu Feb 23 20:30:05 2017 +0100 ---------------------------------------------------------------------- .../freemarker/core/ASTExpStringLiteral.java | 3 +- .../freemarker/core/BuiltInsForStringsMisc.java | 2 +- ...eleaserTemplateSpecifiedEncodingHandler.java | 56 ------------------ .../org/apache/freemarker/core/Template.java | 42 ++++++-------- .../core/TemplateSpecifiedEncodingHandler.java | 60 -------------------- .../org/apache/freemarker/core/_CoreAPI.java | 5 +- .../impl/DefaultTemplateResolver.java | 14 ++--- src/main/javacc/FTL.jj | 31 +++++----- 8 files changed, 47 insertions(+), 166 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java b/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java index b7c8960..25fa037 100644 --- a/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java +++ b/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java @@ -64,8 +64,7 @@ final class ASTExpStringLiteral extends ASTExpression implements TemplateScalarM FMParserTokenManager tkMan = new FMParserTokenManager( simpleCharacterStream); - FMParser parser = new FMParser(parentTemplate, false, tkMan, pcfg, - TemplateSpecifiedEncodingHandler.DEFAULT); + FMParser parser = new FMParser(parentTemplate, false, tkMan, pcfg, null); // We continue from the parent parser's current state: parser.setupStringLiteralMode(parentTkMan, outputFormat); try { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/BuiltInsForStringsMisc.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/BuiltInsForStringsMisc.java b/src/main/java/org/apache/freemarker/core/BuiltInsForStringsMisc.java index 543143a..2f885a5 100644 --- a/src/main/java/org/apache/freemarker/core/BuiltInsForStringsMisc.java +++ b/src/main/java/org/apache/freemarker/core/BuiltInsForStringsMisc.java @@ -94,7 +94,7 @@ class BuiltInsForStringsMisc { } FMParser parser = new FMParser( - parentTemplate, false, tkMan, pCfg, TemplateSpecifiedEncodingHandler.DEFAULT); + parentTemplate, false, tkMan, pCfg, null); exp = parser.ASTExpression(); } catch (TokenMgrError e) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/MarkReleaserTemplateSpecifiedEncodingHandler.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/MarkReleaserTemplateSpecifiedEncodingHandler.java b/src/main/java/org/apache/freemarker/core/MarkReleaserTemplateSpecifiedEncodingHandler.java deleted file mode 100644 index 555bbf3..0000000 --- a/src/main/java/org/apache/freemarker/core/MarkReleaserTemplateSpecifiedEncodingHandler.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.freemarker.core; - -import java.io.InputStream; - -import org.apache.freemarker.core.Template.WrongEncodingException; - -/** - * A {@link TemplateSpecifiedEncodingHandler} that discards the mark of the specified {@link InputStream} when - * the template parsing gets to a point where it's known that we can't receive a template specified encoding anymore. - * This allows freeing up the mark buffer early during parsing. - * - * @since 2.3.16 - */ -public class MarkReleaserTemplateSpecifiedEncodingHandler implements TemplateSpecifiedEncodingHandler { - - private final InputStream markedInputStream; - - /** - * @param markedInputStream Input stream with marked template content start position - */ - public MarkReleaserTemplateSpecifiedEncodingHandler(InputStream markedInputStream) { - this.markedInputStream = markedInputStream; - } - - @Override - public void handle(String templateSpecificEncoding, String constructorSpecifiedEncoding) - throws WrongEncodingException { - TemplateSpecifiedEncodingHandler.DEFAULT.handle(templateSpecificEncoding, constructorSpecifiedEncoding); - - // There was no WrongEncodingException exception, release mark buffer: - markedInputStream.mark(0); // - } - - public InputStream getMarkedInputStream() { - return markedInputStream; - } - -} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/Template.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/Template.java b/src/main/java/org/apache/freemarker/core/Template.java index 48996ac..77c92c4 100644 --- a/src/main/java/org/apache/freemarker/core/Template.java +++ b/src/main/java/org/apache/freemarker/core/Template.java @@ -19,14 +19,7 @@ package org.apache.freemarker.core; -import java.io.BufferedReader; -import java.io.FilterReader; -import java.io.IOException; -import java.io.PrintStream; -import java.io.Reader; -import java.io.StringReader; -import java.io.StringWriter; -import java.io.Writer; +import java.io.*; import java.lang.reflect.UndeclaredThrowableException; import java.util.ArrayList; import java.util.Collections; @@ -218,26 +211,27 @@ public class Template extends Configurable { String name, String sourceName, Reader reader, Configuration cfg, ParserConfiguration customParserConfiguration, String encoding) throws IOException { - this(name, sourceName, reader, cfg, customParserConfiguration, encoding, - TemplateSpecifiedEncodingHandler.DEFAULT); + this(name, sourceName, reader, cfg, customParserConfiguration, encoding, null); } - - /** - * Same as {@link #Template(String, String, Reader, Configuration, ParserConfiguration, String)}, but allows - * specifying a non-default (non-{@link TemplateSpecifiedEncodingHandler#DEFAULT}) behavior regarding encoding - * specified in the template content. - * - * @param templateSpecifiedEncodingHandler Not {@code null}. - * - * @since 2.3.26 - */ + + /** + * Same as {@link #Template(String, String, Reader, Configuration, ParserConfiguration, String)}, but allows + * specifying a non-default (non-{@link TemplateSpecifiedEncodingHandler#DEFAULT}) behavior regarding encoding + * specified in the template content. + * + * @param streamToUnmarkWhenEncEstabd + * If not {@code null}, when during the parsing we reach a point where we know that no {@link + * WrongEncodingException} will be thrown, {@link InputStream#mark(int) mark(0)} will be called on this. + * This is meant to be used when the reader parameter is a {@link InputStreamReader}, and this parameter is + * the underlying {@link InputStream}, and you have a mark at the beginning of the {@link InputStream} so + * that you can retry if a {@link WrongEncodingException} is thrown without extra I/O. As keeping that + * mark consumes some resources, so you may want to release it as soon as possible. + */ public Template( String name, String sourceName, Reader reader, Configuration cfg, ParserConfiguration customParserConfiguration, - String encoding, TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler) throws IOException { + String encoding, InputStream streamToUnmarkWhenEncEstabd) throws IOException { this(name, sourceName, cfg, customParserConfiguration); - - _NullArgumentException.check("templateSpecifiedEncodingHandler", templateSpecifiedEncodingHandler); setEncoding(encoding); LineTableBuilder ltbReader; @@ -255,7 +249,7 @@ public class Template extends Configurable { try { parser = _CoreAPI.newFMParser( - this, reader, actualParserConfiguration, templateSpecifiedEncodingHandler); + this, reader, actualParserConfiguration, streamToUnmarkWhenEncEstabd); try { rootElement = parser.Root(); } catch (IndexOutOfBoundsException exc) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/TemplateSpecifiedEncodingHandler.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/TemplateSpecifiedEncodingHandler.java b/src/main/java/org/apache/freemarker/core/TemplateSpecifiedEncodingHandler.java deleted file mode 100644 index 35a22f5..0000000 --- a/src/main/java/org/apache/freemarker/core/TemplateSpecifiedEncodingHandler.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.freemarker.core; - -import org.apache.freemarker.core.Template.WrongEncodingException; - -/** - * Specifies the behavior when the template specifies its own encoding (via {@code <#ftl encoding=...>}) in the template - * content itself, and also when it doesn't do that. - */ -public interface TemplateSpecifiedEncodingHandler { - - TemplateSpecifiedEncodingHandler DEFAULT = new TemplateSpecifiedEncodingHandler() { - - @Override - public void handle(String templateSpecificEncoding, String constructorSpecifiedEncoding) - throws WrongEncodingException { - if (constructorSpecifiedEncoding != null && templateSpecificEncoding != null - && !constructorSpecifiedEncoding.equalsIgnoreCase(templateSpecificEncoding)) { - throw new Template.WrongEncodingException(templateSpecificEncoding, constructorSpecifiedEncoding); - } - } - - }; - - /** - * Called once during template parsing, either when the {@code #ftl} directive is processed, or near the beginning - * of the template processing when there's no {@code #ftl} directive in the template. - * - * @param templateSpecificEncoding - * The encoding specified via {@code <#ftl encoding=...>}, or {@code null} if that was missing (either - * the {@code encoding} parameter or the whole {@code #ftl} directive). - * @param constructorSpecifiedEncoding - * The encoding specified to the {@link Template} constructor; also the value of - * {@link Template#getEncoding()}. If there was an encoding used for decoding the template file, it - * should be that, or if there was no encoding, it should be {@code null}. - * - * @throws WrongEncodingException - * If the template "file" has to be re-read and the {@link Template} re-created with the encoding - * specified in the constructor of {@link WrongEncodingException}. - */ - void handle(String templateSpecificEncoding, String constructorSpecifiedEncoding) throws WrongEncodingException; - -} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/_CoreAPI.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/_CoreAPI.java b/src/main/java/org/apache/freemarker/core/_CoreAPI.java index bcff837..5f51bd4 100644 --- a/src/main/java/org/apache/freemarker/core/_CoreAPI.java +++ b/src/main/java/org/apache/freemarker/core/_CoreAPI.java @@ -19,6 +19,7 @@ package org.apache.freemarker.core; +import java.io.InputStream; import java.io.Reader; import java.io.Writer; import java.util.Collection; @@ -212,8 +213,8 @@ public class _CoreAPI { } public static FMParser newFMParser(Template template, Reader reader, ParserConfiguration pCfg, - TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler) { - return new FMParser(template, reader, pCfg, templateSpecifiedEncodingHandler); + InputStream streamToUnmarkWhenEncEstabd) { + return new FMParser(template, reader, pCfg, streamToUnmarkWhenEncEstabd); } public static boolean isMacroOrFunction(TemplateModel m) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java b/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java index bff9306..4678988 100644 --- a/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java +++ b/src/main/java/org/apache/freemarker/core/templateresolver/impl/DefaultTemplateResolver.java @@ -32,11 +32,9 @@ import java.util.Locale; import java.util.StringTokenizer; import org.apache.freemarker.core.Configuration; -import org.apache.freemarker.core.MarkReleaserTemplateSpecifiedEncodingHandler; import org.apache.freemarker.core.Template; import org.apache.freemarker.core.TemplateConfiguration; import org.apache.freemarker.core.TemplateNotFoundException; -import org.apache.freemarker.core.TemplateSpecifiedEncodingHandler; import org.apache.freemarker.core._CoreLogs; import org.apache.freemarker.core.templateresolver.CacheStorage; import org.apache.freemarker.core.templateresolver.GetTemplateResult; @@ -566,14 +564,14 @@ public class DefaultTemplateResolver extends TemplateResolver { { Reader reader = templateLoaderResult.getReader(); InputStream inputStream = templateLoaderResult.getInputStream(); - TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler; + InputStream markedInputStream; if (reader != null) { if (inputStream != null) { throw new IllegalStateException("For a(n) " + templateLoaderResult.getClass().getName() + ", both getReader() and getInputStream() has returned non-null."); } initialEncoding = null; // No charset decoding has happened - templateSpecifiedEncodingHandler = TemplateSpecifiedEncodingHandler.DEFAULT; + markedInputStream = null; } else if (inputStream != null) { if (parseAsFTL) { // We need mark support, to restart if the charset suggested by <#ftl encoding=...> differs @@ -582,9 +580,9 @@ public class DefaultTemplateResolver extends TemplateResolver { inputStream = new BufferedInputStream(inputStream); } inputStream.mark(Integer.MAX_VALUE); // Mark is released after the 1st FTL tag - templateSpecifiedEncodingHandler = new MarkReleaserTemplateSpecifiedEncodingHandler(inputStream); + markedInputStream = inputStream; } else { - templateSpecifiedEncodingHandler = null; + markedInputStream = 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 @@ -599,7 +597,7 @@ public class DefaultTemplateResolver extends TemplateResolver { if (parseAsFTL) { try { template = new Template(name, sourceName, reader, config, tc, - initialEncoding, templateSpecifiedEncodingHandler); + initialEncoding, markedInputStream); } catch (Template.WrongEncodingException wee) { final String templateSpecifiedEncoding = wee.getTemplateSpecifiedEncoding(); @@ -615,7 +613,7 @@ public class DefaultTemplateResolver extends TemplateResolver { } template = new Template(name, sourceName, reader, config, tc, - templateSpecifiedEncoding, templateSpecifiedEncodingHandler); + templateSpecifiedEncoding, markedInputStream); } } else { // Read the contents into a StringWriter, then construct a single-text-block template from it. http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0a8f9d1d/src/main/javacc/FTL.jj ---------------------------------------------------------------------- diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj index 5dd65d9..5ad8488 100644 --- a/src/main/javacc/FTL.jj +++ b/src/main/javacc/FTL.jj @@ -78,8 +78,7 @@ public class FMParser { private int autoEscapingPolicy; private boolean autoEscaping; private ParserConfiguration pCfg; - private TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler; - private boolean templateSpecifiedEncodingHandlerInvoked; + private InputStream streamToUnmarkWhenEncEstabd; /** Keeps track of #list and #foreach nesting. */ private List/*<ParserIteratorBlockContext>*/ iteratorBlockContexts; @@ -98,9 +97,8 @@ public class FMParser { private LinkedList escapes = new LinkedList(); private int mixedContentNesting; // for stripText - FMParser(Template template, Reader reader, ParserConfiguration pCfg, - TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler) { - this(template, true, readerToTokenManager(reader, pCfg), pCfg, templateSpecifiedEncodingHandler); + FMParser(Template template, Reader reader, ParserConfiguration pCfg, InputStream streamToUnmarkWhenEncEstabd) { + this(template, true, readerToTokenManager(reader, pCfg), pCfg, streamToUnmarkWhenEncEstabd); } private static FMParserTokenManager readerToTokenManager(Reader reader, ParserConfiguration pCfg) { @@ -110,14 +108,13 @@ public class FMParser { } FMParser(Template template, boolean newTemplate, FMParserTokenManager tkMan, ParserConfiguration pCfg, - TemplateSpecifiedEncodingHandler templateSpecifiedEncodingHandler) { + InputStream streamToUnmarkWhenEncEstabd) { this(tkMan); _NullArgumentException.check(pCfg); this.pCfg = pCfg; - _NullArgumentException.check(templateSpecifiedEncodingHandler); - this.templateSpecifiedEncodingHandler = templateSpecifiedEncodingHandler; + this.streamToUnmarkWhenEncEstabd = streamToUnmarkWhenEncEstabd; _NullArgumentException.check(template); this.template = template; @@ -4041,8 +4038,15 @@ void HeaderElement() : if (vs == null) { throw new ParseException("Expected a string constant for \"" + ks + "\".", exp); } - templateSpecifiedEncodingHandlerInvoked = true; - templateSpecifiedEncodingHandler.handle(vs, template.getEncoding()); + if (template.getEncoding() != null && vs != null + && !template.getEncoding().equalsIgnoreCase(vs)) { + throw new Template.WrongEncodingException(vs, template.getEncoding()); + } + // There will be no WrongEncodingException exception, release mark buffer: + if (streamToUnmarkWhenEncEstabd != null) { + streamToUnmarkWhenEncEstabd.mark(0); + streamToUnmarkWhenEncEstabd = null; + } } else if (ks.equalsIgnoreCase("STRIP_WHITESPACE") || ks.equals("stripWhitespace")) { this.stripWhitespace = getBoolean(exp, true); } else if (ks.equalsIgnoreCase("STRIP_TEXT") || ks.equals("stripText")) { @@ -4232,9 +4236,10 @@ _ASTElement Root() : HeaderElement() ] { - if (!templateSpecifiedEncodingHandlerInvoked) { - templateSpecifiedEncodingHandlerInvoked = true; - templateSpecifiedEncodingHandler.handle(null, template.getEncoding()); + // There will be no WrongEncodingException exception, release mark buffer: + if (streamToUnmarkWhenEncEstabd != null) { + streamToUnmarkWhenEncEstabd.mark(0); + streamToUnmarkWhenEncEstabd = null; } }
