Skip to content

Commit

Permalink
MB-60389: Don't use uri_string:parse.
Browse files Browse the repository at this point in the history
It doesn't allow '{', '[', '"' among others.
For a complete list, see uri_string:allowed_characters().

To ensure backward compatibility, continue to allow those
characters.

Continue to normalize the paths (introduced in MB-59791).
(Note that the issue in MB-59791 is not seen without parse()).

Catch the original issue in MB-58884 where the request doesn't
fit the format '/' ++ X.. this isn't exhaustive by any means.

Change-Id: I255f5ac161be96f9bacae532d8c770982def3477
Reviewed-on: https://review.couchbase.org/c/ns_server/+/203847
Tested-by: Neelima Premsankar <neelima.premsankar@couchbase.com>
Well-Formed: Build Bot <build@couchbase.com>
Well-Formed: Restriction Checker
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Timofey Barmin <timofey.barmin@couchbase.com>
  • Loading branch information
neelima32 committed Jan 17, 2024
1 parent e3dbcaa commit 030738b
Showing 1 changed file with 13 additions and 17 deletions.
30 changes: 13 additions & 17 deletions src/menelaus_web.erl
Expand Up @@ -158,17 +158,13 @@ webconfig() ->
webconfig(Prop) ->
proplists:get_value(Prop, webconfig()).

parse_path_uri(RawPath) ->
RawPathSingleSlash = lists:flatten(mochiweb_util:normalize_path(RawPath)),
case uri_string:parse(RawPathSingleSlash) of
#{path := "/" ++ P} -> P;
#{} ->
?log_debug("Invalid path in: ~p", [RawPath]),
menelaus_util:web_exception(400, "Bad Request");
{error, _, _} = Error ->
?log_debug("Invalid uri in http request: ~p, "
"error: ~p", [RawPath, Error]),
menelaus_util:web_exception(400, "Bad Request")
parse_path(RawPath) ->
RawPathSingleSlash = lists:flatten(mochiweb_util:normalize_path(RawPath)),
case RawPathSingleSlash of
"/" ++ RawPathStripped ->
{Path, _, _} = mochiweb_util:urlsplit_path(RawPathStripped),
Path;
_ -> menelaus_util:web_exception(400, "Bad Request")
end.

loop(Req0, Config) ->
Expand All @@ -184,7 +180,7 @@ loop(Req0, Config) ->
%% handed correctly, in that we delay converting %2F's to slash
%% characters until after we split by slashes.
RawPath = mochiweb_request:get(raw_path, Req),
Path = parse_path_uri(RawPath),
Path = parse_path(RawPath),
PathTokens = lists:map(fun mochiweb_util:unquote/1,
string:tokens(Path, "/")),
request_tracker:request(
Expand Down Expand Up @@ -1347,12 +1343,12 @@ response_time_ms(Req) ->
-ifdef(TEST).
parse_http_path_uri_test() ->
?assertEqual("fakePrefix/diag/eval/",
parse_path_uri("//fakePrefix/diag/eval/")),
parse_path("//fakePrefix/diag/eval/")),
?assertEqual("fakePrefix/diag/eval/",
parse_path_uri("///////fakePrefix/diag/eval/")),
?assertEqual("fake/path", parse_path_uri("/fake/path")),
parse_path("///////fakePrefix/diag/eval/")),
?assertEqual("fake/path", parse_path("/fake/path")),
?assertEqual({web_exception, 400, "Bad Request", []},
catch(parse_path_uri(""))),
catch(parse_path(""))),
?assertEqual({web_exception, 400, "Bad Request", []},
catch(parse_path_uri("\\/\/"))).
catch(parse_path("\\/\/"))).
-endif.

0 comments on commit 030738b

Please sign in to comment.