From 308045fd67d0b37475f924e35a434a8642307cc2 Mon Sep 17 00:00:00 2001 From: geeksilva97 Date: Tue, 9 Jan 2024 16:45:54 -0300 Subject: Reject responses with explicit set-cookie header LH: The tests received a lot of fixes and tweaking. I also reworded the error message to be more concise. --- src/cowboy_req.erl | 18 ++++++++++++++++++ test/handlers/resp_h.erl | 32 ++++++++++++++++++++++++++++++++ test/req_SUITE.erl | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) diff --git a/src/cowboy_req.erl b/src/cowboy_req.erl index 840d349..8edf4ff 100644 --- a/src/cowboy_req.erl +++ b/src/cowboy_req.erl @@ -718,6 +718,9 @@ set_resp_cookie(Name, Value, Req, Opts) -> -spec set_resp_header(binary(), iodata(), Req) -> Req when Req::req(). +set_resp_header(<<"set-cookie">>, _, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); set_resp_header(Name, Value, Req=#{resp_headers := RespHeaders}) -> Req#{resp_headers => RespHeaders#{Name => Value}}; set_resp_header(Name,Value, Req) -> @@ -725,6 +728,9 @@ set_resp_header(Name,Value, Req) -> -spec set_resp_headers(cowboy:http_headers(), Req) -> Req when Req::req(). +set_resp_headers(#{<<"set-cookie">> := _}, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); set_resp_headers(Headers, Req=#{resp_headers := RespHeaders}) -> Req#{resp_headers => maps:merge(RespHeaders, Headers)}; set_resp_headers(Headers, Req) -> @@ -781,6 +787,9 @@ inform(Status, Req) -> inform(_, _, #{has_sent_resp := _}) -> exit({response_error, response_already_sent, 'The final response has already been sent.'}); +inform(_, #{<<"set-cookie">> := _}, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); inform(Status, Headers, Req) when is_integer(Status); is_binary(Status) -> cast({inform, Status, Headers}, Req). @@ -800,6 +809,9 @@ reply(Status, Headers, Req) -> reply(_, _, _, #{has_sent_resp := _}) -> exit({response_error, response_already_sent, 'The final response has already been sent.'}); +reply(_, #{<<"set-cookie">> := _}, _, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); reply(Status, Headers, {sendfile, _, 0, _}, Req) when is_integer(Status); is_binary(Status) -> do_reply(Status, Headers#{ @@ -857,6 +869,9 @@ stream_reply(Status, Req) -> stream_reply(_, _, #{has_sent_resp := _}) -> exit({response_error, response_already_sent, 'The final response has already been sent.'}); +stream_reply(_, #{<<"set-cookie">> := _}, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); %% 204 and 304 responses must NOT send a body. We therefore %% transform the call to a full response and expect the user %% to NOT call stream_body/3 afterwards. (RFC7230 3.3) @@ -908,6 +923,9 @@ stream_events(Events, IsFin, Req=#{has_sent_resp := headers}) -> stream_body({data, self(), IsFin, cow_sse:events(Events)}, Req). -spec stream_trailers(cowboy:http_headers(), req()) -> ok. +stream_trailers(#{<<"set-cookie">> := _}, _) -> + exit({response_error, invalid_header, + 'Response cookies must be set using cowboy_req:set_resp_cookie/3,4.'}); stream_trailers(Trailers, Req=#{has_sent_resp := headers}) -> cast({trailers, Trailers}, Req). diff --git a/test/handlers/resp_h.erl b/test/handlers/resp_h.erl index db3844b..aae9eb9 100644 --- a/test/handlers/resp_h.erl +++ b/test/handlers/resp_h.erl @@ -30,6 +30,10 @@ do(<<"set_resp_cookie4">>, Req0, Opts) -> do(<<"set_resp_header">>, Req0, Opts) -> Req = cowboy_req:set_resp_header(<<"content-type">>, <<"text/plain">>, Req0), {ok, cowboy_req:reply(200, #{}, "OK", Req), Opts}; +do(<<"set_resp_header_cookie">>, Req0, Opts) -> + ct_helper:ignore(cowboy_req, set_resp_header, 3), + Req = cowboy_req:set_resp_header(<<"set-cookie">>, <<"name=value">>, Req0), + {ok, cowboy_req:reply(200, #{}, "OK", Req), Opts}; do(<<"set_resp_header_server">>, Req0, Opts) -> Req = cowboy_req:set_resp_header(<<"server">>, <<"nginx">>, Req0), {ok, cowboy_req:reply(200, #{}, "OK", Req), Opts}; @@ -39,6 +43,12 @@ do(<<"set_resp_headers">>, Req0, Opts) -> <<"content-encoding">> => <<"compress">> }, Req0), {ok, cowboy_req:reply(200, #{}, "OK", Req), Opts}; +do(<<"set_resp_headers_cookie">>, Req0, Opts) -> + ct_helper:ignore(cowboy_req, set_resp_headers, 2), + Req = cowboy_req:set_resp_headers(#{ + <<"set-cookie">> => <<"name=value">> + }, Req0), + {ok, cowboy_req:reply(200, #{}, "OK", Req), Opts}; do(<<"set_resp_headers_http11">>, Req0, Opts) -> Req = cowboy_req:set_resp_headers(#{ <<"connection">> => <<"custom-header, close">>, @@ -147,6 +157,9 @@ do(<<"inform3">>, Req0, Opts) -> <<"error">> -> ct_helper:ignore(cowboy_req, inform, 3), cowboy_req:inform(ok, Headers, Req0); + <<"set_cookie">> -> + ct_helper:ignore(cowboy_req, inform, 3), + cowboy_req:inform(102, #{<<"set-cookie">> => <<"name=value">>}, Req0); <<"twice">> -> cowboy_req:inform(102, Headers, Req0), cowboy_req:inform(102, Headers, Req0); @@ -179,6 +192,9 @@ do(<<"reply3">>, Req0, Opts) -> <<"error">> -> ct_helper:ignore(cowboy_req, reply, 4), cowboy_req:reply(200, ok, Req0); + <<"set_cookie">> -> + ct_helper:ignore(cowboy_req, reply, 4), + cowboy_req:reply(200, #{<<"set-cookie">> => <<"name=value">>}, Req0); Status -> cowboy_req:reply(binary_to_integer(Status), #{<<"content-type">> => <<"text/plain">>}, Req0) @@ -189,6 +205,9 @@ do(<<"reply4">>, Req0, Opts) -> <<"error">> -> ct_helper:ignore(erlang, iolist_size, 1), cowboy_req:reply(200, #{}, ok, Req0); + <<"set_cookie">> -> + ct_helper:ignore(cowboy_req, reply, 4), + cowboy_req:reply(200, #{<<"set-cookie">> => <<"name=value">>}, <<"OK">>, Req0); <<"204body">> -> ct_helper:ignore(cowboy_req, do_reply_ensure_no_body, 4), cowboy_req:reply(204, #{}, <<"OK">>, Req0); @@ -240,6 +259,9 @@ do(<<"stream_reply3">>, Req0, Opts) -> <<"error">> -> ct_helper:ignore(cowboy_req, stream_reply, 3), cowboy_req:stream_reply(200, ok, Req0); + <<"set_cookie">> -> + ct_helper:ignore(cowboy_req, stream_reply, 3), + cowboy_req:stream_reply(200, #{<<"set-cookie">> => <<"name=value">>}, Req0); Status -> cowboy_req:stream_reply(binary_to_integer(Status), #{<<"content-type">> => <<"text/plain">>}, Req0) @@ -395,6 +417,16 @@ do(<<"stream_trailers">>, Req0, Opts) -> <<"grpc-status">> => <<"0">> }, Req), {ok, Req, Opts}; + <<"set_cookie">> -> + ct_helper:ignore(cowboy_req, stream_trailers, 2), + Req = cowboy_req:stream_reply(200, #{ + <<"trailer">> => <<"set-cookie">> + }, Req0), + cowboy_req:stream_body(<<"Hello world!">>, nofin, Req), + cowboy_req:stream_trailers(#{ + <<"set-cookie">> => <<"name=value">> + }, Req), + {ok, Req, Opts}; _ -> Req = cowboy_req:stream_reply(200, #{ <<"trailer">> => <<"grpc-status">> diff --git a/test/req_SUITE.erl b/test/req_SUITE.erl index 4bf2739..f407853 100644 --- a/test/req_SUITE.erl +++ b/test/req_SUITE.erl @@ -809,6 +809,8 @@ set_resp_header(Config) -> doc("Response using set_resp_header."), {200, Headers, <<"OK">>} = do_get("/resp/set_resp_header", Config), true = lists:keymember(<<"content-type">>, 1, Headers), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _, _} = do_get("/resp/set_resp_header_cookie", Config), ok. set_resp_headers(Config) -> @@ -816,6 +818,8 @@ set_resp_headers(Config) -> {200, Headers, <<"OK">>} = do_get("/resp/set_resp_headers", Config), true = lists:keymember(<<"content-type">>, 1, Headers), true = lists:keymember(<<"content-encoding">>, 1, Headers), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _, _} = do_get("/resp/set_resp_headers_cookie", Config), ok. resp_header(Config) -> @@ -893,6 +897,8 @@ inform3(Config) -> {102, Headers, 200, _, _} = do_get_inform("/resp/inform3/102", Config), {102, Headers, 200, _, _} = do_get_inform("/resp/inform3/binary", Config), {500, _} = do_get_inform("/resp/inform3/error", Config), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _} = do_get_inform("/resp/inform3/set_cookie", Config), {102, Headers, 200, _, _} = do_get_inform("/resp/inform3/twice", Config), %% @todo How to test this properly? This isn't enough. {200, _} = do_get_inform("/resp/inform3/after_reply", Config), @@ -918,6 +924,8 @@ reply3(Config) -> {404, Headers3, _} = do_get("/resp/reply3/404", Config), true = lists:keymember(<<"content-type">>, 1, Headers3), {500, _, _} = do_get("/resp/reply3/error", Config), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _, _} = do_get("/resp/reply3/set_cookie", Config), ok. reply4(Config) -> @@ -926,6 +934,8 @@ reply4(Config) -> {201, _, <<"OK">>} = do_get("/resp/reply4/201", Config), {404, _, <<"OK">>} = do_get("/resp/reply4/404", Config), {500, _, _} = do_get("/resp/reply4/error", Config), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _, _} = do_get("/resp/reply4/set_cookie", Config), ok. stream_reply2(Config) -> @@ -975,6 +985,8 @@ stream_reply3(Config) -> {404, Headers3, Body} = do_get("/resp/stream_reply3/404", Config), true = lists:keymember(<<"content-type">>, 1, Headers3), {500, _, _} = do_get("/resp/stream_reply3/error", Config), + %% The set-cookie header is special. set_resp_cookie must be used. + {500, _, _} = do_get("/resp/stream_reply3/set_cookie", Config), ok. stream_body_fin0(Config) -> @@ -1154,6 +1166,27 @@ stream_trailers_no_te(Config) -> <<"Hello world!">> = do_decode(RespHeaders, RespBody), gun:close(ConnPid). +stream_trailers_set_cookie(Config) -> + doc("Trying to send set-cookie in trailers should result in a crash."), + ConnPid = gun_open(Config), + Ref = gun:get(ConnPid, "/resp/stream_trailers/set_cookie", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"te">>, <<"trailers">>} + ]), + {response, nofin, 200, _} = gun:await(ConnPid, Ref, infinity), + case config(protocol, Config) of + http -> + %% Trailers are not sent because of the stream error. + {ok, _Body} = gun:await_body(ConnPid, Ref, infinity), + {error, timeout} = gun:await_body(ConnPid, Ref, 1000), + ok; + http2 -> + {error, {stream_error, {stream_error, internal_error, _}}} + = gun:await_body(ConnPid, Ref, infinity), + ok + end, + gun:close(ConnPid). + do_trailers(Path, Config) -> ConnPid = gun_open(Config), Ref = gun:get(ConnPid, Path, [ -- cgit v1.2.3