CouchDB
  1. CouchDB
  2. COUCHDB-1229

_update handler doesn't support slashes in doc _id

    Details

    • Skill Level:
      Dont Know

      Description

      Let's say you have:

      • a doc with _id foo/bar
      • a show function named baz
      • a update function named baz

      Then _show/baz/foo/bar is valid but _update/baz/foo/bar is not. Only _update/baz/foo%2Fbar works.

      This is particularly annoying when you want to rewrite /something/* to _update/baz/foo/* (rewriting /something/* to _show/baz/foo/* works perfectly).

      1. COUCHDB-1229.patch
        1 kB
        Christopher Bonhage

        Activity

        Hide
        Paul Joseph Davis added a comment -

        In genera, if you need to address a document with a slash in its name, you need to %2F escape the slash. There are a couple places where we relax this a bit by checking both cases. I'm not sure all the spots where we do this though.

        Bottom line is that slashes in doc ids are just weird. Unless someone figures out a decent way to make them not weird while not breaking the rest of the API I don't see much changing in this regard.

        Show
        Paul Joseph Davis added a comment - In genera, if you need to address a document with a slash in its name, you need to %2F escape the slash. There are a couple places where we relax this a bit by checking both cases. I'm not sure all the spots where we do this though. Bottom line is that slashes in doc ids are just weird. Unless someone figures out a decent way to make them not weird while not breaking the rest of the API I don't see much changing in this regard.
        Hide
        Simon Leblanc added a comment -

        Hum, too bad. I find this very useful.
        I hesitated between using : or / as a separator and I went for / since it worked nicely with the rewrite handler.
        I think either / should be allowed or we need a more powerful rewrite handler since not using slashes will make thing much more complicated for me…

        Thanks for your answer!

        Show
        Simon Leblanc added a comment - Hum, too bad. I find this very useful. I hesitated between using : or / as a separator and I went for / since it worked nicely with the rewrite handler. I think either / should be allowed or we need a more powerful rewrite handler since not using slashes will make thing much more complicated for me… Thanks for your answer!
        Hide
        Robert Newson added a comment -

        / is an important character that distinguishes database resources from document resources, etc. Allowing it in doc ids unescaped is not likely any time soon.

        Show
        Robert Newson added a comment - / is an important character that distinguishes database resources from document resources, etc. Allowing it in doc ids unescaped is not likely any time soon.
        Hide
        Christopher Bonhage added a comment -

        I encountered this as well; digging through src/couch_httpd_show.erl, handle_doc_show_req/3 appears to handle the "/" character in Document IDs properly:

        DocParts = [DocId|Rest],
        DocId1 = ?l2b(string:join([?b2l(P)|| P <- DocParts], "/")),

        It seems like a bug that the _show handler joins the path parts back together, but the _update handler does not.

        Obviously / is an important character in URL paths, but I don't see how that affects the trailing path parts here: database/_design/ddoc/_update/handler/docidpart1/docidpart2

        Am I wrong?

        Show
        Christopher Bonhage added a comment - I encountered this as well; digging through src/couch_httpd_show.erl, handle_doc_show_req/3 appears to handle the "/" character in Document IDs properly: DocParts = [DocId|Rest] , DocId1 = ?l2b(string:join( [?b2l(P)|| P <- DocParts] , "/")), It seems like a bug that the _show handler joins the path parts back together, but the _update handler does not. Obviously / is an important character in URL paths, but I don't see how that affects the trailing path parts here: database/_design/ddoc/_update/handler/docidpart1/docidpart2 Am I wrong?
        Hide
        Noah Slater added a comment -

        Just to chime in, I do not see the logic here either. Specifically in regard to: "/ is an important character that distinguishes database resources from document resources, etc." After the first "/" to separate the database from the document, there shouldn't be any need to treat "/" characters as being special. Even a simple split on "/" would let you pull the first index to get the database name, while joining the remainder to reconstruct the docid.

        Show
        Noah Slater added a comment - Just to chime in, I do not see the logic here either. Specifically in regard to: "/ is an important character that distinguishes database resources from document resources, etc." After the first "/" to separate the database from the document, there shouldn't be any need to treat "/" characters as being special. Even a simple split on "/" would let you pull the first index to get the database name, while joining the remainder to reconstruct the docid.
        Hide
        Paul Joseph Davis added a comment -

        Unfortunately, we don't just rely on "/" to distinguish between the dbname and docid. All of the doc handlers and and design handlers have come to rely on "/" as an important delimiter. Unfortunately some proxies unconditionally decode %2F to "/" before forwarding the URL so we also have some special cased logic to re-constitute things like design doc ids and what not.

        Show
        Paul Joseph Davis added a comment - Unfortunately, we don't just rely on "/" to distinguish between the dbname and docid. All of the doc handlers and and design handlers have come to rely on "/" as an important delimiter. Unfortunately some proxies unconditionally decode %2F to "/" before forwarding the URL so we also have some special cased logic to re-constitute things like design doc ids and what not.
        Hide
        Robert Newson added a comment -

        "After the first "/" to separate the database from the document, there shouldn't be any need to treat "/" characters as being special."

        Well, except for attachment urls, surely? /db_name/doc_id/attachment_name

        Show
        Robert Newson added a comment - "After the first "/" to separate the database from the document, there shouldn't be any need to treat "/" characters as being special." Well, except for attachment urls, surely? /db_name/doc_id/attachment_name
        Hide
        Marcello Nuccio added a comment -

        Would it be wise then to forbid the creation of documents with "/" in the _id?

        I thought it was supported because the designs documents have "/" in the _id. But if there are known problems, then I think the user should be promptly made aware of them.

        Show
        Marcello Nuccio added a comment - Would it be wise then to forbid the creation of documents with "/" in the _id? I thought it was supported because the designs documents have "/" in the _id. But if there are known problems, then I think the user should be promptly made aware of them.
        Hide
        Christopher Bonhage added a comment -

        Patch for the _update handler to match the behavior of the _show handler with regards to slashes in docids

        Show
        Christopher Bonhage added a comment - Patch for the _update handler to match the behavior of the _show handler with regards to slashes in docids
        Hide
        Simon Leblanc added a comment -

        Does this mean that slashes in docids will be supported?

        Show
        Simon Leblanc added a comment - Does this mean that slashes in docids will be supported?
        Hide
        Christopher Bonhage added a comment -

        I disagree with this being a "Won't Fix". I am of the opinion that the _show handler has correct behavior here (and by extension, that the _update handler's swallowing of slashes in docids is incorrect behavior). The patch supplied above just duplicates the docid extraction routine from handle_doc_show_req/3 to handle_doc_update_req/3.

        My couchapps use slashes in docids to namespace documents, allowing me to use rewrite rules to force _show, _update, and _list handlers to only handle documents in a specified namespace. If slashes are not allowed in docids, this would not be possible (and would be a massive showstopper for me).

        That being said, I do encounter some woes when it comes to trying to get attachments out of namespaced documents with rewrite rules (there's no way around escaping the %2F there), but that has nothing to do with the behavior of _show and _update handlers.

        Show
        Christopher Bonhage added a comment - I disagree with this being a "Won't Fix". I am of the opinion that the _show handler has correct behavior here (and by extension, that the _update handler's swallowing of slashes in docids is incorrect behavior). The patch supplied above just duplicates the docid extraction routine from handle_doc_show_req/3 to handle_doc_update_req/3. My couchapps use slashes in docids to namespace documents, allowing me to use rewrite rules to force _show, _update, and _list handlers to only handle documents in a specified namespace. If slashes are not allowed in docids, this would not be possible (and would be a massive showstopper for me). That being said, I do encounter some woes when it comes to trying to get attachments out of namespaced documents with rewrite rules (there's no way around escaping the %2F there), but that has nothing to do with the behavior of _show and _update handlers.
        Hide
        Marcello Nuccio added a comment -

        I have the same problem as Christopher. URL rewriting works by splitting the path at "/", so they are a natural choice in docids: you get pretty urls and namespacing for free.

        Does anyone have a better solution for this problem?

        Show
        Marcello Nuccio added a comment - I have the same problem as Christopher. URL rewriting works by splitting the path at "/", so they are a natural choice in docids: you get pretty urls and namespacing for free. Does anyone have a better solution for this problem?
        Hide
        Jan Lehnardt added a comment -

        I think I agree with Christopher et. al. here. Assigning this to myself.

        Show
        Jan Lehnardt added a comment - I think I agree with Christopher et. al. here. Assigning this to myself.
        Hide
        Jan Lehnardt added a comment -

        Added a test case and applied to trunk and 1.1.x

        Show
        Jan Lehnardt added a comment - Added a test case and applied to trunk and 1.1.x

          People

          • Assignee:
            Jan Lehnardt
            Reporter:
            Simon Leblanc
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development