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 cf5164cadb191d58f645bc4d2917482a29105561
Author: ddekany <[email protected]>
AuthorDate: Sat Dec 16 21:45:02 2023 +0100

    For PR #88 forceAutoEscape: Better error messages. Some more test coverage.
---
 src/main/javacc/FTL.jj                             | 72 ++++++++++++----------
 .../java/freemarker/core/OutputFormatTest.java     | 23 ++++++-
 2 files changed, 59 insertions(+), 36 deletions(-)

diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj
index a7447275..55955c5c 100644
--- a/src/main/javacc/FTL.jj
+++ b/src/main/javacc/FTL.jj
@@ -237,11 +237,9 @@ public class FMParser {
                 outputFormat = outputFormatFromExt;
             }
         }
-       if (!(outputFormat instanceof MarkupOutputFormat)
-           && autoEscapingPolicy == Configuration.FORCE_AUTO_ESCAPING_POLICY) {
-           throw new IllegalArgumentException(
-                    "Non-markup output format cannot be used when 
auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY.");
-       }
+        if (!(outputFormat instanceof MarkupOutputFormat) && 
autoEscapingPolicy == Configuration.FORCE_AUTO_ESCAPING_POLICY) {
+            throw new 
IllegalArgumentException(forcedAutoEscapingPolicyExceptionMessage(outputFormat));
+        }
         recalculateAutoEscapingField();
 
         token_source.setParser(this);
@@ -286,17 +284,17 @@ public class FMParser {
             _TemplateAPI.setOutputFormat(template, outputFormat);
         }
     }
