benoitc / couchbeam

Apache CouchDB client in Erlang
Other
242 stars 113 forks source link

couchbeam with url encoded "_id" is not working #18

Closed sendtopms closed 14 years ago

sendtopms commented 14 years ago

couchbeam with url encoded "_id" which contains "/" or Forward Slash is not working http://localhost:5984/forthplace/palace%2Fitem%2F2009%2F11%2F14%2F1258178598 is valid URL and I am able to get the JSON request through browser and CURL.

benoitc commented 14 years ago

Id should be provided undecoded, couchbeam encode it. What about if someone would like to have % in its url ?

sendtopms commented 14 years ago

The issue I found is not symmetric with other tool like CURL or CouchDb's own http interface. Same thing works, so why can't couchbeam. In my app, I am heavily using "/" in the custom ID. Actually I needed to url_encode using couchbeam_util in order to avoid some issue with my app so URL itself encoded before reaching couchbeam_db. Is there a way to un-encode?

benoitc commented 14 years ago

uh ? do you use latest head ? the "/" in uri should be encoded. There is a test for that :

Doc9 = {[{<<"id">>, <<"~!@#$%^&*()+-=[]{}|;':,./<> ?">>}]}, Doc10 = couchbeam_db:save_doc(Db, Doc9), Doc101 = couchbeam_db:opendoc(Db, <<"~!@#$%^&()+-=[]{}|;':,./<> ?">>), etap:ok(case Doc10 of {} -> true; _ -> false end, "doc with special char created ok"), etap:is(couchbeam_doc:get_value(<<"id">>, Doc101), <<"~!@#$%^&()_+-=[]{}|;':,./<> ?">>, "doc with special char created ok 2"),

what are you passing to couchbeam ?

sendtopms commented 14 years ago

I am using latest truck only Create a document with the Id "palace/item/2009/11/14/F1258178598" Then fetch the document using CURL and Browser using following url http://localhost:5984/forthplace/palace%2Fitem%2F2009%2F11%2F14%2F1258178598Note:Here I have forthplace as DB name. Use couchbeam's API couchbeam_db:open_doc (NAME_OF_PROCESS, "palace%2Fitem%2F2009%2F11%2F14%2F1258178598"). you need to get the same result as CouchDb or Curl.

sendtopms commented 14 years ago

The problem with above type of ID value is, it is already encoded. Cocuhbeam or the HTTP API which couchbeam uses also trying to encode it, so at the end the value is double encoded and became unfindable by couchdb.

benoitc commented 14 years ago

