diff --git a/src/yaws.erl b/src/yaws.erl index c9b29192..686009fa 100644 --- a/src/yaws.erl +++ b/src/yaws.erl @@ -2669,7 +2669,38 @@ do_http_get_headers(CliSock, SSL) -> {error, _}=Error -> Error; H -> - {R, H} + %% According to RFC 9112 Section 6.1 Transfer-Encoding we + %% select to process POST requests according to + %% Transfer-Encoding only while ignoring Content-Length if + %% both are supplied. It is also possible to reject these + %% requests, but that would probably introduce backward + %% incompatible behavior. + %% + %% If Transfer-Encoding is not set, we assert a valid + %% Content-Length header. Note that http_collect_headers/5 + %% might have deferred the multiple_content_length_headers + %% error to here, since we need to collect all headers + %% before we can know if transfer encoding is used. + + TE = yaws_api:headers_transfer_encoding(H), + CL = yaws_api:headers_content_length(H), + + case {TE, assert_content_length(CL)} of + {TE, _} when is_list(TE) -> + %% Ignore Content-Length, see above. + {R, H#headers{content_length = undefined}}; + {_, {error, _} = Err} -> + %% No transfer encoding so we return the deferred + %% error frow http_collect_headers/5. + Err; + {_, false} -> + {error, {invalid_content_length, + R#http_request{method = bad_request}}}; + {_, true} -> + %% No transfer encoding and valid content length, + %% proceed as normal. + {R, H} + end end end. @@ -2764,14 +2795,28 @@ http_collect_headers(CliSock, Req, H, SSL, Count) when Count < 1000 -> case H#headers.content_length of undefined -> http_collect_headers(CliSock, Req, - H#headers{content_length = X},SSL, - Count+1); + H#headers{content_length = X}, + SSL, Count+1); X -> %% duplicate header, ignore it http_collect_headers(CliSock, Req, H, SSL, Count+1); + + + %% Defer multiple content-length header error until we know if + %% transfer-encoding is used. If transfer-encoding is used we + %% ignore content-length, see RFC 9112 Section 6.1. + %% + %% We store the error in #headers.content_length and when + %% return it if transfer-encoding is not used. + {error, _} -> + %% stored content-length error, continue + http_collect_headers(CliSock, Req, H, SSL, Count+1); _ -> - {error, {multiple_content_length_headers, - Req#http_request{method=bad_request}}} + CLErr = {error, {multiple_content_length_headers, + Req#http_request{method=bad_request}}}, + http_collect_headers(CliSock, Req, + H#headers{content_length = CLErr}, + SSL, Count+1) end; {ok, {http_header, _Num, 'Content-Type', _, X}} -> http_collect_headers(CliSock, Req, @@ -2842,6 +2887,39 @@ http_collect_headers(_CliSock, Req, _H, _SSL, _Count) -> {error, {too_many_headers, Req}}. +%% @doc Assert valid Content-Length header value. +%% +%% Must be empty or non negative integer. +%% +%% If HTTP method is POST Content-Length MUST be set, if transfer-encoding +%% is not chunked. +%% +%% Note that http_collect_headers/5 can have deferred the +%% multiple_content_length_values error because all headers need to be +%% collected before it can be known if transfer encoding is used. +%% See do_http_get_headers/2. +-spec assert_content_length(string() | Error) -> boolean() | Error when + Error :: {error, {atom(), #http_request{}}}. +assert_content_length(CL) + when CL == undefined orelse CL == "" -> + true; +assert_content_length({error, {multiple_content_length_headers, + #http_request{method = bad_request}}} = Err) -> + Err; +assert_content_length(X) -> + try + X2 = erlang:list_to_integer(X), + true = (0 =< X2) + catch + error:badarg -> + %% not a number + false; + error:{badmatch, false} -> + %% not non neg integer + false + end. + + %% @doc Filter out empty accept headers from list. -spec split_accept_hdrs([string()]) -> [string()]. split_accept_hdrs([]) -> diff --git a/src/yaws_server.erl b/src/yaws_server.erl index 1fc21a41..00757925 100644 --- a/src/yaws_server.erl +++ b/src/yaws_server.erl @@ -1202,6 +1202,17 @@ aloop(CliSock, {IP,Port}=IPPort, GS, Num) -> deliver_431(CliSock, ReqTooMany, NoArg) end, {ok, Num+1}; + {error, {invalid_content_length, ReqMultiCL}} -> + ?Debug("Invalid Content-Length header content~n", []), + case pick_sconf(GS#gs.gconf, #headers{}, GS#gs.group) of + undefined -> + deliver_400(CliSock, ReqMultiCL, NoArg); + SC -> + put(sc, SC), + put(outh, #outh{}), + deliver_400(CliSock, ReqMultiCL, NoArg) + end, + {ok, Num+1}; {error, {multiple_content_length_headers, ReqMultiCL}} -> %% RFC 7230 status code 400 ?Debug("Multiple Content-Length headers~n", []), @@ -1764,6 +1775,8 @@ body_method(CliSock, IPPort, Req, Head) -> end; {_, undefined} -> <<>>; + {_, ""} -> + {error, "Length Required", fun deliver_411/3}; {_, Len} -> Int_len = strip_list_to_integer(Len), if @@ -2796,6 +2809,9 @@ deliver_405(CliSock, Req, Arg, Methods) -> "

"]), deliver_xxx(CliSock, Req, Arg, 405, Methods_msg). +deliver_411(CliSock, Req, Arg) -> + deliver_xxx(CliSock, Req, Arg, 411). % Length Required + deliver_416(CliSock, _Req, Arg, Tot) -> B = ["

416 ", yaws_api:code_to_phrase(416), "

"], Sz = iolist_size(B), @@ -4922,6 +4938,8 @@ flush(Sock, Pos, Sz, "chunked") -> end; flush(_Sock, Pos, undefined, _) -> Pos; +flush(_Sock, Pos, "", _) -> + Pos; flush(Sock, Pos, Sz, TE) when is_list(Sz) -> flush(Sock, Pos, strip_list_to_integer(Sz), TE); flush(Sock, Pos, Sz, _) -> diff --git a/testsuite/main_SUITE.erl b/testsuite/main_SUITE.erl index 395ce5c2..4b2b307f 100644 --- a/testsuite/main_SUITE.erl +++ b/testsuite/main_SUITE.erl @@ -25,6 +25,7 @@ all() -> post_multi_same_content_length, post_bogus_transfer_encoding, post_bad_order_transfer_encoding, + post_content_length_transfer_encoding, flush_small_get, flush_large_get, flush_chunked_get, @@ -54,6 +55,7 @@ all() -> exhtml, accept_ranges, status_and_content_length, + content_length_values, encoded_url, encoded_url_no_docroot_escape, header_order, @@ -388,6 +390,9 @@ post_multi_same_content_length(Config) -> ok. post_transfer_encoding(Config, TE, StatusCode) -> + post_transfer_encoding(Config, TE, "", StatusCode). + +post_transfer_encoding(Config, TE, CL, StatusCode) -> Port = testsuite:get_yaws_port(1, Config), Url = testsuite:make_url(http, "127.0.0.1", Port, "/posttest/chunked/5"), {ok, {Scheme, _, Host, Port, Path, _}} = yaws_dynopts:http_uri_parse(Url), @@ -398,10 +403,12 @@ post_transfer_encoding(Config, TE, StatusCode) -> "Host: " ++ Host ++ "\r\n", "User-Agent: Yaws HTTP client\r\n", "Transfer-Encoding: " ++ TE ++ "\r\n", + CL, %% Content-Length "\r\n", "5\r\n", "hello\r\n", - "0\r\n"], + "0\r\n", + "\r\n"], ok = gen_tcp:send(Sock, Req), ?assertMatch({ok,{{_,StatusCode,_},_,_}}, testsuite:receive_http_response(Sock)) after @@ -426,6 +433,18 @@ post_bad_order_transfer_encoding(Config) -> %% 400 (Bad Request) status code and then close the connection." post_transfer_encoding(Config, "chunked, gzip", 400). +post_content_length_transfer_encoding(Config) -> + %% RFC 9112 Section 6.1 Transfer-Encoding "A server MAY reject + %% a request that contains both Content-Length and Transfer-Encoding + %% or process such a request in accordance with the Transfer-Encoding + %% alone. Regardless, the server MUST close the connection after + %% responding to such a request to avoid the potential attacks." + post_transfer_encoding(Config, "chunked", "Content-Length: bogus\r\n", 200), + post_transfer_encoding(Config, "chunked", + "Content-Length: bogus\r\n" + "Content-Length: bogus2\r\n", + 200). + flush_small_get(Config) -> File = filename:join(?tempdir(?MODULE), "www/1000.txt"), {ok, FI} = file:read_file_info(File), @@ -924,6 +943,84 @@ status_and_content_length(Config) -> ?assertNot(proplists:is_defined("content-length", Hdrs4)), ok. +content_length_values(Config) -> + Port = testsuite:get_yaws_port(1, Config), + HostHdr = {"Host", "127.0.0.1:"++integer_to_list(Port)}, + + %% Positive tests + + CLZeroHdr = {"Content-Length", "0"}, + CLEmptyHdr1 = {"Content-Length", ""}, + CLEmptyHdr2 = {"Content-Length", " "}, + + %% GET Content-Length: 0 + {ok, Sock1} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock1, {get, "/", "HTTP/1.1"}, + [HostHdr, CLZeroHdr])), + {ok, {{_,200,_}, _}} = testsuite:receive_http_headers(Sock1), + ?assertEqual(ok, gen_tcp:close(Sock1)), + + %% GET "Content-Length: " + {ok, Sock2} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock2, {get, "/", "HTTP/1.1"}, + [HostHdr, CLEmptyHdr1])), + {ok, {{_,200,_}, _}} = testsuite:receive_http_headers(Sock2), + ?assertEqual(ok, gen_tcp:close(Sock2)), + + %% GET "Content-Length: " + {ok, Sock3} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock3, {get, "/", "HTTP/1.1"}, + [HostHdr, CLEmptyHdr2])), + {ok, {{_,200,_}, _}} = testsuite:receive_http_headers(Sock3), + ?assertEqual(ok, gen_tcp:close(Sock3)), + + %% Negative tests + + %% "Content-Length: " + %% "Content-Length: " + CLNegHdr = {"Content-Length", "-1"}, + CLNanHdr = {"Content-Length", "abc"}, + + %% POST "Content-Length: " + {ok, Sock4} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock4, {post, "/", "HTTP/1.1"}, + [HostHdr, CLEmptyHdr1])), + {ok, {{_,411,_}, _}} = testsuite:receive_http_headers(Sock4), + ?assertEqual(ok, gen_tcp:close(Sock4)), + + %% POST "Content-Length: " + {ok, Sock5} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock5, {post, "/", "HTTP/1.1"}, + [HostHdr, CLEmptyHdr2])), + {ok, {{_,411,_}, _}} = testsuite:receive_http_headers(Sock5), + ?assertEqual(ok, gen_tcp:close(Sock5)), + + %% Content-Length: -1 + {ok, Sock6} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock6, {get, "/", "HTTP/1.1"}, + [HostHdr, CLNegHdr])), + {ok, {{_,400,_}, _}} = testsuite:receive_http_headers(Sock6), + ?assertEqual(ok, gen_tcp:close(Sock6)), + {ok, Sock7} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock7, {post, "/", "HTTP/1.1"}, + [HostHdr, CLNegHdr])), + {ok, {{_,400,_}, _}} = testsuite:receive_http_headers(Sock7), + ?assertEqual(ok, gen_tcp:close(Sock7)), + + %% Content-Length: abc + {ok, Sock8} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock8, {get, "/", "HTTP/1.1"}, + [HostHdr, CLNanHdr])), + {ok, {{_,400,_}, _}} = testsuite:receive_http_headers(Sock8), + ?assertEqual(ok, gen_tcp:close(Sock8)), + {ok, Sock9} = gen_tcp:connect("127.0.0.1", Port, [binary, {active, false}]), + ?assertEqual(ok, testsuite:send_http_headers(Sock9, {post, "/", "HTTP/1.1"}, + [HostHdr, CLNanHdr])), + {ok, {{_,400,_}, _}} = testsuite:receive_http_headers(Sock9), + ?assertEqual(ok, gen_tcp:close(Sock9)), + + ok. + encoded_url(Config) -> PathInfo = "/%2F/HA", UrlPath = "/policies" ++ PathInfo,