Fix GRAILS-10763 problem with comments inside UrlMappings configuration

grails-plugin-url-mappings/src/main/groovy/org/codehaus/groovy/grails/plugins/web/mapping/UrlMappingsGrailsPlugin.groovy

@@ -83,6 +83,56 @@ class UrlMappingsGrailsPlugin {
         }
     }
 
+    // These regex match the string and regex literals inside groovy to save before remove comments
+    static final GROOVY_STRING_SINGLE_QUOTE_REGEX = /'([^\n']|\\')*[^\\]'/
+    static final GROOVY_STRING_DOUBLE_QUOTE_REGEX = /"([^\n"]|\\")*[^\\]"/
+    static final GROOVY_STRING_TRIPLE_QUOTE_REGEX = /'''([^']|[^']'[^']|[^']''[^'])*'''/
+    static final GROOVY_REGEX_LITERALS_REGEX = /\/([^\n\/]|\\\/)*[^\\]\//
+
+    // These regex match the multiline and single line comments
+    static final GROOVY_MULTILINE_COMMENTS = /\/\*([^*]|(\*+[^*\/]))*\*\//
+    static final GROOVY_SINGLE_LINE_COMMENTS = /\/\/.*/
+
+    /**
+     * Method to remove the comments from the URL Mappings config file
+     */
+    def removeCommentsFromGroovy(content) {
+        if (!content) {
+            return content
+        }
+        def result = content
+
+        // Save the string to restore them afterwards, to avoid the edge case of comment characters
+        // inside strings. ie: "Test /* */"
+        def savedString = [:]
+        def uuidReplacerString = { regexMatch->
+            def key = "%%${UUID.randomUUID() as String}%%"
+            def matchedString = regexMatch[0]
+            if (matchedString) {
+                // Escape $ characters because they break the replace afterwards
+                matchedString = matchedString.replaceAll(/\$/, '\\\\\\$')
+            }
+            savedString[key] = matchedString
+            return key
+        }
+
+        // These regexp match groovy strings
+        result = result.replaceAll(GROOVY_STRING_SINGLE_QUOTE_REGEX, uuidReplacerString)
+        result = result.replaceAll(GROOVY_STRING_DOUBLE_QUOTE_REGEX, uuidReplacerString)
+        result = result.replaceAll(GROOVY_STRING_TRIPLE_QUOTE_REGEX, uuidReplacerString)
+        result = result.replaceAll(GROOVY_REGEX_LITERALS_REGEX, uuidReplacerString)
+
+        // Remove all comments from the file
+        result = result.replaceAll(GROOVY_MULTILINE_COMMENTS,"")
+        result = result.replaceAll(GROOVY_SINGLE_LINE_COMMENTS, "")
+
+        // Restore the saved strings
+        savedString.each {key, val->
+            result = result.replaceAll(key, val)
+        }
+        return result
+    }
+
     def doWithWebDescriptor = { webXml ->
         def filters = webXml.filter
         def lastFilter = filters[filters.size()-1]
@@ -117,7 +167,8 @@ class UrlMappingsGrailsPlugin {
         def appliedErrorCodes = []
         def errorPages = {
             for (Resource r in watchedResources) {
-                r.file.eachLine { line ->
+                def contents = removeCommentsFromGroovy(r.file.text)
+                contents.eachLine { line ->
                     def matcher = line =~ /\s*["'](\d+?)["']\s*\(.+?\)/
                     if (matcher) {
                         def errorCode = matcher[0][1]

grails-test-suite-uber/src/test/groovy/org/codehaus/groovy/grails/plugins/web/mapping/UrlMappingsGrailsPluginTests.groovy

@@ -46,7 +46,7 @@ class UrlMappingsGrailsPluginTests extends GroovyTestCase {
 
     void testNoDuplicateErrorCodes() {
         FileSystemResource.metaClass.getFile = {->
-            [ eachLine: { Closure c -> c.call('"404"(controller:"foo")') }]
+            [ text: '"404"(controller:"foo")' ]
         }
 
         UrlMappingsGrailsPlugin.metaClass.getWatchedResources ={->
@@ -67,7 +67,7 @@ class UrlMappingsGrailsPluginTests extends GroovyTestCase {
 
     void testErrorPageWebXmlPositioning() {
         FileSystemResource.metaClass.getFile = {->
-            [ eachLine: { Closure c -> c.call('"404"(controller:"foo")') }]
+            [ text: '"404"(controller:"foo")' ]
         }
 
         UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
@@ -84,6 +84,199 @@ class UrlMappingsGrailsPluginTests extends GroovyTestCase {
         assertEquals '<web-app><filter><filter-name>sitemesh</filter-name><filter-class>org.codehaus.groovy.grails.web.sitemesh.GrailsPageFilter</filter-class></filter><filter><filter-name>urlMapping</filter-name><filter-class>org.codehaus.groovy.grails.web.mapping.filter.UrlMappingsFilter</filter-class></filter><filter-mapping><filter-name>sitemesh</filter-name><url-pattern>/*</url-pattern></filter-mapping><filter-mapping><filter-name>urlMapping</filter-name><url-pattern>/*</url-pattern><dispatcher>FORWARD</dispatcher><dispatcher>REQUEST</dispatcher></filter-mapping><listener><listener-class>org.springframework.web.util.Log4jConfigListener</listener-class></listener><listener><listener-class>org.codehaus.groovy.grails.web.context.GrailsContextLoaderListener</listener-class></listener><servlet><servlet-name>grails</servlet-name><servlet-class>org.codehaus.groovy.grails.web.servlet.GrailsDispatcherServlet</servlet-class><load-on-startup>1</load-on-startup></servlet><servlet><servlet-name>grails-errorhandler</servlet-name><servlet-class>org.codehaus.groovy.grails.web.servlet.ErrorHandlingServlet</servlet-class></servlet><servlet-mapping><servlet-name>grails</servlet-name><url-pattern>*.dispatch</url-pattern></servlet-mapping><servlet-mapping><servlet-name>grails-errorhandler</servlet-name><url-pattern>/grails-errorhandler</url-pattern></servlet-mapping><welcome-file-list><welcome-file>index.html</welcome-file><welcome-file>index.jsp</welcome-file><welcome-file>index.gsp</welcome-file></welcome-file-list><error-page><error-code>404</error-code><location>/grails-errorhandler</location></error-page></web-app>', sw.toString()
     }
 
+    void testCommentsOnUrlMappingsBeginningOfLine() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text: '// "404"(controller:"foo")' ]
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertFalse content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+    void testCommentsOnUrlMappingsMultiLineComment() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text: '''\
+                /*
+                "404"(controller: "foo")
+                */
+              ''']
+            [ text: '/* \n "404"(controller:"foo") \n */' ]
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertFalse content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+    void testCommentsOnUrlMappingsMultiLineCommentBeforeGoodMapping() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text: '/* "404"(controller:"foo") */ "400"(controller:"foo")' ]
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertFalse content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>400</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+    void testCommentCharactersInsideStrings() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text: '''\
+                "/*" (controller: "foo")
+                "\\\\"bar" (controller: "bar")
+                "400" (controller: "error")
+                '\\\\'bar' (controller: "bar")
+                "404" (controller: "error")
+                "*/" (controller: "foo")
+            ''']
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>400</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+    void testCommentCharactersInsideMultilineStrings() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text:
+                "'''\\\n" +
+                "Test /*\n" +
+                "'''\n"+
+                "\"400\" (controller: \"error\")\n" +
+                "/*Test*/\n"+
+                "\"404\" (controller: \"error\")\n"
+            ]
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>400</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+    void testCommentCharactersInsideRegularExpressions() {
+        FileSystemResource.metaClass.getFile = {->
+            [ text: '''\
+                /\\/*/ (controller: "foo")
+                "404" (controller: "error")
+                /* Comment */
+                "foo" (controller: "foo")
+            ''']
+        }
+
+        UrlMappingsGrailsPlugin.metaClass.getWatchedResources = { ->
+            new FileSystemResource("/dummy/path")
+        }
+        def plugin = new UrlMappingsGrailsPlugin()
+        def xml = new XmlSlurper().parseText(text)
+
+        plugin.doWithWebDescriptor(xml)
+
+        def sw = new StringWriter()
+        sw = new StreamingMarkupBuilder().bind{ out << xml }
+
+        def content = sw.toString()
+
+        assertTrue content.contains(
+            "<error-page>" +
+                "<error-code>404</error-code>" +
+                "<location>/grails-errorhandler</location>" +
+            "</error-page>")
+    }
+
+
     protected void tearDown() {
         GroovySystem.metaClassRegistry.removeMetaClass(FileSystemResource)
         GroovySystem.metaClassRegistry.removeMetaClass(UrlMappingsGrailsPlugin)