Decode destination header for doc copy

[jiangphcn]

Overview

The Fauxton URL request to CouchDB encodes the user input and sets the Destination header with the encoded value of the new ID when sending the COPY request. This PR is aimed to make CouchDB to decode Destination header, and creates the ID without escaped values.

jiangphs-mbp-2:geotest jiangph$ curl -u foo:bar http://127.0.0.1:15984/testdb1/point11 -X COPY -H 'Content-Type: application/json'  -H 'Destination: xxx%E5%95%8Axxx'
{"ok":true,"ok":true,"id":"xxx啊xxx","rev":"1-ffb6c2ae737918f8f911e1fce668333e"}
jiangphs-mbp-2:geotest jiangph$ curl -u foo:bar http://127.0.0.1:15984/testdb1/point11 -X COPY -H 'Content-Type: application/json'  -H 'Destination: this/is/an/example'
{"ok":true,"ok":true,"id":"this/is/an/example","rev":"1-ffb6c2ae737918f8f911e1fce668333e"}

Testing recommendations

make check skip_deps+=couch_epi apps=chttpd tests=all_test_
======================== EUnit ========================
chttpd db tests
Application crypto was left running!
  chttpd_db_test:84: should_return_ok_true_on_bulk_update...[0.089 s] ok
  chttpd_db_test:111: should_accept_live_as_an_alias_for_continuous...[0.043 s] ok
  chttpd_db_test:126: should_return_404_for_delete_att_on_notadoc...[0.004 s] ok
  chttpd_db_test:148: should_return_409_for_del_att_without_rev...[0.038 s] ok
  chttpd_db_test:166: should_return_200_for_del_att_with_rev...[0.069 s] ok
  chttpd_db_test:187: should_return_409_for_put_att_nonexistent_rev...[0.011 s] ok
  chttpd_db_test:282: should_return_correct_id_on_doc_copy...[0.083 s] ok
[os_mon] cpu supervisor port (cpu_sup): Erlang has closed
  [done in 3.898 s]
=======================================================
  All 7 tests passed.
==> rel (eunit)
==> couchdb (eunit)

Related Issues or Pull Requests

issue #977

Checklist

• Code is written and works correctly;
• Changes are covered by tests;
• Documentation reflects the changes;

[rnewson]

I think the problem here is that the doc id isn't always escaped (that change is fairly recent).

[jiangphcn]

Thanks, Bob. According to statement from Garren in issue #977

Which means the API works although it doesn't follow 100% the Destination header's spec which states the header should contain an URI, hence its value should be encoded if it includes characters outside the valid range.

If spec implies that destination header for couchdb should be encoded, we need to decode here. If there is no escaped character in docid for target doc, calling mochiweb_util:unquote/1 will not change the content of docid.

[rnewson]

There's several places where we url encode and url decode, and we've changed where we do that over multiple versions of couchdb. I'm struggling to see if we can call unquote unilaterally here. Do we need to care about this across versions or is it sufficient to be internally consistent?

[jiangphcn]

https://tools.ietf.org/html/rfc4918#section-10.3
https://tools.ietf.org/html/rfc3986

From above links, we know that

10.3.  Destination Header

   The Destination request header specifies the URI that identifies a
   destination resource for methods such as COPY and MOVE, which take
   two URIs as parameters.

      Destination = "Destination" ":" Simple-ref


   If the Destination value is an absolute-URI (Section 4.3 of
   [RFC3986]), it may name a different server (or different port or
   scheme).  If the source server cannot attempt a copy to the remote
   server, it MUST fail the request.  Note that copying and moving
   resources to remote servers is not fully defined in this
   specification (e.g., specific error conditions).

Abstract

   A Uniform Resource Identifier (URI) is a compact sequence of
   characters that identifies an abstract or physical resource.  This
   specification defines the generic URI syntax and a process for
   resolving URI references that might be in relative form, along with
   guidelines and security considerations for the use of URIs on the
   Internet.  The URI syntax defines a grammar that is a superset of all
   valid URIs, allowing an implementation to parse the common components
   of a URI reference without knowing the scheme-specific requirements
   of every possible identifier.  This specification does not define a
   generative grammar for URIs; that task is performed by the individual
   specifications of each URI scheme.

[jiangphcn]

My understanding is that it is not restricted to encode or un-encode URI. For with url encoding, it is safe from security's point of view.

When discussing with Bob and Paul, Bob mentioned "we should encode the destination header value, and decode it wherever we might need to (if anywhere)".

Here is one place we need to decode URI.

[davisp]

+1