You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ei...@apache.org on 2020/08/13 17:29:30 UTC

[couchdb] branch validate-db-create-params created (now e485b58)

This is an automated email from the ASF dual-hosted git repository.

eiri pushed a change to branch validate-db-create-params
in repository https://gitbox.apache.org/repos/asf/couchdb.git.


      at e485b58  Validate shard specific query params on db create request

This branch includes the following new commits:

     new e485b58  Validate shard specific query params on db create request

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[couchdb] 01/01: Validate shard specific query params on db create request

Posted by ei...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

eiri pushed a commit to branch validate-db-create-params
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit e485b58ec18d8d4cc36f00c54ac2b2475c742162
Author: Eric Avdey <ei...@eiri.ca>
AuthorDate: Thu Aug 13 14:28:52 2020 -0300

    Validate shard specific query params on db create request
---
 src/chttpd/src/chttpd_db.erl | 127 ++++++++++++++++++++++++++++++++++++++++---
 src/couch/src/couch_util.erl |  12 ++++
 2 files changed, 130 insertions(+), 9 deletions(-)

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 6a3df6d..aaf131f 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -383,17 +383,10 @@ handle_design_info_req(Req, _Db, _DDoc) ->
 
 create_db_req(#httpd{}=Req, DbName) ->
     couch_httpd:verify_is_server_admin(Req),
-    N = chttpd:qs_value(Req, "n", config:get("cluster", "n", "3")),
-    Q = chttpd:qs_value(Req, "q", config:get("cluster", "q", "8")),
-    P = chttpd:qs_value(Req, "placement", config:get("cluster", "placement")),
+    ShardsOpt = parse_shards_opt(Req),
     EngineOpt = parse_engine_opt(Req),
     DbProps = parse_partitioned_opt(Req),
-    Options = [
-        {n, N},
-        {q, Q},
-        {placement, P},
-        {props, DbProps}
-    ] ++ EngineOpt,
+    Options = lists:append([ShardsOpt, [{props, DbProps}], EngineOpt]),
     DocUrl = absolute_uri(Req, "/" ++ couch_util:url_encode(DbName)),
     case fabric:create_db(DbName, Options) of
     ok ->
@@ -1702,6 +1695,37 @@ get_md5_header(Req) ->
 parse_doc_query(Req) ->
     lists:foldl(fun parse_doc_query/2, #doc_query_args{}, chttpd:qs(Req)).
 
+parse_shards_opt(Req) ->
+    [
+        {n, parse_shards_opt("n", Req, config:get("cluster", "n", "3"))},
+        {q, parse_shards_opt("q", Req, config:get("cluster", "q", "8"))},
+        {placement, parse_shards_opt(
+            "placement", Req, config:get("cluster", "placement"))}
+    ].
+
+parse_shards_opt("placement", Req, Default) ->
+    Err = <<"The `placement` value should be in a format `zone:n`.">>,
+    case chttpd:qs_value(Req, "placement", Default) of
+        Default -> Default;
+        Val ->
+            try
+                [_, N] = string:tokens(Val, ":"),
+                true = couch_util:validate_positive_int(N),
+                Val
+            catch _:_ ->
+                throw({bad_request, Err})
+            end
+    end;
+
+parse_shards_opt(Param, Req, Default) ->
+    Val = chttpd:qs_value(Req, Param, Default),
+    Err = ?l2b(["The `", Param, "` value should be a positive integer."]),
+    case couch_util:validate_positive_int(Val) of
+        true -> Val;
+        false -> throw({bad_request, Err})
+    end.
+
+
 parse_engine_opt(Req) ->
     case chttpd:qs_value(Req, "engine") of
         undefined ->
@@ -2118,8 +2142,26 @@ parse_partitioned_opt_test_() ->
         ]
     }.
 
