Fix for GRAILS-9247 and GRAILS-9392 g:fieldValue encoding issues

lhotari · lhotari · commit 7f0f3af924b3 · 2012-09-10T07:01:31.000+03:00
diff --git a/grails-plugin-codecs/src/main/groovy/org/codehaus/groovy/grails/plugins/codecs/HTMLCodec.java b/grails-plugin-codecs/src/main/groovy/org/codehaus/groovy/grails/plugins/codecs/HTMLCodec.java
@@ -14,6 +14,7 @@
  */
 package org.codehaus.groovy.grails.plugins.codecs;
 
+import org.codehaus.groovy.grails.web.servlet.GrailsApplicationAttributes;
 import org.codehaus.groovy.grails.web.util.StreamCharBuffer;
 import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.RequestContextHolder;
@@ -40,7 +41,7 @@ public static CharSequence encode(Object target) {
     public static boolean shouldEncode() {
         final RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
         if (attributes != null) {
-            Object codecName = attributes.getAttribute("org.codehaus.groovy.grails.GSP_CODEC",
+            Object codecName = attributes.getAttribute(GrailsApplicationAttributes.GSP_CODEC,
                     RequestAttributes.SCOPE_REQUEST);
             if (codecName != null && codecName.toString().equalsIgnoreCase("html")) {
                 return false;
diff --git a/grails-plugin-gsp/src/main/groovy/org/codehaus/groovy/grails/plugins/web/taglib/ValidationTagLib.groovy b/grails-plugin-gsp/src/main/groovy/org/codehaus/groovy/grails/plugins/web/taglib/ValidationTagLib.groovy
@@ -23,6 +23,7 @@ import java.text.DecimalFormat
 import java.text.DecimalFormatSymbols
 import org.apache.commons.lang.StringEscapeUtils
 import org.codehaus.groovy.grails.plugins.codecs.HTMLCodec
+import org.codehaus.groovy.grails.web.taglib.GroovyPageAttributes;
 import org.springframework.beans.PropertyEditorRegistry
 import org.springframework.context.MessageSourceResolvable
 import org.springframework.context.NoSuchMessageException
@@ -76,6 +77,9 @@ class ValidationTagLib {
     Closure fieldValue = { attrs, body ->
         def bean = attrs.bean
         def field = attrs.field?.toString()
+        
+        def tagSyntaxCall = (attrs instanceof GroovyPageAttributes) ? attrs.isGspTagSyntaxCall() : false
+        
         if (bean && field) {
             if (bean.metaClass.hasProperty(bean,'errors')) {
                 Errors errors = bean.errors
@@ -87,7 +91,7 @@ class ValidationTagLib {
                     }
                 }
                 if (rejectedValue != null) {
-                    out << formatValue(rejectedValue, field)
+                    out << formatValue(rejectedValue, field, tagSyntaxCall)
                 }
             }
             else {
@@ -96,7 +100,7 @@ class ValidationTagLib {
                     rejectedValue = rejectedValue?."$fieldPart"
                 }
                 if (rejectedValue != null) {
-                    out << formatValue(rejectedValue, field)
+                    out << formatValue(rejectedValue, field, tagSyntaxCall)
                 }
             }
         }
@@ -422,12 +426,12 @@ class ValidationTagLib {
      * formatted according to the current user's locale during the
      * conversion to a string.
      */
-    def formatValue(value, String propertyPath = null) {
+    def formatValue(value, String propertyPath = null, Boolean tagSyntaxCall = false) {
         PropertyEditorRegistry registry = RequestContextHolder.currentRequestAttributes().getPropertyEditorRegistry()
         PropertyEditor editor = registry.findCustomEditor(value.getClass(), propertyPath)
         if (editor) {
             editor.setValue(value)
-            return HTMLCodec.shouldEncode() && !(value instanceof Number) ? editor.asText?.encodeAsHTML() : editor.asText
+            return (tagSyntaxCall || HTMLCodec.shouldEncode()) && !(value instanceof Number) ? editor.asText?.encodeAsHTML() : editor.asText
         }
 
         if (value instanceof Number) {
@@ -445,6 +449,6 @@ class ValidationTagLib {
             value = message(message: value)
         }
 
-        return HTMLCodec.shouldEncode() ? value.toString().encodeAsHTML() : value
+        return (tagSyntaxCall || HTMLCodec.shouldEncode()) ? value.toString().encodeAsHTML() : value
     }
 }
diff --git a/grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/web/taglib/ValidationTagLibTests.groovy b/grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/web/taglib/ValidationTagLibTests.groovy
@@ -1,5 +1,6 @@
 package org.codehaus.groovy.grails.web.taglib
 
+import org.codehaus.groovy.grails.support.MockStringResourceLoader
 import org.codehaus.groovy.grails.web.servlet.GrailsApplicationAttributes
 import org.springframework.beans.factory.support.RootBeanDefinition
 import org.springframework.context.MessageSourceResolvable
@@ -118,14 +119,49 @@ enum Title implements org.springframework.context.MessageSourceResolvable {
         b.properties = [title:"<script>alert('escape me')</script>"]
 
         def template = '''<g:fieldValue bean="${book}" field="title" />'''
+        def htmlCodecDirective = '<%@page defaultCodec="HTML" %>'
+        def expected = "&lt;script&gt;alert(&#39;escape me&#39;)&lt;/script&gt;"
+        assertOutputEquals(expected, template, [book:b])
+        assertOutputEquals(expected, htmlCodecDirective + template, [book:b])
+    }
+    
+    void testFieldValueHtmlEscapingWithFunctionSyntaxCall() {
+        def b = ga.getDomainClass("ValidationTagLibBook").newInstance()
+        b.properties = [title:"<script>alert('escape me')</script>"]
 
-        assertOutputEquals("&lt;script&gt;alert(&#39;escape me&#39;)&lt;/script&gt;", template, [book:b])
+        def template = '''${fieldValue(bean:book, field:"title")}'''
+        def htmlCodecDirective = '<%@page defaultCodec="HTML" %>'
+        def expected = "&lt;script&gt;alert(&#39;escape me&#39;)&lt;/script&gt;"
+        assertOutputEquals(expected, template, [book:b])
+        assertOutputEquals(expected, htmlCodecDirective + template, [book:b])
+    }
 
-        request.setAttribute("org.codehaus.groovy.grails.GSP_CODEC", 'html')
+    void testFieldValueHtmlEscapingDifferentEncodings() {
+        def b = ga.getDomainClass("ValidationTagLibBook").newInstance()
+        b.properties = [title:"<script>alert('escape me')</script>"]
 
-        assertOutputEquals("<script>alert('escape me')</script>", template, [book:b])
+        def template = '''${fieldValue(bean:book, field:"title")}'''
+        def htmlCodecDirective = '<%@page defaultCodec="HTML" %>'
+        def expected = "&lt;script&gt;alert(&#39;escape me&#39;)&lt;/script&gt;"
+        
+        def resourceLoader = new MockStringResourceLoader()
+        resourceLoader.registerMockResource('/_sometemplate.gsp', htmlCodecDirective + template)
+        resourceLoader.registerMockResource('/_sometemplate_nocodec.gsp', template)
+        appCtx.groovyPagesTemplateEngine.groovyPageLocator.addResourceLoader(resourceLoader)
+        
+        assertOutputEquals(expected, '<g:render template="/sometemplate" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, template + '<g:render template="/sometemplate" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, htmlCodecDirective + template + '<g:render template="/sometemplate" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, '<g:render template="/sometemplate" model="[book:book]" />' + template, [book:b])
+        assertOutputEquals(expected + expected, htmlCodecDirective + '<g:render template="/sometemplate" model="[book:book]" />' + template, [book:b])
+
+        assertOutputEquals(expected, '<g:render template="/sometemplate_nocodec" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, template + '<g:render template="/sometemplate_nocodec" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, htmlCodecDirective + template + '<g:render template="/sometemplate_nocodec" model="[book:book]" />', [book:b])
+        assertOutputEquals(expected + expected, '<g:render template="/sometemplate_nocodec" model="[book:book]" />' + template, [book:b])
+        assertOutputEquals(expected + expected, htmlCodecDirective + '<g:render template="/sometemplate_nocodec" model="[book:book]" />' + template, [book:b])
     }
-
+    
     void testFieldValueTag() {
         def b = ga.getDomainClass("ValidationTagLibBook").newInstance()
         b.properties = [publisherURL:"a_bad_url"]
diff --git a/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/pages/GroovyPage.java b/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/pages/GroovyPage.java
@@ -381,6 +381,7 @@ public final void invokeTag(String tagName, String tagNamespace, int lineNumber,
                         if (!(attrs instanceof GroovyPageAttributes)) {
                             attrs = new GroovyPageAttributes(attrs);
                         }
+                        ((GroovyPageAttributes)attrs).setGspTagSyntaxCall(true);                        
                         switch (tag.getParameterTypes().length) {
                             case 1:
                                 tagresult = tag.call(new Object[]{attrs});
@@ -473,8 +474,9 @@ public final static Object captureTagOutput(TagLibraryLookup gspTagLibraryLookup
                                                 String tagName, Map attrs, Object body, GrailsWebRequest webRequest) {
 
         if (!(attrs instanceof GroovyPageAttributes)) {
-            attrs = new GroovyPageAttributes(attrs);
+            attrs = new GroovyPageAttributes(attrs, false);
         }
+        ((GroovyPageAttributes)attrs).setGspTagSyntaxCall(false);        
         GroovyObject tagLib = lookupCachedTagLib(gspTagLibraryLookup, namespace, tagName);
 
         if (tagLib == null) {
diff --git a/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/pages/GroovyPageWritable.java b/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/pages/GroovyPageWritable.java
@@ -157,15 +157,21 @@ public Writer writeTo(Writer out) throws IOException {
             }
 
             GroovyPageBinding binding = createBinding(parentBinding);
+            String previousGspCode = null;
             if (hasRequest) {
                 request.setAttribute(GrailsApplicationAttributes.PAGE_SCOPE, binding);
+                previousGspCode = (String)request.getAttribute(GrailsApplicationAttributes.GSP_CODEC);                
             }
+            
             if (metaInfo.getCodecClass() != null) {
                 if (hasRequest) {
-                    request.setAttribute("org.codehaus.groovy.grails.GSP_CODEC", metaInfo.getCodecName());
+                    request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, metaInfo.getCodecName());
                 }
                 binding.setVariableDirectly(GroovyPage.CODEC_VARNAME, metaInfo.getCodecClass());
             } else {
+                if (hasRequest) {
+                    request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, null);
+                }
                 binding.setVariableDirectly(GroovyPage.CODEC_VARNAME, gspNoneCodeInstance);
             }
             binding.setVariableDirectly(GroovyPage.RESPONSE, response);
@@ -213,6 +219,7 @@ public Writer writeTo(Writer out) throws IOException {
                     } else  {
                         request.setAttribute(GrailsApplicationAttributes.PAGE_SCOPE, parentBinding);
                     }
+                    request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, previousGspCode);
                 }
             }
             if (debugTemplates) {
diff --git a/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/servlet/GrailsApplicationAttributes.java b/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/servlet/GrailsApplicationAttributes.java
@@ -51,6 +51,7 @@ public interface GrailsApplicationAttributes extends ApplicationAttributes {
     String TAG_CACHE = "org.codehaus.groovy.grails.TAG_CACHE";
     String ID_PARAM = "id";
     String GSP_TO_RENDER = "org.codehaus.groovy.grails.GSP_TO_RENDER";
+    String GSP_CODEC = "org.codehaus.groovy.grails.GSP_CODEC";
     String WEB_REQUEST = "org.codehaus.groovy.grails.WEB_REQUEST";
     String PAGE_SCOPE = "org.codehaus.groovy.grails.PAGE_SCOPE";
     String GSP_TMP_WRITER = "org.codehaus.groovy.grails.GSP_TMP_WRITER";
diff --git a/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/taglib/GroovyPageAttributes.java b/grails-web/src/main/groovy/org/codehaus/groovy/grails/web/taglib/GroovyPageAttributes.java
@@ -30,14 +30,29 @@
  */
 @SuppressWarnings({ "unchecked", "rawtypes" })
 public class GroovyPageAttributes extends TypeConvertingMap {
+    boolean gspTagSyntaxCall=true;
+    
     public GroovyPageAttributes() {
         super();
     }
-
+    
     public GroovyPageAttributes(Map map) {
-        super(map);
+        this(map, true);
     }
 
+    public GroovyPageAttributes(Map map, boolean gspTagSyntaxCall) {
+        super(map);
+        this.gspTagSyntaxCall=gspTagSyntaxCall;
+    }
+    
+    public boolean isGspTagSyntaxCall() {
+        return gspTagSyntaxCall;
+    }
+    
+    public void setGspTagSyntaxCall(boolean gspTagSyntaxCall) {
+        this.gspTagSyntaxCall=gspTagSyntaxCall;
+    }
+    
     @Override
     public Object clone() {
         return new GroovyPageAttributes(new LinkedHashMap(wrappedMap));

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ import java.text.DecimalFormat`
`23`	`23`	`import java.text.DecimalFormatSymbols`
`24`	`24`	`import org.apache.commons.lang.StringEscapeUtils`
`25`	`25`	`import org.codehaus.groovy.grails.plugins.codecs.HTMLCodec`
	`26`	`+import org.codehaus.groovy.grails.web.taglib.GroovyPageAttributes;`
`26`	`27`	`import org.springframework.beans.PropertyEditorRegistry`
`27`	`28`	`import org.springframework.context.MessageSourceResolvable`
`28`	`29`	`import org.springframework.context.NoSuchMessageException`
`@@ -76,6 +77,9 @@ class ValidationTagLib {`
`76`	`77`	`Closure fieldValue = { attrs, body ->`
`77`	`78`	`def bean = attrs.bean`
`78`	`79`	`def field = attrs.field?.toString()`
	`80`	`+`
	`81`	`+ def tagSyntaxCall = (attrs instanceof GroovyPageAttributes) ? attrs.isGspTagSyntaxCall() : false`
	`82`	`+`
`79`	`83`	`if (bean && field) {`
`80`	`84`	`if (bean.metaClass.hasProperty(bean,'errors')) {`
`81`	`85`	`Errors errors = bean.errors`
`@@ -87,7 +91,7 @@ class ValidationTagLib {`
`87`	`91`	`}`
`88`	`92`	`}`
`89`	`93`	`if (rejectedValue != null) {`
`90`		`- out << formatValue(rejectedValue, field)`
	`94`	`+ out << formatValue(rejectedValue, field, tagSyntaxCall)`
`91`	`95`	`}`
`92`	`96`	`}`
`93`	`97`	`else {`
`@@ -96,7 +100,7 @@ class ValidationTagLib {`
`96`	`100`	`rejectedValue = rejectedValue?."$fieldPart"`
`97`	`101`	`}`
`98`	`102`	`if (rejectedValue != null) {`
`99`		`- out << formatValue(rejectedValue, field)`
	`103`	`+ out << formatValue(rejectedValue, field, tagSyntaxCall)`
`100`	`104`	`}`
`101`	`105`	`}`
`102`	`106`	`}`
`@@ -422,12 +426,12 @@ class ValidationTagLib {`
`422`	`426`	`* formatted according to the current user's locale during the`
`423`	`427`	`* conversion to a string.`
`424`	`428`	`*/`
`425`		`- def formatValue(value, String propertyPath = null) {`
	`429`	`+ def formatValue(value, String propertyPath = null, Boolean tagSyntaxCall = false) {`
`426`	`430`	`PropertyEditorRegistry registry = RequestContextHolder.currentRequestAttributes().getPropertyEditorRegistry()`
`427`	`431`	`PropertyEditor editor = registry.findCustomEditor(value.getClass(), propertyPath)`
`428`	`432`	`if (editor) {`
`429`	`433`	`editor.setValue(value)`
`430`		`- return HTMLCodec.shouldEncode() && !(value instanceof Number) ? editor.asText?.encodeAsHTML() : editor.asText`
	`434`	`+ return (tagSyntaxCall \|\| HTMLCodec.shouldEncode()) && !(value instanceof Number) ? editor.asText?.encodeAsHTML() : editor.asText`
`431`	`435`	`}`
`432`	`436`
`433`	`437`	`if (value instanceof Number) {`
`@@ -445,6 +449,6 @@ class ValidationTagLib {`
`445`	`449`	`value = message(message: value)`
`446`	`450`	`}`
`447`	`451`
`448`		`- return HTMLCodec.shouldEncode() ? value.toString().encodeAsHTML() : value`
	`452`	`+ return (tagSyntaxCall \|\| HTMLCodec.shouldEncode()) ? value.toString().encodeAsHTML() : value`
`449`	`453`	`}`
`450`	`454`	`}`
Original file line number	Diff line number	Diff line change
`@@ -157,15 +157,21 @@ public Writer writeTo(Writer out) throws IOException {`
`157`	`157`	`}`
`158`	`158`
`159`	`159`	`GroovyPageBinding binding = createBinding(parentBinding);`
	`160`	`+ String previousGspCode = null;`
`160`	`161`	`if (hasRequest) {`
`161`	`162`	`request.setAttribute(GrailsApplicationAttributes.PAGE_SCOPE, binding);`
	`163`	`+ previousGspCode = (String)request.getAttribute(GrailsApplicationAttributes.GSP_CODEC);`
`162`	`164`	`}`
	`165`	`+`
`163`	`166`	`if (metaInfo.getCodecClass() != null) {`
`164`	`167`	`if (hasRequest) {`
`165`		`- request.setAttribute("org.codehaus.groovy.grails.GSP_CODEC", metaInfo.getCodecName());`
	`168`	`+ request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, metaInfo.getCodecName());`
`166`	`169`	`}`
`167`	`170`	`binding.setVariableDirectly(GroovyPage.CODEC_VARNAME, metaInfo.getCodecClass());`
`168`	`171`	`} else {`
	`172`	`+ if (hasRequest) {`
	`173`	`+ request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, null);`
	`174`	`+ }`
`169`	`175`	`binding.setVariableDirectly(GroovyPage.CODEC_VARNAME, gspNoneCodeInstance);`
`170`	`176`	`}`
`171`	`177`	`binding.setVariableDirectly(GroovyPage.RESPONSE, response);`
`@@ -213,6 +219,7 @@ public Writer writeTo(Writer out) throws IOException {`
`213`	`219`	`} else {`
`214`	`220`	`request.setAttribute(GrailsApplicationAttributes.PAGE_SCOPE, parentBinding);`
`215`	`221`	`}`
	`222`	`+ request.setAttribute(GrailsApplicationAttributes.GSP_CODEC, previousGspCode);`
`216`	`223`	`}`
`217`	`224`	`}`
`218`	`225`	`if (debugTemplates) {`