From a371437a90fb4ef514e7230a92b851b9da9137a4 Mon Sep 17 00:00:00 2001 From: yaxit24 Date: Wed, 1 Apr 2026 00:53:38 +0530 Subject: [PATCH] Fix #5866: Error on inverted range for _all_docs When a _view request has an impossible start_key/end_key combination, CouchDB returns a 400 Bad Request with query_parse_error. This adds the same validation to _all_docs. Uses check_range function in couch_mrview_util. Added tests spanning both the HTTP API clustered path and direct Erlang MRView API. --- src/chttpd/test/eunit/chttpd_view_test.erl | 48 ++++++++++++++- src/couch_mrview/src/couch_mrview.erl | 1 + src/couch_mrview/src/couch_mrview_util.erl | 1 + .../eunit/couch_mrview_all_docs_tests.erl | 60 ++++++++++++++++++- src/fabric/src/fabric.erl | 1 + 5 files changed, 107 insertions(+), 4 deletions(-) diff --git a/src/chttpd/test/eunit/chttpd_view_test.erl b/src/chttpd/test/eunit/chttpd_view_test.erl index beb71f82315..42c5c07cbe5 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 244f668af03..58b4342cfde 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 48138bf76f8..2e7508ff8e4 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 b2a01ec723b..bce9538be19 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 1b8434b03e7..ba7c2bbd2db 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))