Improve handling of + in URLs

9 files changed, 152 insertions, 56 deletions

diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index a65d667dc..6085e5134 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -108,6 +108,9 @@ bind_address = 127.0.0.1
 ; Maximum allowed http request size. Applies to both clustered and local port.
 ;max_http_request_size = 4294967296 ; 4GB
 
+; Set to true to decode + to space in db and doc_id parts.
+; decode_plus_to_space = false
+
 ;[jwt_auth]
 ; List of claims to validate
 ; can be the name of a claim like "exp" or a tuple if the claim requires
diff --git a/src/chttpd/src/chttpd.erl b/src/chttpd/src/chttpd.erl
index 1f32e8260..7a05b2c49 100644
--- a/src/chttpd/src/chttpd.erl
+++ b/src/chttpd/src/chttpd.erl
@@ -322,7 +322,7 @@ handle_request_int(MochiReq) ->
         nonce = Nonce,
         method = Method,
         path_parts = [
-            list_to_binary(chttpd:unquote(Part))
+            list_to_binary(unquote(Part))
          || Part <- string:tokens(Path, "/")
         ],
         requested_path_parts = [
@@ -802,7 +802,10 @@ absolute_uri(#httpd{absolute_uri = URI}, Path) ->
     URI ++ Path.
 
 unquote(UrlEncodedString) ->
-    mochiweb_util:unquote(UrlEncodedString).
+    case config:get_boolean("chttpd", "decode_plus_to_space", false) of
+        true -> mochiweb_util:unquote(UrlEncodedString);
+        false -> mochiweb_util:unquote_path(UrlEncodedString)
+    end.
 
 quote(UrlDecodedString) ->
     mochiweb_util:quote_plus(UrlDecodedString).
diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 9edf2434a..c9a843aae 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1283,7 +1283,7 @@ db_doc_req(#httpd{method = 'COPY'} = Req, Db, SourceDocId) ->
             Rev -> Rev
         end,
     {TargetDocId0, TargetRevs} = chttpd_util:parse_copy_destination_header(Req),
-    TargetDocId = list_to_binary(mochiweb_util:unquote(TargetDocId0)),
+    TargetDocId = list_to_binary(chttpd:unquote(TargetDocId0)),
     % open old doc
     Doc = couch_doc_open(Db, SourceDocId, SourceRev, []),
     % save new doc
diff --git a/src/chttpd/src/chttpd_httpd_handlers.erl b/src/chttpd/src/chttpd_httpd_handlers.erl
index c8a399c7b..53f2aa893 100644
--- a/src/chttpd/src/chttpd_httpd_handlers.erl
+++ b/src/chttpd/src/chttpd_httpd_handlers.erl
@@ -417,7 +417,7 @@ handler_info(_, _, _) ->
 get_copy_destination(Req) ->
     try
         {DocIdStr, _} = chttpd_util:parse_copy_destination_header(Req),
-        list_to_binary(mochiweb_util:unquote(DocIdStr))
+        list_to_binary(chttpd:unquote(DocIdStr))
     catch
         _:_ ->
             unknown
diff --git a/src/chttpd/test/eunit/chttpd_cors_test.erl b/src/chttpd/test/eunit/chttpd_cors_test.erl
index 2faa629bf..fd4a48745 100644
--- a/src/chttpd/test/eunit/chttpd_cors_test.erl
+++ b/src/chttpd/test/eunit/chttpd_cors_test.erl
@@ -136,59 +136,84 @@ assert_not_preflight_(Val) ->
 cors_disabled_test_() ->
     {"CORS disabled tests", [
         {"Empty user",
-            {foreach, fun empty_cors_config/0, [
-                fun test_no_access_control_method_preflight_request_/1,
-                fun test_no_headers_/1,
-                fun test_no_headers_server_/1,
-                fun test_no_headers_db_/1
-            ]}}
+            {setup,
+                fun chttpd_test_util:start_couch/0,
+                fun chttpd_test_util:stop_couch/1,
+                {foreach, fun empty_cors_config/0, [
+                    fun test_no_access_control_method_preflight_request_/1,
+                    fun test_no_headers_/1,
+                    fun test_no_headers_server_/1,
+                    fun test_no_headers_db_/1
+                ]}
+            }
+        }
     ]}.
 
 %% CORS enabled tests
 
 cors_enabled_minimal_config_test_() ->
     {"Minimal CORS enabled, no Origins",
-        {foreach, fun minimal_cors_config/0, [
-            fun test_no_access_control_method_preflight_request_/1,
-            fun test_incorrect_origin_simple_request_/1,
-            fun test_incorrect_origin_preflight_request_/1
-        ]}}.
+        {setup,
+            fun chttpd_test_util:start_couch/0,
+            fun chttpd_test_util:stop_couch/1,
+            {foreach, fun minimal_cors_config/0, [
+                fun test_no_access_control_method_preflight_request_/1,
+                fun test_incorrect_origin_simple_request_/1,
+                fun test_incorrect_origin_preflight_request_/1
+            ]}
+        }
+    }.
 
 cors_enabled_simple_config_test_() ->
     {"Simple CORS config",
-        {foreach, fun simple_cors_config/0, [
-            fun test_no_access_control_method_preflight_request_/1,
-            fun test_preflight_request_/1,
-            fun test_bad_headers_preflight_request_/1,
-            fun test_good_headers_preflight_request_/1,
-            fun test_db_request_/1,
-            fun test_db_preflight_request_/1,
-            fun test_db_host_origin_request_/1,
-            fun test_preflight_with_port_no_origin_/1,
-            fun test_preflight_with_scheme_no_origin_/1,
-            fun test_preflight_with_scheme_port_no_origin_/1,
-            fun test_case_sensitive_mismatch_of_allowed_origins_/1
-        ]}}.
+        {setup,
+            fun chttpd_test_util:start_couch/0,
+            fun chttpd_test_util:stop_couch/1,
+            {foreach, fun simple_cors_config/0, [
+                fun test_no_access_control_method_preflight_request_/1,
+                fun test_preflight_request_/1,
+                fun test_bad_headers_preflight_request_/1,
+                fun test_good_headers_preflight_request_/1,
+                fun test_db_request_/1,
+                fun test_db_preflight_request_/1,
+                fun test_db_host_origin_request_/1,
+                fun test_preflight_with_port_no_origin_/1,
+                fun test_preflight_with_scheme_no_origin_/1,
+                fun test_preflight_with_scheme_port_no_origin_/1,
+                fun test_case_sensitive_mismatch_of_allowed_origins_/1
+            ]}
+        }
+    }.
 
 cors_enabled_custom_config_test_() ->
     {"Simple CORS config with custom allow_methods/allow_headers/exposed_headers",
-        {foreach, fun custom_cors_config/0, [
-            fun test_good_headers_preflight_request_with_custom_config_/1,
-            fun test_db_request_with_custom_config_/1
-        ]}}.
+        {setup,
+            fun chttpd_test_util:start_couch/0,
+            fun chttpd_test_util:stop_couch/1,
+            {foreach, fun custom_cors_config/0, [
+                fun test_good_headers_preflight_request_with_custom_config_/1,
+                fun test_db_request_with_custom_config_/1
+            ]}
+        }
+    }.
 
 cors_enabled_multiple_config_test_() ->
     {"Multiple options CORS config",
-        {foreach, fun multiple_cors_config/0, [
-            fun test_no_access_control_method_preflight_request_/1,
-            fun test_preflight_request_/1,
-            fun test_db_request_/1,
-            fun test_db_preflight_request_/1,
-            fun test_db_host_origin_request_/1,
-            fun test_preflight_with_port_with_origin_/1,
-            fun test_preflight_with_scheme_with_origin_/1,
-            fun test_preflight_with_scheme_port_with_origin_/1
-        ]}}.
+        {setup,
+            fun chttpd_test_util:start_couch/0,
+            fun chttpd_test_util:stop_couch/1,
+            {foreach, fun multiple_cors_config/0, [
+                fun test_no_access_control_method_preflight_request_/1,
+                fun test_preflight_request_/1,
+                fun test_db_request_/1,
+                fun test_db_preflight_request_/1,
+                fun test_db_host_origin_request_/1,
+                fun test_preflight_with_port_with_origin_/1,
+                fun test_preflight_with_scheme_with_origin_/1,
+                fun test_preflight_with_scheme_port_with_origin_/1
+            ]}
+        }
+    }.
 
 %% Access-Control-Allow-Credentials tests
 
