Skip to content

Commit 3fc6fe2

Browse files
committed
Add key range check for _all_docs
Using key and start/end_key in a different order will produce different responses when querying `_all_docs`. To reduce confusion, add a key range check for `_all_docs`. - If direction=fwd, start_key > end_key throws an error - If direction=rev, start_key < end_key throws an error - Otherwise, return relevant responses
1 parent 290ea87 commit 3fc6fe2

File tree

3 files changed

+59
-37
lines changed

3 files changed

+59
-37
lines changed

src/chttpd/test/eunit/chttpd_view_test.erl

+37-36
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242
<<"error">> := <<"query_parse_error">>,
4343
<<"reason">> := <<"`keys` is incompatible with `key`, `start_key` and `end_key`">>
4444
}).
45+
-define(ERROR_KEY_RANGE, #{
46+
<<"error">> := <<"query_parse_error">>,
47+
<<"reason">> :=
48+
<<"No rows can match your key range, reverse your ",
49+
"start_key and end_key or set descending=true">>
50+
}).
4551

4652
% seconds
4753
-define(TIMEOUT, 60).
@@ -163,26 +169,17 @@ t_view_with_multiple_queries(Db) ->
163169
?assertEqual(200, Code).
164170

165171
t_view_with_key_and_start_key(Db) ->
166-
{Code1, Res1} = req(get, url(Db, "_design/ddoc/_view/map", "key=\"a\"&startkey=\"b\"")),
167-
{Code2, Res2} = req(get, url(Db, "_design/ddoc/_view/map", "startkey=\"b\"&key=\"a\"")),
168-
?assertMatch(
169-
#{
170-
<<"error">> := <<"query_parse_error">>,
171-
<<"reason">> :=
172-
<<"No rows can match your key range, reverse your start_key and end_key or set descending=true">>
173-
},
174-
Res1
175-
),
176-
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
177-
?assertEqual(400, Code1),
178-
?assertEqual(200, Code2).
172+
test_helper_key_and_start_key(Db, "_design/ddoc/_view/map").
179173

180174
t_all_docs_with_key_and_start_key(Db) ->
181-
{Code1, Res1} = req(get, url(Db, "_all_docs", "key=\"a\"&startkey=\"b\"")),
182-
{Code2, Res2} = req(get, url(Db, "_all_docs", "startkey=\"b\"&key=\"a\"")),
183-
?assertMatch(#{<<"rows">> := []}, Res1),
175+
test_helper_key_and_start_key(Db, "_all_docs").
176+
177+
test_helper_key_and_start_key(Db, Path) ->
178+
{Code1, Res1} = req(get, url(Db, Path, "key=\"a\"&startkey=\"b\"")),
179+
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&key=\"a\"")),
180+
?assertMatch(?ERROR_KEY_RANGE, Res1),
184181
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
185-
?assertEqual(200, Code1),
182+
?assertEqual(400, Code1),
186183
?assertEqual(200, Code2).
187184

188185
t_view_with_key_and_end_key(Db) ->
@@ -200,31 +197,35 @@ test_helper_key_and_end_key(Db, Path) ->
200197
?assertEqual(200, Code2).
201198

202199
t_view_with_single_keys_and_start_key(Db) ->
203-
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map?keys=[\"a\"]&startkey=\"b\"")),
204-
?assertMatch(
205-
#{
206-
<<"error">> := <<"query_parse_error">>,
207-
<<"reason">> :=
208-
<<"No rows can match your key range, reverse your start_key and end_key or set descending=true">>
209-
},
210-
Res
211-
),
212-
?assertEqual(400, Code).
200+
test_helper_single_keys_and_start_key(Db, "_design/ddoc/_view/map").
213201

