CouchDB
  1. CouchDB
  2. COUCHDB-2031

Paths ending in '/' rewrite query string params incorrectly

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: HTTP Interface
    • Labels:
      None

      Description

      Problem is discussed in this SO thread.

      To summarize, with

      rewrites.js
      [ { "from": "/dbname/*", "to: ../../*" },
        { "from": "/*", "to: *" } ]
      

      /dbname/?foo=bar doesn't work; it rewrites to /dbname/_design/..?foo=bar instead.

        Activity

        Hide
        Calvin Metcalf added a comment -

        which affects pouchdb's attempts to cache bust in IE (See this issue)

        Show
        Calvin Metcalf added a comment - which affects pouchdb's attempts to cache bust in IE (See this issue )
        Hide
        ASF subversion and git services added a comment -

        Commit 147adec8ef3763cb1821f411f393d6560aaccad2 in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=147adec ]

        Move addition of qs params after normalization

        This refactor executes normalize_path/1 before appending the bound query
        string parameters.

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 147adec8ef3763cb1821f411f393d6560aaccad2 in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=147adec ] Move addition of qs params after normalization This refactor executes normalize_path/1 before appending the bound query string parameters. COUCHDB-2031
        Hide
        Adam Kocoloski added a comment - - edited

        The rewriter is not really my area of expertise but it looks like the issue is that the normalization code partitions paths based on "/" and then looks specifically for ".." when removing path parts, but we feed it an input that already has the bound qs params tacked onto the end. I refactored a little bit to normalize the path before appending the query string. Existing tests pass, I haven't added one specifically for this bug yet.

        Show
        Adam Kocoloski added a comment - - edited The rewriter is not really my area of expertise but it looks like the issue is that the normalization code partitions paths based on "/" and then looks specifically for ".." when removing path parts, but we feed it an input that already has the bound qs params tacked onto the end. I refactored a little bit to normalize the path before appending the query string. Existing tests pass, I haven't added one specifically for this bug yet.
        Hide
        Eric Drechsel added a comment - - edited

        I set up a dev env to test, and Adam Kocoloski's branch seems to fix the issue. Can I buy you a beer?

        I would take a crack at writing a test case for the bug, but erlang looks like moon runes to me.

        Show
        Eric Drechsel added a comment - - edited I set up a dev env to test, and Adam Kocoloski 's branch seems to fix the issue. Can I buy you a beer? I would take a crack at writing a test case for the bug, but erlang looks like moon runes to me.
        Hide
        Adam Kocoloski added a comment -

        Ah, the test in this case would probably be in JavaScript in the rewrites.js file.

        Show
        Adam Kocoloski added a comment - Ah, the test in this case would probably be in JavaScript in the rewrites.js file.
        Hide
        ASF subversion and git services added a comment -

        Commit 0041c06f2772e6f86c187260d304adca7277bddd in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=0041c06 ]

        Add test for path normalization with qs params

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 0041c06f2772e6f86c187260d304adca7277bddd in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=0041c06 ] Add test for path normalization with qs params COUCHDB-2031
        Hide
        Adam Kocoloski added a comment -

        So I added a basic test, but interestingly enough it fails when the DB name contains an encoded "/" (the rewriter is unable to bind the path). I can modify the rewrite rule in the test to look for the unencoded DB name and make a request like

        GET /test_suite_db%2Fwith_slashes/_design/test/_rewrite/test_suite_db/with_slashes/foo

        which works, but that seems a bit incongruous. Looking for feedback here.

        Show
        Adam Kocoloski added a comment - So I added a basic test, but interestingly enough it fails when the DB name contains an encoded "/" (the rewriter is unable to bind the path). I can modify the rewrite rule in the test to look for the unencoded DB name and make a request like GET /test_suite_db%2Fwith_slashes/_design/test/_rewrite/test_suite_db/with_slashes/foo which works, but that seems a bit incongruous. Looking for feedback here.
        Hide
        ASF subversion and git services added a comment -

        Commit cb5e7f6a423865d45b134495888d10aa2e8ac474 in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=cb5e7f6 ]

        Add test for path normalization with qs params

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit cb5e7f6a423865d45b134495888d10aa2e8ac474 in branch refs/heads/2031-fix-qs-rewrite from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=cb5e7f6 ] Add test for path normalization with qs params COUCHDB-2031
        Hide
        Adam Kocoloski added a comment -

        Meh, that's beside the point of this ticket. Updating the test to just use "/db/" as the match that provides the raw DB API rather than using the actual DB name.

        Show
        Adam Kocoloski added a comment - Meh, that's beside the point of this ticket. Updating the test to just use "/db/" as the match that provides the raw DB API rather than using the actual DB name.
        Hide
        Adam Kocoloski added a comment -

        Benoit Chesneau any interest in reading through this patch before I merge?

        Show
        Adam Kocoloski added a comment - Benoit Chesneau any interest in reading through this patch before I merge?
        Hide
        Benoit Chesneau added a comment -

        Adam Kocoloski I will have a look on the branch today

        Show
        Benoit Chesneau added a comment - Adam Kocoloski I will have a look on the branch today
        Hide
        Adam Kocoloski added a comment -

        Heya Benoit Chesneau did you have a chance to take a look?

        Show
        Adam Kocoloski added a comment - Heya Benoit Chesneau did you have a chance to take a look?
        Hide
        Benoit Chesneau added a comment -

        Adam Kocoloski sorry for the delay. The patch looks OK for me. Also tests pass. +1 for me

        Show
        Benoit Chesneau added a comment - Adam Kocoloski sorry for the delay. The patch looks OK for me. Also tests pass. +1 for me
        Hide
        ASF subversion and git services added a comment -

        Commit 147adec8ef3763cb1821f411f393d6560aaccad2 in branch refs/heads/master from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=147adec ]

        Move addition of qs params after normalization

        This refactor executes normalize_path/1 before appending the bound query
        string parameters.

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 147adec8ef3763cb1821f411f393d6560aaccad2 in branch refs/heads/master from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=147adec ] Move addition of qs params after normalization This refactor executes normalize_path/1 before appending the bound query string parameters. COUCHDB-2031
        Hide
        ASF subversion and git services added a comment -

        Commit cb5e7f6a423865d45b134495888d10aa2e8ac474 in branch refs/heads/master from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=cb5e7f6 ]

        Add test for path normalization with qs params

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit cb5e7f6a423865d45b134495888d10aa2e8ac474 in branch refs/heads/master from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=cb5e7f6 ] Add test for path normalization with qs params COUCHDB-2031
        Hide
        ASF subversion and git services added a comment -

        Commit 37c8459693dbf55cd4683c7e288dcc9ca8899e97 in branch refs/heads/master from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=37c8459 ]

        Merge branch '2031-fix-qs-rewrite'

        Closes COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 37c8459693dbf55cd4683c7e288dcc9ca8899e97 in branch refs/heads/master from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=37c8459 ] Merge branch '2031-fix-qs-rewrite' Closes COUCHDB-2031
        Hide
        Adam Kocoloski added a comment -

        No worries, thanks for the review.

        Show
        Adam Kocoloski added a comment - No worries, thanks for the review.
        Hide
        ASF subversion and git services added a comment -

        Commit 37c8459693dbf55cd4683c7e288dcc9ca8899e97 in branch refs/heads/1953-fix-mime-multipart-parsing from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=37c8459 ]

        Merge branch '2031-fix-qs-rewrite'

        Closes COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 37c8459693dbf55cd4683c7e288dcc9ca8899e97 in branch refs/heads/1953-fix-mime-multipart-parsing from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=37c8459 ] Merge branch '2031-fix-qs-rewrite' Closes COUCHDB-2031
        Hide
        ASF subversion and git services added a comment -

        Commit a608b83fffeda84be7d24814c6c80439362083d7 in branch refs/heads/1994-merge-rcouch from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=a608b83 ]

        Move addition of qs params after normalization

        This refactor executes normalize_path/1 before appending the bound query
        string parameters.

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit a608b83fffeda84be7d24814c6c80439362083d7 in branch refs/heads/1994-merge-rcouch from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=a608b83 ] Move addition of qs params after normalization This refactor executes normalize_path/1 before appending the bound query string parameters. COUCHDB-2031
        Hide
        ASF subversion and git services added a comment -

        Commit 705d4332e2c2ed001d1ae58b8ef5bfd4ead44489 in branch refs/heads/1994-merge-rcouch from Adam Kocoloski
        [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=705d433 ]

        Add test for path normalization with qs params

        COUCHDB-2031

        Show
        ASF subversion and git services added a comment - Commit 705d4332e2c2ed001d1ae58b8ef5bfd4ead44489 in branch refs/heads/1994-merge-rcouch from Adam Kocoloski [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=705d433 ] Add test for path normalization with qs params COUCHDB-2031

          People

          • Assignee:
            Unassigned
            Reporter:
            Eric Drechsel
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development