This is an automated email from the ASF dual-hosted git repository. ddekany pushed a commit to branch 2.3-gae in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit e5fa45f4c589b306b5a8ee8c5a42229d378eb607 Author: ddekany <[email protected]> AuthorDate: Wed Dec 28 21:46:24 2022 +0100 Improved StringUtil.jsStringEnc to support both quotation marks and apostrophe. Added a mode that is both compatible with JSON and JavaScript, without decreasing safety when used in JavaScript. --- .../freemarker/core/AbstractLegacyCFormat.java | 5 +- src/main/java/freemarker/core/JSONCFormat.java | 4 +- .../java/freemarker/core/JavaScriptCFormat.java | 4 +- .../freemarker/template/utility/StringUtil.java | 117 ++++++++++++++++----- src/manual/en_US/book.xml | 8 ++ .../template/utility/StringUtilTest.java | 77 +++++++++++++- 6 files changed, 183 insertions(+), 32 deletions(-) diff --git a/src/main/java/freemarker/core/AbstractLegacyCFormat.java b/src/main/java/freemarker/core/AbstractLegacyCFormat.java index 99440684..bdf7577d 100644 --- a/src/main/java/freemarker/core/AbstractLegacyCFormat.java +++ b/src/main/java/freemarker/core/AbstractLegacyCFormat.java @@ -25,6 +25,8 @@ import freemarker.template.TemplateException; import freemarker.template.TemplateModelException; import freemarker.template.TemplateNumberModel; import freemarker.template.utility.StringUtil; +import freemarker.template.utility.StringUtil.JsStringEncCompatibility; +import freemarker.template.utility.StringUtil.JsStringEncQuotation; /** * Super class of {@link CFormat}-s that merely exist to mimic old {@code ?c} behavior for backward compatibility. @@ -43,7 +45,8 @@ public abstract class AbstractLegacyCFormat extends CFormat { @Override final String formatString(String s, Environment env) throws TemplateException { - return StringUtil.jsStringEnc(s, true, true); + return StringUtil.jsStringEnc( + s, JsStringEncCompatibility.JAVA_SCRIPT_OR_JSON, JsStringEncQuotation.QUOTATION_MARK); } @Override diff --git a/src/main/java/freemarker/core/JSONCFormat.java b/src/main/java/freemarker/core/JSONCFormat.java index 5cf4c52a..ee469941 100644 --- a/src/main/java/freemarker/core/JSONCFormat.java +++ b/src/main/java/freemarker/core/JSONCFormat.java @@ -23,6 +23,8 @@ import freemarker.template.Configuration; import freemarker.template.TemplateException; import freemarker.template.Version; import freemarker.template.utility.StringUtil; +import freemarker.template.utility.StringUtil.JsStringEncCompatibility; +import freemarker.template.utility.StringUtil.JsStringEncQuotation; /** * JSON {@link CFormat}; when this is used, values output by {@code ?c} are valid JSON values, and therefore also @@ -46,7 +48,7 @@ public final class JSONCFormat extends AbstractJSONLikeFormat { @Override String formatString(String s, Environment env) throws TemplateException { - return StringUtil.jsStringEnc(s, true, true); + return StringUtil.jsStringEnc(s, JsStringEncCompatibility.JSON, JsStringEncQuotation.QUOTATION_MARK); } @Override diff --git a/src/main/java/freemarker/core/JavaScriptCFormat.java b/src/main/java/freemarker/core/JavaScriptCFormat.java index d9ba7229..1cb44adf 100644 --- a/src/main/java/freemarker/core/JavaScriptCFormat.java +++ b/src/main/java/freemarker/core/JavaScriptCFormat.java @@ -21,6 +21,8 @@ package freemarker.core; import freemarker.template.TemplateException; import freemarker.template.utility.StringUtil; +import freemarker.template.utility.StringUtil.JsStringEncCompatibility; +import freemarker.template.utility.StringUtil.JsStringEncQuotation; /** * JavaScript {@link CFormat}. This is almost the same as {@link JSONCFormat}, but it uses shorter forms where @@ -41,7 +43,7 @@ public final class JavaScriptCFormat extends AbstractJSONLikeFormat { @Override String formatString(String s, Environment env) throws TemplateException { - return StringUtil.jsStringEnc(s, false, true); + return StringUtil.jsStringEnc(s, JsStringEncCompatibility.JAVA_SCRIPT, JsStringEncQuotation.QUOTATION_MARK); } @Override diff --git a/src/main/java/freemarker/template/utility/StringUtil.java b/src/main/java/freemarker/template/utility/StringUtil.java index 0d76a48f..51f22a73 100644 --- a/src/main/java/freemarker/template/utility/StringUtil.java +++ b/src/main/java/freemarker/template/utility/StringUtil.java @@ -19,6 +19,8 @@ package freemarker.template.utility; +import static freemarker.template.utility.StringUtil.JsStringEncQuotation.*; + import java.io.IOException; import java.io.UnsupportedEncodingException; import java.io.Writer; @@ -1320,7 +1322,7 @@ public class StringUtil { /** * Escapes a {@link String} to be safely insertable into a JavaScript string literal; for more see - * {@link #jsStringEnc(String, boolean, boolean) jsStringEnc(s, false, false)}. + * {@link #jsStringEnc(String, JsStringEncCompatibility, JsStringEncQuotation) jsStringEnc(s, false, QuotationMode.NONE)}. */ public static String javaScriptStringEnc(String s) { return jsStringEnc(s, false); @@ -1340,12 +1342,12 @@ public class StringUtil { /** * Escapes a {@link String} to be safely insertable into a JSON or JavaScript string literal; for more see - * {@link #jsStringEnc(String, boolean, boolean) jsStringEnc(s, json, false)}. + * {@link #jsStringEnc(String, JsStringEncCompatibility, JsStringEncQuotation) jsStringEnc(s, json, QuotationMode.NONE)}. * * @since 2.3.20 */ public static String jsStringEnc(String s, boolean json) { - return jsStringEnc(s, json, false); + return jsStringEnc(s, json ? JsStringEncCompatibility.JSON : JsStringEncCompatibility.JAVA_SCRIPT, null); } /** @@ -1399,22 +1401,24 @@ public class StringUtil { * </table> * * @param s The string to escape - * @param json If escaping should restrict itself to rules that are valid in both JSON and JavaScript. - * @param quote If quotation marks should be added around the result. - * Currently, it's always ({@code "}, not {@code '}). + * @param compatibility If escaping should restrict itself to rules that are valid in JSON, in JavaScript, or in both. + * @param quotation In not {@code null}, quotation marks of this type are added around the value. * * @since 2.3.32 */ - public static String jsStringEnc(String s, boolean json, boolean quote) { + public static String jsStringEnc(String s, JsStringEncCompatibility compatibility, JsStringEncQuotation quotation) { NullArgumentException.check("s", s); int ln = s.length(); StringBuilder sb; - if (quote) { - sb = new StringBuilder(ln + 8); - sb.append('"'); - } else { + if (quotation == null) { sb = null; + } else { + if (quotation == APOSTROPHE && compatibility.jsonCompatible) { + throw new IllegalArgumentException("JSON compatible mode doesn't allow quotationMode=" + quotation); + } + sb = new StringBuilder(ln + 8); + sb.append(quotation.getSymbol()); } for (int i = 0; i < ln; i++) { final char c = s.charAt(i); @@ -1435,19 +1439,23 @@ public class StringUtil { escapeType = ESC_HEXA; } } else if (c == '"') { - escapeType = ESC_BACKSLASH; + escapeType = quotation == APOSTROPHE ? NO_ESC : ESC_BACKSLASH; } else if (c == '\'') { - escapeType = json || quote ? NO_ESC : ESC_BACKSLASH; + escapeType = !compatibility.javaScriptCompatible || quotation == QUOTATION_MARK ? NO_ESC + : (compatibility.jsonCompatible ? ESC_HEXA : ESC_BACKSLASH); } else if (c == '\\') { escapeType = ESC_BACKSLASH; - } else if (c == '/' && (i == 0 || s.charAt(i - 1) == '<')) { // against closing elements - escapeType = quote ? NO_ESC : ESC_BACKSLASH; - } else if (c == '>') { // against "]]> and "-->" + } else if (c == '/' + && (i == 0 && quotation == null || i != 0 && s.charAt(i - 1) == '<')) { + // against closing elements with "</" + escapeType = ESC_BACKSLASH; + } else if (c == '>') { + // against "]]> and "-->" final boolean dangerous; - if (i == 0) { - dangerous = true; - } else if (quote) { + if (quotation != null && i < 2) { dangerous = false; + } else if (i == 0) { + dangerous = true; } else { final char prevC = s.charAt(i - 1); if (prevC == ']' || prevC == '-') { @@ -1461,13 +1469,12 @@ public class StringUtil { dangerous = false; } } - escapeType = dangerous ? (json ? ESC_HEXA : ESC_BACKSLASH) : NO_ESC; - } else if (c == '<') { // against "<!" + escapeType = dangerous ? (compatibility.jsonCompatible ? ESC_HEXA : ESC_BACKSLASH) : NO_ESC; + } else if (c == '<') { + // against "<!" final boolean dangerous; if (i == ln - 1) { - dangerous = true; - } else if (quote) { - dangerous = false; + dangerous = quotation == null; } else { char nextC = s.charAt(i + 1); dangerous = nextC == '!' || nextC == '?'; @@ -1491,7 +1498,7 @@ public class StringUtil { if (escapeType > 0x20) { sb.append((char) escapeType); } else if (escapeType == ESC_HEXA) { - if (!json && c < 0x100) { + if (!compatibility.jsonCompatible && c < 0x100) { sb.append('x'); sb.append(toHexDigitUpperCase(c >> 4)); sb.append(toHexDigitUpperCase(c & 0xF)); @@ -1515,8 +1522,8 @@ public class StringUtil { if (sb != null) sb.append(c); } // for each characters - if (quote) { - sb.append('"'); + if (quotation != null) { + sb.append(quotation.getSymbol()); } return sb == null ? s : sb.toString(); @@ -2168,5 +2175,59 @@ public class StringUtil { } return sb.toString(); } - + + /** + * Used as the argument of {@link StringUtil#jsStringEnc(String, JsStringEncCompatibility, JsStringEncQuotation)}. + * + * @since 2.3.32 + */ + public enum JsStringEncCompatibility { + /** + * Output is expected to be used in JavaScript, not in JSON. + */ + JAVA_SCRIPT(true, false), + + /** + * Output is expected to be used in JSON, not in JavaScript. While JSON is compatible with JavaScript, in this + * mode we don't care about escaping apostrophe, as it's not special in JSON. + */ + JSON(false, true), + + /** + * Output is expected to be used both in JSON and JavaScript. + */ + JAVA_SCRIPT_OR_JSON(true, true); + + JsStringEncCompatibility(boolean javaScriptCompatible, boolean jsonCompatible) { + this.javaScriptCompatible = javaScriptCompatible; + this.jsonCompatible = jsonCompatible; + } + + private final boolean javaScriptCompatible; + private final boolean jsonCompatible; + + boolean isJSONCompatible() { + return jsonCompatible; + } + } + + /** + * Used as the argument of {@link StringUtil#jsStringEnc(String, JsStringEncCompatibility, JsStringEncQuotation)}. + * + * @since 2.3.32 + */ + public enum JsStringEncQuotation { + QUOTATION_MARK('"'), + APOSTROPHE('\''); + + private final char symbol; + + JsStringEncQuotation(char symbol) { + this.symbol = symbol; + } + + public char getSymbol() { + return symbol; + } + } } diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index ed9ea259..b4288ced 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -30211,6 +30211,14 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> mode.</para> </listitem> + <listitem> + <para>Improved <literal>StringUtil.jsStringEnc</literal> and + <literal>javaSctringEsc</literal> to support quoting. Also + <literal>jsStringEnc</literal> now have a mode that targets both + JavaScript and JSON, and doesn't give up apostrophe + escaping.</para> + </listitem> + <listitem> <para><link xlink:href="https://issues.apache.org/jira/browse/FREEMARKER-198";>FREEMARKER-198</link>: diff --git a/src/test/java/freemarker/template/utility/StringUtilTest.java b/src/test/java/freemarker/template/utility/StringUtilTest.java index 0bb29345..1614dc0d 100644 --- a/src/test/java/freemarker/template/utility/StringUtilTest.java +++ b/src/test/java/freemarker/template/utility/StringUtilTest.java @@ -19,6 +19,8 @@ package freemarker.template.utility; +import static freemarker.template.utility.StringUtil.JsStringEncCompatibility.*; +import static freemarker.template.utility.StringUtil.JsStringEncQuotation.*; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; @@ -30,6 +32,7 @@ import org.hamcrest.Matchers; import org.junit.Test; import freemarker.core.ParseException; +import freemarker.template.utility.StringUtil.JsStringEncCompatibility; public class StringUtilTest { @@ -449,5 +452,77 @@ public class StringUtilTest { StringUtil.RTFEnc(in, sw); assertEquals(expected, sw.toString()); } - + + @Test + public void jsStringEncQuotationTests() { + for (JsStringEncCompatibility anyCompatibility : JsStringEncCompatibility.values()) { + JsStringEncCompatibility anyJsonCompatible = anyCompatibility.isJSONCompatible() ? anyCompatibility : JSON; + + assertEquals("", StringUtil.jsStringEnc("", anyCompatibility, null)); + assertEquals("''", StringUtil.jsStringEnc("", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"\"", StringUtil.jsStringEnc("", anyCompatibility, QUOTATION_MARK)); + + assertEquals("a", StringUtil.jsStringEnc("a", anyCompatibility, null)); + assertEquals("a", StringUtil.jsStringEnc("a", anyCompatibility, null)); + assertEquals("'a'", StringUtil.jsStringEnc("a", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"a\"", StringUtil.jsStringEnc("a", anyCompatibility, QUOTATION_MARK)); + + try { + StringUtil.jsStringEnc("", anyJsonCompatible, APOSTROPHE); + fail(); + } catch (IllegalArgumentException e) { + // expected + } + + assertEquals("a\\'b", StringUtil.jsStringEnc("a'b", JAVA_SCRIPT, null)); + assertEquals("a'b", StringUtil.jsStringEnc("a'b", JSON, null)); + assertEquals("a\\u0027b", StringUtil.jsStringEnc("a'b", JAVA_SCRIPT_OR_JSON, null)); + assertEquals("'a\\'b'", StringUtil.jsStringEnc("a'b", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"a'b\"", StringUtil.jsStringEnc("a'b", anyCompatibility, QUOTATION_MARK)); + + assertEquals("<\\/e>", StringUtil.jsStringEnc("</e>", anyCompatibility, null)); + assertEquals("'<\\/e>'", StringUtil.jsStringEnc("</e>", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"<\\/e>\"", StringUtil.jsStringEnc("</e>", anyCompatibility, QUOTATION_MARK)); + + assertEquals("\\/e>", StringUtil.jsStringEnc("/e>", anyCompatibility, null)); + assertEquals("'/e>'", StringUtil.jsStringEnc("/e>", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"/e>\"", StringUtil.jsStringEnc("/e>", anyCompatibility, QUOTATION_MARK)); + + assertEquals("\\>", StringUtil.jsStringEnc(">", JAVA_SCRIPT, null)); + assertEquals("\\u003E", StringUtil.jsStringEnc(">", JSON, null)); + assertEquals("'>'", StringUtil.jsStringEnc(">", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\">\"", StringUtil.jsStringEnc(">", anyCompatibility, QUOTATION_MARK)); + + assertEquals("-\\>", StringUtil.jsStringEnc("->", JAVA_SCRIPT, null)); + assertEquals("-\\u003E", StringUtil.jsStringEnc("->", JSON, null)); + assertEquals("'->'", StringUtil.jsStringEnc("->", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"->\"", StringUtil.jsStringEnc("->", JAVA_SCRIPT, QUOTATION_MARK)); + + assertEquals("--\\>", StringUtil.jsStringEnc("-->", JAVA_SCRIPT, null)); + assertEquals("--\\u003E", StringUtil.jsStringEnc("-->", anyJsonCompatible, null)); + assertEquals("'--\\>'", StringUtil.jsStringEnc("-->", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"--\\>\"", StringUtil.jsStringEnc("-->", JAVA_SCRIPT, QUOTATION_MARK)); + assertEquals("\"--\\u003E\"", StringUtil.jsStringEnc("-->", anyJsonCompatible, QUOTATION_MARK)); + + assertEquals("x->", StringUtil.jsStringEnc("x->", anyCompatibility, null)); + assertEquals("'x->'", StringUtil.jsStringEnc("x->", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"x->\"", StringUtil.jsStringEnc("x->", anyCompatibility, QUOTATION_MARK)); + + assertEquals("\\x3C", StringUtil.jsStringEnc("<", JAVA_SCRIPT, null)); + assertEquals("\\u003C", StringUtil.jsStringEnc("<", JSON, null)); + assertEquals("'<'", StringUtil.jsStringEnc("<", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"<\"", StringUtil.jsStringEnc("<", anyCompatibility, QUOTATION_MARK)); + + assertEquals("\\x3C!", StringUtil.jsStringEnc("<!", JAVA_SCRIPT, null)); + assertEquals("\\u003C!", StringUtil.jsStringEnc("<!", JSON, null)); + assertEquals("'\\x3C!'", StringUtil.jsStringEnc("<!", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"\\x3C!\"", StringUtil.jsStringEnc("<!", JAVA_SCRIPT, QUOTATION_MARK)); + assertEquals("\"\\u003C!\"", StringUtil.jsStringEnc("<!", anyJsonCompatible, QUOTATION_MARK)); + + assertEquals("<x", StringUtil.jsStringEnc("<x", anyCompatibility, null)); + assertEquals("'<x'", StringUtil.jsStringEnc("<x", JAVA_SCRIPT, APOSTROPHE)); + assertEquals("\"<x\"", StringUtil.jsStringEnc("<x", anyCompatibility, QUOTATION_MARK)); + } + } + }