+parse_shards_opt_test_() ->
+    {
+        foreach,
+        fun setup/0,
+        fun teardown/1,
+        [
+            t_should_allow_valid_q(),
+            t_should_default_on_missing_q(),
+            t_should_throw_on_invalid_q(),
+            t_should_allow_valid_n(),
+            t_should_default_on_missing_n(),
+            t_should_throw_on_invalid_n(),
+            t_should_allow_valid_placement(),
+            t_should_default_on_missing_placement(),
+            t_should_throw_on_invalid_placement()
+        ]
+    }.
 
 setup() ->
+    meck:expect(config, get, fun(_, _, Default) -> Default end),
     ok.
 
 teardown(_) ->
@@ -2158,4 +2200,71 @@ t_returns_empty_array_for_no_partitioned_qs() ->
         ?assertEqual(parse_partitioned_opt(Req), [])
     end).
 
+t_should_allow_valid_q() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?q=1"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual("1", couch_util:get_value(q, Opts))
+    end).
+
+t_should_default_on_missing_q() ->
+    ?_test(begin
+        Req = mock_request("/all-test21"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual("8", couch_util:get_value(q, Opts))
+    end).
+
+t_should_throw_on_invalid_q() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?q="),
+        Err = <<"The `q` value should be a positive integer.">>,
+        ?assertThrow({bad_request, Err}, parse_shards_opt(Req))
+    end).
+
+t_should_allow_valid_n() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?n=1"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual("1", couch_util:get_value(n, Opts))
+    end).
+
+t_should_default_on_missing_n() ->
+    ?_test(begin
+        Req = mock_request("/all-test21"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual("3", couch_util:get_value(n, Opts))
+    end).
+
+t_should_throw_on_invalid_n() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?n="),
+        Err = <<"The `n` value should be a positive integer.">>,
+        ?assertThrow({bad_request, Err}, parse_shards_opt(Req))
+    end).
+
+t_should_allow_valid_placement() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?placement=az:1"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual("az:1", couch_util:get_value(placement, Opts))
+    end).
+
+t_should_default_on_missing_placement() ->
+    ?_test(begin
+        Req = mock_request("/all-test21"),
+        Opts = parse_shards_opt(Req),
+        ?assertEqual(undefined, couch_util:get_value(placement, Opts))
+    end).
+
+t_should_throw_on_invalid_placement() ->
+    ?_test(begin
+        Req = mock_request("/all-test21?placement="),
+        Err = <<"The `placement` value should be in a format `zone:n`.">>,
+        ?assertThrow({bad_request, Err}, parse_shards_opt(Req)),
+        Req2 = mock_request("/all-test21?placement=moon"),
+        ?assertThrow({bad_request, Err}, parse_shards_opt(Req2)),
+        Req3 = mock_request("/all-test21?placement=moon:eagle"),
+        ?assertThrow({bad_request, Err}, parse_shards_opt(Req2))
+    end).
+
 -endif.
diff --git a/src/couch/src/couch_util.erl b/src/couch/src/couch_util.erl
index dffb681..95780e8 100644
--- a/src/couch/src/couch_util.erl
+++ b/src/couch/src/couch_util.erl
@@ -31,6 +31,7 @@
 -export([with_db/2]).
 -export([rfc1123_date/0, rfc1123_date/1]).
 -export([integer_to_boolean/1, boolean_to_integer/1]).
+-export([validate_positive_int/1]).
 -export([find_in_binary/2]).
 -export([callback_exists/3, validate_callback_exists/3]).
 -export([with_proc/4]).
@@ -624,6 +625,17 @@ boolean_to_integer(false) ->
     0.
 
 
+validate_positive_int(N) when is_list(N) ->
+    try
+        I = list_to_integer(N),
+        validate_positive_int(I)
+    catch error:badarg ->
+        false
+    end;
+validate_positive_int(N) when is_integer(N), N > 0 -> true;
+validate_positive_int(_) -> false.
+
+
 find_in_binary(_B, <<>>) ->
     not_found;