diff --git a/src/chttpd/test/eunit/chttpd_view_test.erl b/src/chttpd/test/eunit/chttpd_view_test.erl index beb71f8231..42c5c07cbe 100644 --- a/src/chttpd/test/eunit/chttpd_view_test.erl +++ b/src/chttpd/test/eunit/chttpd_view_test.erl @@ -117,7 +117,10 @@ all_docs_test_() -> ?TDEF_FE(t_all_docs_with_limit), ?TDEF_FE(t_all_docs_with_skip), ?TDEF_FE(t_all_docs_with_limit_and_skip), - ?TDEF_FE(t_all_docs_with_configured_query_limit) + ?TDEF_FE(t_all_docs_with_configured_query_limit), + ?TDEF_FE(t_all_docs_impossible_range_fwd), + ?TDEF_FE(t_all_docs_valid_range_descending), + ?TDEF_FE(t_all_docs_impossible_range_descending) ] } } @@ -186,9 +189,17 @@ t_view_with_key_and_start_key(Db) -> t_all_docs_with_key_and_start_key(Db) -> {Code1, Res1} = req(get, url(Db, "_all_docs", "key=\"a\"&startkey=\"b\"")), {Code2, Res2} = req(get, url(Db, "_all_docs", "startkey=\"b\"&key=\"a\"")), - ?assertMatch(#{<<"rows">> := []}, Res1), + ?assertMatch( + #{ + <<"error">> := <<"query_parse_error">>, + <<"reason">> := + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=true">> + }, + Res1 + ), ?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"a">>}]}, Res2), - ?assertEqual(200, Code1), + ?assertEqual(400, Code1), ?assertEqual(200, Code2). t_view_with_key_and_end_key(Db) -> @@ -436,6 +447,37 @@ t_all_docs_with_configured_query_limit(Db) -> Res ). +t_all_docs_impossible_range_fwd(Db) -> + {Code, Res} = req(get, url(Db, "_all_docs", "startkey=\"z\"&endkey=\"a\"")), + ?assertMatch( + #{ + <<"error">> := <<"query_parse_error">>, + <<"reason">> := + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=true">> + }, + Res + ), + ?assertEqual(400, Code). + +t_all_docs_valid_range_descending(Db) -> + {Code, Res} = req(get, url(Db, "_all_docs", "startkey=\"c\"&endkey=\"a\"&descending=true")), + ?assertMatch(#{<<"rows">> := [#{<<"id">> := <<"c">>}, #{<<"id">> := <<"b">>}, #{<<"id">> := <<"a">>}]}, Res), + ?assertEqual(200, Code). + +t_all_docs_impossible_range_descending(Db) -> + {Code, Res} = req(get, url(Db, "_all_docs", "startkey=\"a\"&endkey=\"z\"&descending=true")), + ?assertMatch( + #{ + <<"error">> := <<"query_parse_error">>, + <<"reason">> := + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=false">> + }, + Res + ), + ?assertEqual(400, Code). + %%%%%%%%%%%%%%%%%%%% Utility Functions %%%%%%%%%%%%%%%%%%%% url(Db) -> Addr = config:get("chttpd", "bind_address", "127.0.0.1"), diff --git a/src/couch_mrview/src/couch_mrview.erl b/src/couch_mrview/src/couch_mrview.erl index 244f668af0..58b4342cfd 100644 --- a/src/couch_mrview/src/couch_mrview.erl +++ b/src/couch_mrview/src/couch_mrview.erl @@ -297,6 +297,7 @@ query_all_docs(Db, Args0, Callback, Acc) -> % remove. % Args2 = couch_mrview_util:validate_all_docs_args(Db, Args1), + couch_mrview_util:check_range(Args2, fun(A, B) -> A < B end), {ok, Acc1} = case Args2#mrargs.preflight_fun of PFFun when is_function(PFFun, 2) -> PFFun(Sig, Acc); diff --git a/src/couch_mrview/src/couch_mrview_util.erl b/src/couch_mrview/src/couch_mrview_util.erl index 48138bf76f..2e7508ff8e 100644 --- a/src/couch_mrview/src/couch_mrview_util.erl +++ b/src/couch_mrview/src/couch_mrview_util.erl @@ -29,6 +29,7 @@ -export([calculate_external_size/1]). -export([calculate_active_size/1]). -export([validate_all_docs_args/2, validate_args/1, validate_args/3]). +-export([check_range/2]). -export([maybe_load_doc/3, maybe_load_doc/4]). -export([maybe_update_index_file/1]). -export([extract_view/4, extract_view_reduce/1]). diff --git a/src/couch_mrview/test/eunit/couch_mrview_all_docs_tests.erl b/src/couch_mrview/test/eunit/couch_mrview_all_docs_tests.erl index b2a01ec723..bce9538be1 100644 --- a/src/couch_mrview/test/eunit/couch_mrview_all_docs_tests.erl +++ b/src/couch_mrview/test/eunit/couch_mrview_all_docs_tests.erl @@ -54,7 +54,13 @@ all_docs_test_() -> ?TDEF_FE(http_query_with_limit_and_skip), ?TDEF_FE(http_query_with_limit_and_skip_and_query_limit), ?TDEF_FE(http_query_with_query_limit_and_over_limit), - ?TDEF_FE(http_query_with_include_docs) + ?TDEF_FE(http_query_with_include_docs), + ?TDEF_FE(should_throw_range_error_fwd), + ?TDEF_FE(should_throw_range_error_rev), + ?TDEF_FE(should_not_throw_range_error_valid_fwd), + ?TDEF_FE(should_not_throw_range_error_valid_rev), + ?TDEF_FE(http_impossible_range_fwd), + ?TDEF_FE(http_impossible_range_rev) ] } } @@ -300,6 +306,58 @@ http_query_with_include_docs(Db) -> http_query(Db, Params) ). +%% Direct Erlang API tests for check_range on _all_docs + +should_throw_range_error_fwd(Db) -> + Error = {query_parse_error, + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=true">>}, + ?assertThrow(Error, run_query(Db, [{start_key, <<"z">>}, {end_key, <<"a">>}])). + +should_throw_range_error_rev(Db) -> + Error = {query_parse_error, + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=false">>}, + ?assertThrow(Error, run_query(Db, [ + {start_key, <<"a">>}, {end_key, <<"z">>}, {direction, rev} + ])). + +should_not_throw_range_error_valid_fwd(Db) -> + {ok, _} = run_query(Db, [{start_key, <<"3">>}, {end_key, <<"5">>}]). + +should_not_throw_range_error_valid_rev(Db) -> + {ok, _} = run_query(Db, [ + {start_key, <<"5">>}, {end_key, <<"3">>}, {direction, rev} + ]). + +%% HTTP tests for check_range on _all_docs + +http_impossible_range_fwd(Db) -> + {Code, Res} = http_req(Db, "?start_key=\"z\"&end_key=\"a\""), + ?assertEqual(400, Code), + ?assertMatch( + #{ + <<"error">> := <<"query_parse_error">>, + <<"reason">> := + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=true">> + }, + Res + ). + +http_impossible_range_rev(Db) -> + {Code, Res} = http_req(Db, "?descending=true&start_key=\"a\"&end_key=\"z\""), + ?assertEqual(400, Code), + ?assertMatch( + #{ + <<"error">> := <<"query_parse_error">>, + <<"reason">> := + <<"No rows can match your key range, reverse your ", + "start_key and end_key or set descending=false">> + }, + Res + ). + db_url(Db) -> DbName = couch_db:name(Db), Addr = config:get("httpd", "bind_address", "127.0.0.1"), diff --git a/src/fabric/src/fabric.erl b/src/fabric/src/fabric.erl index 1b8434b03e..ba7c2bbd2d 100644 --- a/src/fabric/src/fabric.erl +++ b/src/fabric/src/fabric.erl @@ -480,6 +480,7 @@ all_docs(DbName, Options, Callback, Acc0, #mrargs{} = QueryArgs0) when is_function(Callback, 2) -> QueryArgs = fabric_util:validate_all_docs_args(DbName, QueryArgs0), + couch_mrview_util:check_range(QueryArgs, fun(A, B) -> A < B end), fabric_view_all_docs:go(dbname(DbName), opts(Options), QueryArgs, Callback, Acc0); %% @doc convenience function that takes a keylist rather than a record %% @equiv all_docs(DbName, Callback, Acc0, kl_to_query_args(QueryArgs))