Jetty
  1. Jetty
  2. JETTY-88

Path mapping will match on a partial substring

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 6.0.0RC0
    • Fix Version/s: 6.0.0rc1
    • Component/s: Servlet
    • Labels:
      None
    • Number of attachments :
      0

      Description

      PathMap.match("/foo/*", "foodies") returns true, when it should be false.

      (Reported by Mark Hobson on jetty-support)

        Activity

        Hide
        Tim Vernum added a comment -

        Fixed in SVN r670

        Show
        Tim Vernum added a comment - Fixed in SVN r670
        Hide
        Greg Wilkins added a comment -

        tim,

        thanks for this fix, but a small problem...the isPathWildcardMatch can
        create upto 3 new strings when it should not need to create any.

        As a often called method it would be good to optimize this (but without
        adding the bugs that I obviously had

        I would use a region match and then check the character after
        the region to see if it is / ; # ? etc.

        assign the bug to me if you want me to do this.

        cheers

        Show
        Greg Wilkins added a comment - tim, thanks for this fix, but a small problem...the isPathWildcardMatch can create upto 3 new strings when it should not need to create any. As a often called method it would be good to optimize this (but without adding the bugs that I obviously had I would use a region match and then check the character after the region to see if it is / ; # ? etc. assign the bug to me if you want me to do this. cheers
        Hide
        Greg Wilkins added a comment -

        while pondering if # and ? support was needed, i ended up fixing this.
        I don't think # and ? are needed, but added support for them anyway in case paths from other sources are passed in....

        But now I am thinking that is wrong.... because this is a decoded path and
        /foo%3F should not match /foo/*

        I will back that bit out....

        Show
        Greg Wilkins added a comment - while pondering if # and ? support was needed, i ended up fixing this. I don't think # and ? are needed, but added support for them anyway in case paths from other sources are passed in.... But now I am thinking that is wrong.... because this is a decoded path and /foo%3F should not match /foo/* I will back that bit out....
        Hide
        Greg Wilkins added a comment -

        OK - this is still not right

        /foo%3B should not match /foo/*

        so the problem is not in pathMap - the ;arg should not be offerred to
        the PathMap match UNLESS it was encoded in the original URL.

        obviously it is too late for me to be coding ... so i will leave this for now
        and think about it....

        Show
        Greg Wilkins added a comment - OK - this is still not right /foo%3B should not match /foo/* so the problem is not in pathMap - the ;arg should not be offerred to the PathMap match UNLESS it was encoded in the original URL. obviously it is too late for me to be coding ... so i will leave this for now and think about it....
        Hide
        Greg Wilkins added a comment -

        OK - the fix was to remove all support for ; and ? from PathMap. In Jetty6, these are stripped out long before pathmap is called and any special characters that are there were encoded and are part of the path
        and non special.

        So PathMap got a little cleaner as a result of this.

        Show
        Greg Wilkins added a comment - OK - the fix was to remove all support for ; and ? from PathMap. In Jetty6, these are stripped out long before pathmap is called and any special characters that are there were encoded and are part of the path and non special. So PathMap got a little cleaner as a result of this.
        Hide
        Tim Vernum added a comment -

        Closing issue.
        Appears to have been satisfactorily fixed in 6.0.0rc1

        Show
        Tim Vernum added a comment - Closing issue. Appears to have been satisfactorily fixed in 6.0.0rc1

          People

          • Assignee:
            Unassigned
            Reporter:
            Tim Vernum
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: