Repository: incubator-freemarker Updated Branches: refs/heads/2.3-gae f64aa1f0d -> 43fd9737a
FreemarkerServlet ResponseCharacterEncoding init-param now works properly with the legacy content_type template attribute (usually specified in the #ftl tag) Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/43fd9737 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/43fd9737 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/43fd9737 Branch: refs/heads/2.3-gae Commit: 43fd9737a8acdc33c1d5273439e5d6dcb041accc Parents: f64aa1f Author: ddekany <[email protected]> Authored: Sat Nov 21 12:41:22 2015 +0100 Committer: ddekany <[email protected]> Committed: Sat Nov 21 12:41:22 2015 +0100 ---------------------------------------------------------------------- .../ext/servlet/FreemarkerServlet.java | 104 +++++++++++------ .../ext/servlet/FreemarkerServletTest.java | 114 ++++++++++++------- 2 files changed, 142 insertions(+), 76 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/43fd9737/src/main/java/freemarker/ext/servlet/FreemarkerServlet.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/ext/servlet/FreemarkerServlet.java b/src/main/java/freemarker/ext/servlet/FreemarkerServlet.java index 695ac8d..ae48af6 100644 --- a/src/main/java/freemarker/ext/servlet/FreemarkerServlet.java +++ b/src/main/java/freemarker/ext/servlet/FreemarkerServlet.java @@ -407,7 +407,7 @@ public class FreemarkerServlet extends HttpServlet { private static final String DEPR_INITPARAM_TEMPLATE_EXCEPTION_HANDLER_IGNORE = "ignore"; private static final String DEPR_INITPARAM_DEBUG = "debug"; - static final String DEFAULT_CONTENT_TYPE = "text/html"; + private static final ContentType DEFAULT_CONTENT_TYPE = new ContentType("text/html"); public static final String INIT_PARAM_VALUE_NEVER = "never"; public static final String INIT_PARAM_VALUE_ALWAYS = "always"; @@ -511,12 +511,12 @@ public class FreemarkerServlet extends HttpServlet { private Configuration config; @SuppressFBWarnings(value="SE_BAD_FIELD", justification="Not investing into making this Servlet serializable") private ObjectWrapper wrapper; - private String contentType; + private ContentType contentType; private OverrideResponseContentType overrideResponseContentType = initParamValueToEnum( getDefaultOverrideResponseContentType(), OverrideResponseContentType.values()); private ResponseCharacterEncoding responseCharacterEncoding = ResponseCharacterEncoding.LEGACY; + @SuppressFBWarnings(value="SE_BAD_FIELD", justification="Not investing into making this Servlet serializable") private Charset forcedResponseCharacterEncoding; - private boolean contentTypeContainsCharset; private OverrideResponseLocale overrideResponseLocale = OverrideResponseLocale.ALWAYS; private List/*<MetaInfTldSource>*/ metaInfTldSources; private List/*<String>*/ classpathTlds; @@ -661,7 +661,7 @@ public class FreemarkerServlet extends HttpServlet { } else if (name.equals(INIT_PARAM_DEBUG)) { debug = StringUtil.getYesNo(value); } else if (name.equals(INIT_PARAM_CONTENT_TYPE)) { - contentType = value; + contentType = new ContentType(value); } else if (name.equals(INIT_PARAM_OVERRIDE_RESPONSE_CONTENT_TYPE)) { overrideResponseContentType = initParamValueToEnum(value, OverrideResponseContentType.values()); } else if (name.equals(INIT_PARAM_RESPONSE_CHARACTER_ENCODING)) { @@ -693,32 +693,14 @@ public class FreemarkerServlet extends HttpServlet { } } // while initpnames - contentTypeContainsCharset = contentTypeContainsCharset(contentType); - if (contentTypeContainsCharset && responseCharacterEncoding != ResponseCharacterEncoding.LEGACY) { - throw new InitParamValueException(INIT_PARAM_CONTENT_TYPE, contentType, + if (contentType.containsCharset && responseCharacterEncoding != ResponseCharacterEncoding.LEGACY) { + throw new InitParamValueException(INIT_PARAM_CONTENT_TYPE, contentType.httpHeaderValue, new IllegalStateException("You can't specify the charset in the content type, because the \"" + INIT_PARAM_RESPONSE_CHARACTER_ENCODING + "\" init-param isn't set to " + "\"" + INIT_PARAM_VALUE_LEGACY + "\".")); - } + } } - private boolean contentTypeContainsCharset(String contentType) { - int charsetIdx = contentType.toLowerCase().indexOf("charset="); - if (charsetIdx != -1) { - char c = 0; - charsetIdx--; - while (charsetIdx >= 0) { - c = contentType.charAt(charsetIdx); - if (!Character.isWhitespace(c)) break; - charsetIdx--; - } - if (charsetIdx == -1 || c == ';') { - return true; - } - } - return false; - } - private List/*<MetaInfTldSource>*/ parseAsMetaInfTldLocations(String value) throws ParseException { List/*<MetaInfTldSource>*/ metaInfTldSources = null; @@ -837,16 +819,24 @@ public class FreemarkerServlet extends HttpServlet { "Unexpected error when loading template " + StringUtil.jQuoteNoXSS(templatePath) + ".", e); } + boolean tempSpecContentTypeContainsCharset = false; if (response.getContentType() == null || overrideResponseContentType != OverrideResponseContentType.NEVER) { - String templateSpecificContentType = getTemplateSpecificContentType(template); + ContentType templateSpecificContentType = getTemplateSpecificContentType(template); if (templateSpecificContentType != null) { - response.setContentType(templateSpecificContentType); + // With ResponseCharacterEncoding.LEGACY we should append the charset, but we don't do that for b. c. + response.setContentType( + responseCharacterEncoding != ResponseCharacterEncoding.DO_NOT_SET + ? templateSpecificContentType.httpHeaderValue + : templateSpecificContentType.getMimeType()); + tempSpecContentTypeContainsCharset = templateSpecificContentType.containsCharset; } else if (response.getContentType() == null || overrideResponseContentType == OverrideResponseContentType.ALWAYS) { - if (!contentTypeContainsCharset && responseCharacterEncoding == ResponseCharacterEncoding.LEGACY) { - response.setContentType(contentType + "; charset=" + getTemplateSpecificOutputEncoding(template)); + if (responseCharacterEncoding == ResponseCharacterEncoding.LEGACY && !contentType.containsCharset) { + // In legacy mode we don't call response.setCharacterEncoding, so the charset must be set here: + response.setContentType( + contentType.httpHeaderValue + "; charset=" + getTemplateSpecificOutputEncoding(template)); } else { - response.setContentType(contentType); + response.setContentType(contentType.httpHeaderValue); } } } @@ -855,7 +845,9 @@ public class FreemarkerServlet extends HttpServlet { && responseCharacterEncoding != ResponseCharacterEncoding.DO_NOT_SET) { // Using the Servlet 2.4 way of setting character encoding. if (responseCharacterEncoding != ResponseCharacterEncoding.FORCE_CHARSET) { - response.setCharacterEncoding(getTemplateSpecificOutputEncoding(template)); + if (!tempSpecContentTypeContainsCharset) { + response.setCharacterEncoding(getTemplateSpecificOutputEncoding(template)); + } } else { response.setCharacterEncoding(forcedResponseCharacterEncoding.name()); } @@ -920,21 +912,21 @@ public class FreemarkerServlet extends HttpServlet { return outputEncoding != null ? outputEncoding : template.getEncoding(); } - private String getTemplateSpecificContentType(final Template template) { + private ContentType getTemplateSpecificContentType(final Template template) { Object contentTypeAttr = template.getCustomAttribute("content_type"); if (contentTypeAttr != null) { // Converted with toString() for backward compatibility. - // Don't add charset for backward compatibility. - return contentTypeAttr.toString(); + return new ContentType(contentTypeAttr.toString()); } String outputFormatMimeType = template.getOutputFormat().getMimeType(); if (outputFormatMimeType != null) { if (responseCharacterEncoding == ResponseCharacterEncoding.LEGACY) { // In legacy mode we won't call serlvetResponse.setCharacterEncoding(...), so: - outputFormatMimeType += "; charset=" + getTemplateSpecificOutputEncoding(template); + return new ContentType(outputFormatMimeType + "; charset=" + getTemplateSpecificOutputEncoding(template), true); + } else { + return new ContentType(outputFormatMimeType, false); } - return outputFormatMimeType; } return null; @@ -1554,6 +1546,46 @@ public class FreemarkerServlet extends HttpServlet { } + private static class ContentType { + private final String httpHeaderValue; + private final boolean containsCharset; + + public ContentType(String httpHeaderValue) { + this(httpHeaderValue, contentTypeContainsCharset(httpHeaderValue)); + } + + public ContentType(String httpHeaderValue, boolean containsCharset) { + this.httpHeaderValue = httpHeaderValue; + this.containsCharset = containsCharset; + } + + private static boolean contentTypeContainsCharset(String contentType) { + int charsetIdx = contentType.toLowerCase().indexOf("charset="); + if (charsetIdx != -1) { + char c = 0; + charsetIdx--; + while (charsetIdx >= 0) { + c = contentType.charAt(charsetIdx); + if (!Character.isWhitespace(c)) break; + charsetIdx--; + } + if (charsetIdx == -1 || c == ';') { + return true; + } + } + return false; + } + + /** + * Extracts the MIME type without the charset specifier or other such extras. + */ + private String getMimeType() { + int scIdx = httpHeaderValue.indexOf(';'); + return (scIdx == -1 ? httpHeaderValue : httpHeaderValue.substring(0, scIdx)).trim(); + } + + } + private <T extends InitParamValueEnum> T initParamValueToEnum(String initParamValue, T[] enumValues) { for (T enumValue : enumValues) { String enumInitParamValue = enumValue.getInitParamValue(); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/43fd9737/src/test/java/freemarker/ext/servlet/FreemarkerServletTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/freemarker/ext/servlet/FreemarkerServletTest.java b/src/test/java/freemarker/ext/servlet/FreemarkerServletTest.java index b95cda5..f008280 100644 --- a/src/test/java/freemarker/ext/servlet/FreemarkerServletTest.java +++ b/src/test/java/freemarker/ext/servlet/FreemarkerServletTest.java @@ -53,6 +53,7 @@ public class FreemarkerServletTest { private static final String OUTPUT_FORMAT_HEADER_FTL = "outputFormatHeader.ftl"; private static final String CONTENT_TYPE_ATTR_FTL = "contentTypeAttr.ftl"; + private static final String CONTENT_TYPE_ATTR_WITH_CHARSET_FTL = "contentTypeAttrWithCharset.ftl"; private static final String FOO_FTL = "foo.ftl"; private static final String FOO_SRC_UTF8_FTL = "foo-src-utf8.ftl"; private static final String FOO_OUT_UTF8_FTL = "foo-out-utf8.ftl"; @@ -61,6 +62,7 @@ public class FreemarkerServletTest { private static final String CFG_DEFAULT_ENCODING = "US-ASCII"; /** According to the Servlet Specification */ private static final String SERVLET_RESPONSE_DEFAULT_CHARSET = "ISO-8859-1"; + private static final String DEFAULT_CONTENT_TYPE = "text/html"; private MockServletContext servletContext; @@ -182,42 +184,55 @@ public class FreemarkerServletTest { @Test public void testResponseOutputCharsetInitParam() throws Exception { - // Legacy mode is not aware of the outputEncoding, thus it doesn't set it: - assertOutputEncodingEquals( - CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding - null, // <- expected env.outputEncoding - null, // <- init-param - FOO_FTL); - assertOutputEncodingEquals( - CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding - null, // <- expected env.outputEncoding - FreemarkerServlet.INIT_PARAM_VALUE_LEGACY, // <- init-param - FOO_FTL); - // Legacy mode follows the source encoding of the template: - assertOutputEncodingEquals( - "UTF-8", // <- expected response.characterEncoding - null, // <- expected env.outputEncoding - null, // <- init-param - FOO_SRC_UTF8_FTL); - // Legacy mode doesn't deal with outputEncoding, but it's inherited by the Environment from the Template: - assertOutputEncodingEquals( - CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding - "UTF-8", // <- expected env.outputEncoding - null, // <- init-param - FOO_OUT_UTF8_FTL); - // Charset in content type is the strongest: - assertOutputEncodingEquals( - "ISO-8859-2", // <- expected response.characterEncoding - null, // <- expected env.outputEncoding - null, // <- init-param - "text/html; charset=ISO-8859-2", // ContentType init-param - FOO_FTL); - assertOutputEncodingEquals( - "ISO-8859-2", // <- expected response.characterEncoding - null, // <- expected env.outputEncoding - null, // <- init-param - "text/html; charset=ISO-8859-2", // ContentType init-param - FOO_SRC_UTF8_FTL); + for (String initParamValue : new String[] { null, FreemarkerServlet.INIT_PARAM_VALUE_LEGACY }) { + // Legacy mode is not aware of the outputEncoding, thus it doesn't set it: + assertOutputEncodingEquals( + CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + FOO_FTL); + assertOutputEncodingEquals( + CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + FOO_FTL); + // Legacy mode follows the source encoding of the template: + assertOutputEncodingEquals( + "UTF-8", // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + FOO_SRC_UTF8_FTL); + // Legacy mode doesn't deal with outputEncoding, but it's inherited by the Environment from the Template: + assertOutputEncodingEquals( + CFG_DEFAULT_ENCODING, // <- expected response.characterEncoding + "UTF-8", // <- expected env.outputEncoding + initParamValue, // <- init-param + FOO_OUT_UTF8_FTL); + // Charset in content type is the strongest: + assertOutputEncodingEquals( + "ISO-8859-2", // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + "text/html; charset=ISO-8859-2", // ContentType init-param + FOO_FTL); + assertOutputEncodingEquals( + "ISO-8859-2", // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + "text/html; charset=ISO-8859-2", // ContentType init-param + FOO_SRC_UTF8_FTL); + assertOutputEncodingEquals( + "UTF-8", // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + CONTENT_TYPE_ATTR_WITH_CHARSET_FTL); + assertOutputEncodingEquals( + "UTF-8", // <- expected response.characterEncoding + null, // <- expected env.outputEncoding + initParamValue, // <- init-param + "text/html; charset=ISO-8859-2", // ContentType init-param + CONTENT_TYPE_ATTR_WITH_CHARSET_FTL); + } // Non-legacy mode always keeps env.outputEncoding in sync. with the Servlet response encoding: assertOutputEncodingEquals( @@ -249,6 +264,12 @@ public class FreemarkerServletTest { } catch (ServletException e) { assertThat(e.getCause().getCause().getMessage(), containsString(FreemarkerServlet.INIT_PARAM_VALUE_LEGACY)); } + // But the legacy content_type template attribute can still set the output charset: + assertOutputEncodingEquals( + "UTF-8", // <- expected response.characterEncoding + "UTF-8", // <- expected env.outputEncoding + FreemarkerServlet.INIT_PARAM_VALUE_FROM_TEMPLATE, // <- init-param + CONTENT_TYPE_ATTR_WITH_CHARSET_FTL); // Do not set mode: assertOutputEncodingEquals( @@ -269,8 +290,8 @@ public class FreemarkerServletTest { // Not allowed to specify the charset in the contentType init-param: try { assertOutputEncodingEquals( - null, // <- expected response.characterEncoding - null, // <- expected env.outputEncoding + SERVLET_RESPONSE_DEFAULT_CHARSET, // <- expected response.characterEncoding + SERVLET_RESPONSE_DEFAULT_CHARSET, // <- expected env.outputEncoding FreemarkerServlet.INIT_PARAM_VALUE_DO_NOT_SET, // <- init-param "text/html; charset=ISO-8859-2", // ContentType init-param FOO_FTL); @@ -278,6 +299,12 @@ public class FreemarkerServletTest { } catch (ServletException e) { assertThat(e.getCause().getCause().getMessage(), containsString(FreemarkerServlet.INIT_PARAM_VALUE_LEGACY)); } + // The legacy content_type template attribute can still specify an output charset, though it will be ignored: + assertOutputEncodingEquals( + SERVLET_RESPONSE_DEFAULT_CHARSET, // <- expected response.characterEncoding + SERVLET_RESPONSE_DEFAULT_CHARSET, // <- expected env.outputEncoding + FreemarkerServlet.INIT_PARAM_VALUE_DO_NOT_SET, // <- init-param + CONTENT_TYPE_ATTR_WITH_CHARSET_FTL); // Forced mode: assertOutputEncodingEquals( @@ -308,8 +335,8 @@ public class FreemarkerServletTest { // Not allowed to specify the charset in the contentType init-param: try { assertOutputEncodingEquals( - null, // <- expected response.characterEncoding - null, // <- expected env.outputEncoding + "UTF-16LE", // <- expected response.characterEncoding + "UTF-16LE", // <- expected env.outputEncoding FreemarkerServlet.INIT_PARAM_VALUE_FORCE_PREFIX + "UTF-16LE", // <- init-param "text/html; charset=ISO-8859-2", // ContentType init-param FOO_FTL); @@ -317,6 +344,12 @@ public class FreemarkerServletTest { } catch (ServletException e) { assertThat(e.getCause().getCause().getMessage(), containsString(FreemarkerServlet.INIT_PARAM_VALUE_LEGACY)); } + // The legacy content_type template attribute can still specify an output charset, though it will be overridden: + assertOutputEncodingEquals( + "UTF-16LE", // <- expected response.characterEncoding + "UTF-16LE", // <- expected env.outputEncoding + FreemarkerServlet.INIT_PARAM_VALUE_FORCE_PREFIX + "UTF-16LE", // <- init-param + CONTENT_TYPE_ATTR_WITH_CHARSET_FTL); } private void assertResponseContentTypeEquals( @@ -495,6 +528,7 @@ public class FreemarkerServletTest { tl.putTemplate(FOO_SRC_UTF8_FTL, "foo"); tl.putTemplate(FOO_OUT_UTF8_FTL, "foo"); tl.putTemplate(CONTENT_TYPE_ATTR_FTL, "<#ftl attributes={ 'content_type': 'text/plain' }>foo"); + tl.putTemplate(CONTENT_TYPE_ATTR_WITH_CHARSET_FTL, "<#ftl attributes={ 'content_type': 'text/plain; charset=UTF-8' }>foo"); tl.putTemplate(OUTPUT_FORMAT_HEADER_FTL, "<#ftl outputFormat='plainText'>foo"); return tl; } else {
