Improve handling of + in urls 3.x

7 files changed, 116 insertions, 36 deletions

diff --git a/rebar.config.script b/rebar.config.script
index 0a8fa5144..52b9d99d7 100644
--- a/rebar.config.script
+++ b/rebar.config.script
@@ -162,7 +162,7 @@ DepDescs = [
 {hyper,            "hyper",            {tag, "CouchDB-2.2.0-7"}},
 {ibrowse,          "ibrowse",          {tag, "CouchDB-4.4.2-5"}},
 {jiffy,            "jiffy",            {tag, "CouchDB-1.0.5-1"}},
-{mochiweb,         "mochiweb",         {tag, "v2.21.0"}},
+{mochiweb,         "mochiweb",         {tag, "CouchDB-v2.21.0-1"}},
 {meck,             "meck",             {tag, "0.9.2"}},
 {recon,            "recon",            {tag, "2.5.0"}}
 ].
diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index 5f916a0af..d64fb0a0c 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -167,6 +167,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 = true
+
 ;[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 8fd05597d..aea1ee407 100644
--- a/src/chttpd/src/chttpd.erl
+++ b/src/chttpd/src/chttpd.erl
@@ -231,7 +231,7 @@ handle_request_int(MochiReq) ->
         original_method = Method1,
         nonce = Nonce,
         method = Method,
-        path_parts = [list_to_binary(chttpd:unquote(Part))
+        path_parts = [list_to_binary(unquote(Part))
                 || Part <- string:tokens(Path, "/")],
         requested_path_parts = [?l2b(unquote(Part))
                 || Part <- string:tokens(RequestedPath, "/")]
@@ -631,7 +631,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", true) 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 d45727879..ffdab2a29 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1101,7 +1101,7 @@ db_doc_req(#httpd{method='COPY', user_ctx=Ctx}=Req, Db, SourceDocId) ->
         Rev -> Rev
     end,
     {TargetDocId0, TargetRevs} = couch_httpd_db: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/test/eunit/chttpd_cors_test.erl b/src/chttpd/test/eunit/chttpd_cors_test.erl
index 19e851561..44644b571 100644
--- a/src/chttpd/test/eunit/chttpd_cors_test.erl
+++ b/src/chttpd/test/eunit/chttpd_cors_test.erl
@@ -137,17 +137,20 @@ 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
-                    ]}}]}.
+    {"CORS disabled tests", [
+        {"Empty user",
+            {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
@@ -155,20 +158,24 @@ cors_disabled_test_() ->
 
 cors_enabled_minimal_config_test_() ->
     {"Minimal CORS enabled, no Origins",
-        {foreach,
-            fun minimal_cors_config/0,
-            [
+        {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,
-            [
+        {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,
@@ -180,23 +187,29 @@ cors_enabled_simple_config_test_() ->
                 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,
-            [
+        {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,
-            [
+        {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,
@@ -205,7 +218,9 @@ cors_enabled_multiple_config_test_() ->
                 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
@@ -251,9 +266,10 @@ db_request_credentials_header_on_test_() ->
 
 cors_enabled_wildcard_test_() ->
     {"Wildcard CORS config",
-        {foreach,
-            fun wildcard_cors_config/0,
-            [
+        {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,
@@ -265,7 +281,9 @@ cors_enabled_wildcard_test_() ->
                 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 00379bbb4..535fc9245 100644
--- a/src/couch/src/couch_httpd.erl
+++ b/src/couch/src/couch_httpd.erl
@@ -579,7 +579,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/test/elixir/test/basics_test.exs b/test/elixir/test/basics_test.exs
index dbdc7d193..e6fb20938 100644
--- a/test/elixir/test/basics_test.exs
+++ b/test/elixir/test/basics_test.exs
@@ -58,6 +58,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,