-    
+
     void setupStringLiteralMode(FMParser parentParser, OutputFormat 
outputFormat) {
         FMParserTokenManager parentTokenSource = parentParser.token_source;
-         
+
         token_source.initialNamingConvention = 
parentTokenSource.initialNamingConvention;
         token_source.namingConvention = parentTokenSource.namingConvention;
         token_source.namingConventionEstabilisher = 
parentTokenSource.namingConventionEstabilisher;
         token_source.SwitchTo(NO_DIRECTIVE);
-        
+
         this.outputFormat = outputFormat;
-        recalculateAutoEscapingField();                                
+        recalculateAutoEscapingField();
         if (incompatibleImprovements < _VersionInts.V_2_3_24) {
             // Emulate bug, where the string literal parser haven't inherited 
the IcI:
             incompatibleImprovements = _VersionInts.V_2_3_0;
@@ -527,6 +525,15 @@ public class FMParser {
         }
     }
 
+    private static String 
forcedAutoEscapingPolicyExceptionMessage(OutputFormat outputFormat) {
+        return forcedAutoEscapingPolicyExceptionMessage("Non-markup output 
format " + outputFormat);
+    }
+
+    private static String forcedAutoEscapingPolicyExceptionMessage(String 
whatCanNotBeUsed) {
+        return whatCanNotBeUsed + " can't be used when the \"" + 
Configuration.AUTO_ESCAPING_POLICY_KEY + "\" "
+                + "configuration setting was set to \"force\" 
(FORCE_AUTO_ESCAPING_POLICY).";
+    }
+
     private ParserIteratorBlockContext pushIteratorBlockContext() {
         if (iteratorBlockContexts == null) {
             iteratorBlockContexts = new 
ArrayList<ParserIteratorBlockContext>(4);
@@ -571,7 +578,7 @@ public class FMParser {
            // [2.4] Use camel case as the default
            return token_source.namingConvention == 
Configuration.CAMEL_CASE_NAMING_CONVENTION ? "#forEach" : "#foreach";
        }
-    
+
 }
 
 PARSER_END(FMParser)
@@ -2230,13 +2237,13 @@ Expression BuiltIn(Expression lhoExp) :
             return result;
         }
 
-       if (result instanceof BuiltInBannedWhenForcedAutoEscaping) {
-           if (autoEscapingPolicy == Configuration.FORCE_AUTO_ESCAPING_POLICY) 
{
-               throw new ParseException(
-                       "Using ?" + t.image + " is not allowed while 
auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY.",
-                       template, t);
-           }
-       }
+        if (result instanceof BuiltInBannedWhenForcedAutoEscaping) {
+            if (autoEscapingPolicy == 
Configuration.FORCE_AUTO_ESCAPING_POLICY) {
+                throw new ParseException(
+                        forcedAutoEscapingPolicyExceptionMessage("The ?" + 
t.image + " expression"),
+                        template, t);
+            }
+        }
 
         if (result instanceof MarkupOutputFormatBoundBuiltIn) {
             if (!(outputFormat instanceof MarkupOutputFormat)) {
@@ -4060,12 +4067,10 @@ OutputFormatBlock OutputFormat() :
             } else {
                 outputFormat = 
template.getConfiguration().getOutputFormat(paramStr);
             }
-           if (!(outputFormat instanceof MarkupOutputFormat)
-               && autoEscapingPolicy == 
Configuration.FORCE_AUTO_ESCAPING_POLICY) {
-               throw new ParseException(
-                       "Non-markup output format cannot be used when 
auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY.",
-                       template, start);
-           }
+            if (!(outputFormat instanceof MarkupOutputFormat)
+                    && autoEscapingPolicy == 
Configuration.FORCE_AUTO_ESCAPING_POLICY) {
+                throw new 
ParseException(forcedAutoEscapingPolicyExceptionMessage(outputFormat), 
template, start);
+            }
             recalculateAutoEscapingField();
         } catch (IllegalArgumentException e) {
             throw new ParseException("Invalid format name: " + e.getMessage(), 
template, start, e.getCause());
@@ -4120,11 +4125,12 @@ NoAutoEscBlock NoAutoEsc() :
 {
     start = <NOAUTOESC>
     {
-       if (autoEscapingPolicy == Configuration.FORCE_AUTO_ESCAPING_POLICY) {
-           throw new ParseException(
-               "<#noautoesc> cannot be used when auto_escaping_policy is 
FORCE_AUTO_ESCAPING_POLICY.",
-               template, start);
-       }
+        if (autoEscapingPolicy == Configuration.FORCE_AUTO_ESCAPING_POLICY) {
+            throw new ParseException(
+                    forcedAutoEscapingPolicyExceptionMessage(
+                            "<#" + (token_source.namingConvention == 
Configuration.CAMEL_CASE_NAMING_CONVENTION ? "noAutoEsc" : "noautoesc") + ">"),
+                    template, start);
+        }
         lastAutoEscapingPolicy = autoEscapingPolicy;
         autoEscapingPolicy = Configuration.DISABLE_AUTO_ESCAPING_POLICY;
         recalculateAutoEscapingField();
@@ -4564,11 +4570,11 @@ void HeaderElement() :
                                 autoEscRequester = key;
                                 autoEscapingPolicy = 
Configuration.ENABLE_IF_SUPPORTED_AUTO_ESCAPING_POLICY;
                             } else {
-                               if (autoEscapingPolicy == 
Configuration.FORCE_AUTO_ESCAPING_POLICY) {
-                                   throw new ParseException(
-                                           "auto_esc setting cannot be used 
when auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY.",
-                                           exp);
-                               }
+                                if (autoEscapingPolicy == 
Configuration.FORCE_AUTO_ESCAPING_POLICY) {
+                                    throw new ParseException(
+                                            
forcedAutoEscapingPolicyExceptionMessage("auto_esc setting"),
+                                            exp);
+                                }
                                 autoEscapingPolicy = 
Configuration.DISABLE_AUTO_ESCAPING_POLICY;
                             }
                             recalculateAutoEscapingField();
diff --git a/src/test/java/freemarker/core/OutputFormatTest.java 
b/src/test/java/freemarker/core/OutputFormatTest.java
index 14d3ecfc..03672c76 100644
--- a/src/test/java/freemarker/core/OutputFormatTest.java
+++ b/src/test/java/freemarker/core/OutputFormatTest.java
@@ -868,10 +868,27 @@ public class OutputFormatTest extends TemplateTest {
         cfg.setOutputFormat(DummyOutputFormat.INSTANCE);
         assertOutput(commonFTL, esced);
 
+        // TODO Should work:
+        // cfg.setOutputFormat(PlainTextOutputFormat.INSTANCE);
+        // assertOutput("<#ftl outputFormat='seldomEscaped'>" + commonFTL, 
esced);
+
+        cfg.setOutputFormat(HTMLOutputFormat.INSTANCE);
+        assertOutput("<#outputFormat 'seldomEscaped'>" + commonFTL + 
"</#outputFormat>", esced);
+
+        cfg.setOutputFormat(PlainTextOutputFormat.INSTANCE);
+        assertErrorContains("", IllegalArgumentException.class,
+                "plainText", "auto_escaping_policy", "force");
         cfg.setOutputFormat(DummyOutputFormat.INSTANCE);
-        assertErrorContains("<#outputformat 'plainText'></#outputformat>", 
"auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY");
-        assertErrorContains("<#noAutoEsc></#noAutoEsc>", "auto_escaping_policy 
is FORCE_AUTO_ESCAPING_POLICY");
-        assertErrorContains("<#assign foo='bar'>${foo?noEsc}", 
"auto_escaping_policy is FORCE_AUTO_ESCAPING_POLICY");
+        assertErrorContains("<#outputformat 'plainText'></#outputformat>", 
ParseException.class,
+                "plainText", "auto_escaping_policy", "force");
+        assertErrorContains("<#noAutoEsc></#noAutoEsc>", ParseException.class,
+                "noAutoEsc", "auto_escaping_policy", "force");
+        assertErrorContains("<#noautoesc></#noautoesc>", ParseException.class,
+                "noautoesc", "auto_escaping_policy", "force");
+        assertErrorContains("<#assign foo='bar'>${foo?no_esc}", 
ParseException.class,
+                "?no_esc", "auto_escaping_policy", "force");
+        assertErrorContains("<#assign foo='bar'>${foo?noEsc}", 
ParseException.class,
+                "?noEsc", "auto_escaping_policy", "force");
     }
 
     @Test

Reply via email to