Uploaded image for project: 'Wicket'
  1. Wicket
  2. WICKET-1806

JavascriptStripper ignores context when looking for multiline comments

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.4
    • Fix Version/s: 1.3.5
    • Component/s: wicket
    • Labels:
      None

      Description

      org.apache.wicket.util.string.JavascriptStripper#stripCommentsAndWhitespace doesn't check the context when looking for multiline comments. In my case, it interprets the default accept header in the jQuery source code as a multiline comment:

      accepts: {
      xml: "application/xml, text/xml",
      html: "text/html",
      script: "text/javascript, application/javascript",
      json: "application/json, text/javascript",
      text: "text/plain",
      _default: "/"
      }

      The culprit being this string: "/"

      I don't have a patch and I wouldn't try to write my own JS parser. Maybe replacing the current implementation with a Rhino-based one is an option. Its available via Maven:

      <dependency>
      <groupId>rhino</groupId>
      <artifactId>js</artifactId>
      <version>1.7R1</version>
      </dependency>

        Activity

        Hide
        knopp Matej Knopp added a comment -

        Fixed, committed for 1.3 branch and trunk.

        Show
        knopp Matej Knopp added a comment - Fixed, committed for 1.3 branch and trunk.
        Show
        joern.zaefferer Jörn Zaefferer added a comment - Here is the file: http://code.google.com/p/jqueryjs/downloads/detail?name=jquery-1.2.6.js
        Hide
        joern.zaefferer Jörn Zaefferer added a comment -

        I tested it with jQuery 1.2.6, which contains the offending line. The stripping didn't fail (no exception), but the resulting code got messed up.

        Show
        joern.zaefferer Jörn Zaefferer added a comment - I tested it with jQuery 1.2.6, which contains the offending line. The stripping didn't fail (no exception), but the resulting code got messed up.
        Hide
        knopp Matej Knopp added a comment -

        I added the testcase you've provided to Javascript stripper test and it strips it just fine. No problem at all. Can you attach the whole javascript that fails to get stripped successfully?

        Show
        knopp Matej Knopp added a comment - I added the testcase you've provided to Javascript stripper test and it strips it just fine. No problem at all. Can you attach the whole javascript that fails to get stripped successfully?

          People

          • Assignee:
            knopp Matej Knopp
            Reporter:
            joern.zaefferer Jörn Zaefferer
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development