214202
t_all_docs_with_single_keys_and_start_key(Db) ->
215-
{Code, Res} = req(get, url(Db, "_all_docs?keys=[\"a\"]&startkey=\"b\"")),
216-
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
217-
?assertEqual(400, Code).
203+
test_helper_single_keys_and_start_key(Db, "_all_docs").
204+
205+
test_helper_single_keys_and_start_key(Db, Path) ->
206+
{Code1, Res1} = req(get, url(Db, Path, "keys=[\"a\"]&startkey=\"b\"")),
207+
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&keys=[\"a\"]")),
208+
case Path of
209+
"_all_docs" -> ?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res1);
210+
_ -> ?assertMatch(?ERROR_KEY_RANGE, Res1)
211+
end,
212+
?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2),
213+
?assertEqual(400, Code1),
214+
?assertEqual(200, Code2).
218215

219216
t_view_with_keys_and_start_key(Db) ->
220-
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map", "keys=[\"a\",\"b\"]&start_key=\"b\"")),
221-
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
222-
?assertEqual(400, Code).
217+
test_helper_keys_and_start_key(Db, "_design/ddoc/_view/map").
223218

224219
t_all_docs_with_keys_and_start_key(Db) ->
225-
{Code, Res} = req(get, url(Db, "_all_docs", "keys=[\"a\",\"b\"]&start_key=\"b\"")),
226-
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res),
227-
?assertEqual(400, Code).
220+
test_helper_keys_and_start_key(Db, "_all_docs").
221+
222+
test_helper_keys_and_start_key(Db, Path) ->
223+
{Code1, Res1} = req(get, url(Db, Path, "keys=[\"a\",\"b\"]&start_key=\"b\"")),
224+
{Code2, Res2} = req(get, url(Db, Path, "startkey=\"b\"&keys=[\"a\",\"b\"]")),
225+
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res1),
226+
?assertMatch(?ERROR_KEYS_INCOMPATIBLE, Res2),
227+
?assertEqual(400, Code1),
228+
?assertEqual(400, Code2).
228229

229230
t_view_with_key_non_existent_docs(Db) ->
230231
{Code, Res} = req(get, url(Db, "_design/ddoc/_view/map", "key=\"not_exist\"")),

src/couch_mrview/src/couch_mrview_util.erl

+21
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,7 @@ apply_limit(ViewPartitioned, Args) ->
533533

534534
validate_all_docs_args(Db, Args0) ->
535535
Args = validate_args(Args0),
536+
check_range_all_docs(Args),
536537

537538
DbPartitioned = couch_db:is_partitioned(Db),
538539
Partition = get_extra(Args, partition),
@@ -782,6 +783,26 @@ apply_all_docs_partition(#mrargs{} = Args, Partition) ->
782783
end_key = EK1
783784
}.
784785

786+
check_range_all_docs(#mrargs{direction = Dir, start_key = SK, end_key = EK}) ->
787+
case {Dir, SK, EK} of
788+
{_, undefined, _} ->
789+
ok;
790+
{_, _, undefined} ->
791+
ok;
792+
{fwd, SK, EK} when SK > EK ->
793+
mrverror(
794+
<<"No rows can match your key range, reverse your ",
795+
"start_key and end_key or set descending=true">>
796+
);
797+
{rev, SK, EK} when SK < EK ->
798+
mrverror(
799+
<<"No rows can match your key range, reverse your ",
800+
"start_key and end_key or set descending=false">>
801+
);
802+
_ ->
803+
ok
804+
end.
805+
785806
check_range(#mrargs{start_key = undefined}, _Cmp) ->
786807
ok;
787808
check_range(#mrargs{end_key = undefined}, _Cmp) ->

src/docs/src/api/ddoc/views.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ multi-element ``keys``. For single-element ``keys``, treat it as a ``key``.
651651
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?key="a"'
652652
{"rows":[{"key":null,"value":1}]}
653653
654-
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys="[\"a\"]"'
654+
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys=\["a"\]'
655655
{"rows":[{"key":null,"value":1}]}
656656
657657
$ curl http://adm:[email protected]:5984/db/_design/ddoc/_view/reduce'?keys=\["a","b"\]'

0 commit comments

Comments
 (0)