CouchDB
  1. CouchDB
  2. COUCHDB-317

Badmatch error if _replicate target starts with slash

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 0.9.1
    • Fix Version/s: 1.3
    • Component/s: Replication
    • Labels:
      None
    • Environment:

      svn r761188

    • Skill Level:
      New Contributors Level (Easy)

      Description

      Giving a replication source or target which starts with a slash gives an Erlang barf error. (The wiki at http://wiki.apache.org/couchdb/Replication said that this slash was necessary. I will fix the wiki, but the error is still unsightly)

      $ curl -X POST -d '

      {"source":"/foo","target":"/bar"}

      ' http://127.0.0.1:5984/_replicate
      {"error":"case_clause","reason":"{error,{badmatch,{error,illegal_database_name,\n [

      {couch_rep,init,1}

      ,\n

      {gen_server,init_it,6}

      ,\n

      {proc_lib,init_p,5}

      ]},\n {child,undefined,\"7898c2bfbbcf8adbb305371e6c3c949e\",\n {gen_server,start_link,\n [couch_rep,\n [\"7898c2bfbbcf8adbb305371e6c3c949e\",\n {local,<<\"/foo\">>,{user_ctx,null,[<<\"_admin\">>]}},\n {local,<<\"/bar\">>,\n {user_ctx,null,[<<\"_admin\">>]}}],\n []]},\n transient,1,worker,\n [couch_rep]}}}"}

      1. replicate-slash-fix.patch
        0.5 kB
        Joan Touzet
      2. replicate-slash-fix-v2.patch
        1 kB
        Joan Touzet

        Activity

        Hide
        Jan Lehnardt added a comment -

        merged in 7fa217b and 5d92ae8. Thanks!

        Show
        Jan Lehnardt added a comment - merged in 7fa217b and 5d92ae8. Thanks!
        Hide
        Robert Newson added a comment -

        +1 to merge to 1.3.x (also, yay test!)

        Show
        Robert Newson added a comment - +1 to merge to 1.3.x (also, yay test!)
        Show
        Jan Lehnardt added a comment - branch: https://git-wip-us.apache.org/repos/asf?p=couchdb.git;a=shortlog;h=refs/heads/317-fix-replicator-slashes-in-db-names for master & 1.3.x @rnewson: SYN merge to 1.3.x
        Hide
        Jan Lehnardt added a comment -

        I buy the "the user’s app then thinks it’s a valid name and other stuff breaks" argument

        +1 on the patch then.

        Show
        Jan Lehnardt added a comment - I buy the "the user’s app then thinks it’s a valid name and other stuff breaks" argument +1 on the patch then.
        Hide
        Joan Touzet added a comment -

        I disagree, that changes semantics. Don't try and read the user's mind...I understand trying to be friendly, but that means absolutely everywhere where a DbName is not found, we could try and number of arbitrary transformations (What about more than one leading slash? Or an escaped leading slash?) to eventually get to something reasonable.

        But if we accept it, then it's possible the user thinks that ///foobar is a valid DbName and uses it in a bunch of other places, potentially in application logic as well. That allows inaccuracy and problems to bleed through to lots of other areas.

        Show
        Joan Touzet added a comment - I disagree, that changes semantics. Don't try and read the user's mind...I understand trying to be friendly, but that means absolutely everywhere where a DbName is not found, we could try and number of arbitrary transformations (What about more than one leading slash? Or an escaped leading slash?) to eventually get to something reasonable. But if we accept it, then it's possible the user thinks that ///foobar is a valid DbName and uses it in a bunch of other places, potentially in application logic as well. That allows inaccuracy and problems to bleed through to lots of other areas.
        Hide
        Jan Lehnardt added a comment -

        yay bikeshed, I’d suggest we strip the the first character if it is a slash and let things succeed.

        I haven’t thought a whole lot about it, so let us know if you can poke a hole in the logic.

        Show
        Jan Lehnardt added a comment - yay bikeshed, I’d suggest we strip the the first character if it is a slash and let things succeed. I haven’t thought a whole lot about it, so let us know if you can poke a hole in the logic.
        Hide
        Joan Touzet added a comment -

        New version of this patch includes a JavaScript test to validate the new behaviour. Yay testing!

        Show
        Joan Touzet added a comment - New version of this patch includes a JavaScript test to validate the new behaviour. Yay testing!
        Hide
        Joan Touzet added a comment -

        Here's a working fix that, for the original poster's curl, will return:

        {"error":"db_not_found","reason":"could not open /foo"}
        Show
        Joan Touzet added a comment - Here's a working fix that, for the original poster's curl, will return: {"error":"db_not_found","reason":"could not open /foo"}
        Hide
        Jan Lehnardt added a comment -

        Bump to 1.3.x.

        Show
        Jan Lehnardt added a comment - Bump to 1.3.x.
        Hide
        Adam Kocoloski added a comment -

        Confirmed that this is still a problem in 0.10.0. I kinda think those paths ought to be valid, but either way we should certainly catch the illegal_database_name error.

        Show
        Adam Kocoloski added a comment - Confirmed that this is still a problem in 0.10.0. I kinda think those paths ought to be valid, but either way we should certainly catch the illegal_database_name error.

          People

          • Assignee:
            Joan Touzet
            Reporter:
            Brian Candler
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development