Wicket
  1. Wicket
  2. WICKET-1806

JavascriptStripper ignores context when looking for multiline comments

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major 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
        Matej Knopp added a comment -

        Fixed, committed for 1.3 branch and trunk.

        Show
        Matej Knopp added a comment - Fixed, committed for 1.3 branch and trunk.
        Show
        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
        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
        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
        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
        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:
            Matej Knopp
            Reporter:
            Jörn Zaefferer
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development