@@ -225,19 +250,24 @@ db_request_credentials_header_on_test_() ->
 
 cors_enabled_wildcard_test_() ->
     {"Wildcard CORS config",
-        {foreach, fun wildcard_cors_config/0, [
-            fun test_no_access_control_method_preflight_request_/1,
-            fun test_preflight_request_/1,
-            fun test_preflight_request_no_allow_credentials_/1,
-            fun test_preflight_request_empty_request_headers_/1,
-            fun test_db_request_/1,
-            fun test_db_preflight_request_/1,
-            fun test_db_host_origin_request_/1,
-            fun test_preflight_with_port_with_origin_/1,
-            fun test_preflight_with_scheme_with_origin_/1,
-            fun test_preflight_with_scheme_port_with_origin_/1,
-            fun test_case_sensitive_mismatch_of_allowed_origins_/1
-        ]}}.
+        {setup,
+            fun chttpd_test_util:start_couch/0,
+            fun chttpd_test_util:stop_couch/1,
+            {foreach, fun wildcard_cors_config/0, [
+                fun test_no_access_control_method_preflight_request_/1,
+                fun test_preflight_request_/1,
+                fun test_preflight_request_no_allow_credentials_/1,
+                fun test_preflight_request_empty_request_headers_/1,
+                fun test_db_request_/1,
+                fun test_db_preflight_request_/1,
+                fun test_db_host_origin_request_/1,
+                fun test_preflight_with_port_with_origin_/1,
+                fun test_preflight_with_scheme_with_origin_/1,
+                fun test_preflight_with_scheme_port_with_origin_/1,
+                fun test_case_sensitive_mismatch_of_allowed_origins_/1
+            ]}
+        }
+    }.
 
 %% Test generators
 