Yes. Solution is to not encode the docid. Why would like to do it ? Like I said in a message (too bad github don't forward here too) is that we could maybe detect if %2F is in the docid then decide to not encode. Simply relying on % isn't enough sinc e soem could want to have it in their urls. Other solution would be to add a switch allowing passing encoed docid. I'm in favour to pass by default non encoded urls because that it is what couchdb store, and that's is what you should have at first.

benoitc commented 14 years ago

I will integrate later 2 days both solutions :

sendtopms commented 14 years ago

It is not only %2f Or %2F but some other char which couchbeam_util does in url_encode method. Thanks for the understanding the issue. I will also try to do some more research on how not to encode twice or multiple times if the data is already encoded.

sendtopms commented 14 years ago

mochiweb_util:unquote does unquoting. May be calling this method before encoding will help to avoid double encoding.

sendtopms commented 14 years ago

I handled it my application

benoitc commented 14 years ago

so am not sure to understand this report. Actually what couchbeam do is :

which should be the correct behavior. Am I missed something ?

sendtopms commented 14 years ago

When doc id is already encoded, ie before reaching couchbeam, couchebeam is not detecting this, so it also kicks on encoding and ending up with double encoding. Existing code will work as long as input doc id is not encoded. Solution may be detecting whether Id is already encoded or not and react based on the outcome or have one more method one more boolean parameter to decide encoding. Thanks

benoitc commented 14 years ago

I'm looking for a way to detect encoding. for now I didn't find one but it would be indeed the best way to do it.

sendtopms commented 14 years ago

Probably have one more function open_doc/3 with encode default to false. Based on encode, decide whether encode or not. Is it OK?

benoitc commented 14 years ago

default should be true imo. This is'nt natural to pass encoded docid. Also maybe a switch ? -define(ENCODE_DOCID, true). ?

benoitc commented 14 years ago

-define(ENCODE_DOCID, true). has been added to couchbeam_db

sendtopms commented 14 years ago

Thats fine. Since we are passing encode (true|false) as part of function argument, I don't think we need a switch.

sendtopms commented 14 years ago

I am talking encoding only in the context of open_doc. But the same time Documents which is returned from couchbeam's open_doc, fetch_view, fetch_attachment should be handled independent of open_doc. Is my assumption true?

benoitc commented 14 years ago

Returned DocId is alwasy decoded (couchdb store it undecoded). Now the problem is that if you don't want to encode docid it will be at all level (open,delete, fetch, save, ....) that's why I suppose a global switch is better for that. Having another args to function is a pain. There is also the possibility to use erlang:put ?

sendtopms commented 14 years ago

The problem is in attachment, this is the flow in my app i am currently having,

  1. User uploads the file with document Id to be attached
  2. My app receives the attachment and docid and open's the document using couchbeam_db:open_doc
  3. Uses this document to attach the uploaded document, In step 3, The document returned by Couchbeam is not encoded and it can't be used in add attachement. This is where flow breaks.
sendtopms commented 14 years ago

encoding has 2 kinds of impact.

  1. Documents coming from app to couchbeam
  2. Documents going out of couchbeam to app If we take these 2 scenario, we can handle or hide how couchdb actually handles. So you Macro will not be sufficient, it is my humble opinion.
benoitc commented 14 years ago

I'm trying to summarize : 1 -> you get docid + attachment to make. is docid encoded ? 2 -> you open doc (bettet to unencode docid if its) 3 -> you get doc. docid is undecoded in it. 4 -> You attach doc. Here I don't know what you do. 2 possibilities :

where doc is the doc you got on 3

i'm not sure in both case where is the problem if you got the doc in 3) ?

sendtopms commented 14 years ago

Thanks for the reply. In couchbeam_doc:add_attachment(Doc, Content, AName) will not work if my "_id" is not encoded. I need to retrieve id from Doc and encode it before making attachment. Since Doc coming out of Couchbeam is not encoded, the same Doc will not work when I use it in Attachment. The problem here is my "id" has "/", so these explicit encode is needed.

benoitc commented 14 years ago

mmm not sure to follow. afailk couchbeam_db:save_doc(Db, Doc1) encode the docid in Doc1. http://github.com/benoitc/couchbeam/blob/master/src/couchbeam_db.erl#L265

or there smth I don't understand :/

sendtopms commented 14 years ago

sorry.... Please do the following steps to understand the issue.

  1. Create a document with id "item/demo/item1"
  2. latter add attachment to the document created in step1
  3. Make sure that, through couchbeam retrieve the document first and attach using couchbeam's add attachment API. It will not work because the document id which couchbeam returns will not be encoded. --- I meant here -- couchbeam accepts either a) {docId, rev} + attachment details or doc, + b) attachment details. option (b) will not work. This is the issue I am trying to explain.

The problem with couchbeam is in save_doc it encodes but it did not encode other places which i explained above.

benoitc commented 14 years ago

uh ? couchbeam_db:put_attachment encode docid :

http://github.com/benoitc/couchbeam/blob/master/src/couchbeam_db.erl#L322

could you provide me a snippet or better an etap test as your convenience I could works on ?

sendtopms commented 14 years ago

This is my snippet for the error, Sorry I didn't test it, but it is the scenario I am encountering.

 
  Doc11 = {[
           {<<"_id">>, <<"test/test/2">>}
       ]},

    Doc1110 = couchbeam_db:save_doc(Db, Doc11),
    Doc111 = couchbeam_db:open_doc(Db, "test/test/2"),
    Doc112= couchbeam_db:put_attachment(Db, Doc111, "test", "test", length("test")),
    Attachment10 = couchbeam_db:fetch_attachment(Db, Doc111, "test"),
    etap:is(Attachment10, <<"test">>, "fetch attachment with encoded id ok"),   
benoitc commented 14 years ago

afaik fetch_attachment for now take only DocId not a doc obj. i will add this new feature soon.

sendtopms commented 14 years ago

sorry I actually tried with add_attachment which supports document or {docid, rev}, this is the one failing.