diff --git a/src/couch/src/couch_httpd.erl b/src/couch/src/couch_httpd.erl
index c2ee42e4f..1de47a3d3 100644
--- a/src/couch/src/couch_httpd.erl
+++ b/src/couch/src/couch_httpd.erl
@@ -252,7 +252,7 @@ absolute_uri(_Req, _Path) ->
     throw({bad_request, "path must begin with a /."}).
 
 unquote(UrlEncodedString) ->
-    mochiweb_util:unquote(UrlEncodedString).
+    chttpd:unquote(UrlEncodedString).
 
 quote(UrlDecodedString) ->
     mochiweb_util:quote_plus(UrlDecodedString).
diff --git a/src/weatherreport/weatherreport b/src/weatherreport/weatherreport
new file mode 100755
index 000000000..0b317bfd2
--- /dev/null
+++ b/src/weatherreport/weatherreport
diff --git a/test/elixir/test/basics_test.exs b/test/elixir/test/basics_test.exs
index 37af32bd7..2f6dbaf7f 100644
--- a/test/elixir/test/basics_test.exs
+++ b/test/elixir/test/basics_test.exs
@@ -72,6 +72,62 @@ defmodule BasicsTest do
     assert context[:db_name] in Couch.get("/_all_dbs").body, "Db name in _all_dbs"
   end
 
+  test "Database name with '+' should encode to '+'", _context do
+    set_config({"chttpd", "decode_plus_to_space", "false"})
+
+    random_number = :rand.uniform(16_000_000)
+    db_name = "random+test+db+#{random_number}"
+    resp = Couch.put("/#{db_name}")
+
+    assert resp.status_code == 201
+    assert resp.body["ok"] == true
+
+    resp = Couch.get("/#{db_name}")
+
+    assert resp.status_code == 200
+    assert resp.body["db_name"] == db_name
+  end
+
+  test "Database name with '%2B' should encode to '+'", _context do
+    set_config({"chttpd", "decode_plus_to_space", "true"})
+
+    random_number = :rand.uniform(16_000_000)
+    db_name = "random%2Btest%2Bdb2%2B#{random_number}"
+    resp = Couch.put("/#{db_name}")
+
+    assert resp.status_code == 201
+    assert resp.body["ok"] == true
+
+    resp = Couch.get("/#{db_name}")
+
+    assert resp.status_code == 200
+    assert resp.body["db_name"] == "random+test+db2+#{random_number}"
+  end
+
+  @tag :with_db
+  test "'+' in document name should encode to '+'", context do
+    set_config({"chttpd", "decode_plus_to_space", "false"})
+
+    db_name = context[:db_name]
+    doc_id = "test+doc"
+    resp = Couch.put("/#{db_name}/#{doc_id}", body: %{})
+
+    assert resp.status_code == 201
+    assert resp.body["id"] == "test+doc"
+  end
+
+  @tag :with_db
+  test "'+' in document name should encode to space", context do
+    set_config({"chttpd", "decode_plus_to_space", "true"})
+
+    db_name = context[:db_name]
+    doc_id = "test+doc+2"
+    resp = Couch.put("/#{db_name}/#{doc_id}", body: %{})
+
+    assert resp.status_code == 201
+    assert resp.body["id"] == "test doc 2"
+  end
+
   @tag :with_db
   test "Empty database should have zero docs", context do
     assert Couch.get("/#{context[:db_name]}").body["doc_count"] == 0,
diff --git a/test/elixir/test/config/suite.elixir b/test/elixir/test/config/suite.elixir
index 467ef2c34..e49825063 100644
--- a/test/elixir/test/config/suite.elixir
+++ b/test/elixir/test/config/suite.elixir
@@ -58,6 +58,10 @@
     "Creating a new DB with slashes should return Location header (COUCHDB-411)",
     "DELETE'ing a non-existent doc should 404",
     "Database should be in _all_dbs",
+    "Database name with '+' should encode to '+'",
+    "'+' in document name should encode to '+'",
+    "Database name with '%2B' should encode to '+'",
+    "'+' in document name should encode to space",
     "Default headers are returned for doc with open_revs=all",
     "Empty database should have zero docs",
     "Exceeding configured DB name size limit returns an error",