From 9784179498cd36dd6d59fdb7174d61c6d24c98bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Mon, 8 Jan 2024 10:17:50 +0100 Subject: Always add vary: accept-encoding in cowboy_compress_h We must add it even if we don't end up compressing because it indicates that we might. This indication doesn't mean that the user agent's accept-encoding values will ever result in content encoding being applied. --- doc/src/manual/cowboy_compress_h.asciidoc | 2 ++ src/cowboy_compress_h.erl | 37 ++++++++++++++++++++++--------- test/compress_SUITE.erl | 20 ++++++++--------- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/doc/src/manual/cowboy_compress_h.asciidoc b/doc/src/manual/cowboy_compress_h.asciidoc index 048a4ac..6551567 100644 --- a/doc/src/manual/cowboy_compress_h.asciidoc +++ b/doc/src/manual/cowboy_compress_h.asciidoc @@ -57,6 +57,8 @@ The compress stream handler does not produce any event. * *2.11*: Compression is now disabled when the etag header is in the response headers. +* *2.11*: The vary: accept-encoding header is now + always set when this handler is enabled. * *2.6*: The options `compress_buffering` and `compress_threshold` were added. * *2.0*: Module introduced. diff --git a/src/cowboy_compress_h.erl b/src/cowboy_compress_h.erl index bb4a265..57d3dab 100644 --- a/src/cowboy_compress_h.erl +++ b/src/cowboy_compress_h.erl @@ -103,7 +103,7 @@ check_resp_headers(_, State) -> State. fold(Commands, State=#state{compress=undefined}) -> - {Commands, State}; + fold_vary_only(Commands, State, []); fold(Commands, State) -> fold(Commands, State, []). @@ -111,32 +111,32 @@ fold([], State, Acc) -> {lists:reverse(Acc), State}; %% We do not compress full sendfile bodies. fold([Response={response, _, _, {sendfile, _, _, _}}|Tail], State, Acc) -> - fold(Tail, State, [Response|Acc]); + fold(Tail, State, [vary_response(Response)|Acc]); %% We compress full responses directly, unless they are lower than %% the configured threshold or we find we are not able to by looking at the headers. fold([Response0={response, _, Headers, Body}|Tail], State0=#state{threshold=CompressThreshold}, Acc) -> case check_resp_headers(Headers, State0) of State=#state{compress=undefined} -> - fold(Tail, State, [Response0|Acc]); + fold(Tail, State, [vary_response(Response0)|Acc]); State1 -> BodyLength = iolist_size(Body), if BodyLength =< CompressThreshold -> - fold(Tail, State1, [Response0|Acc]); + fold(Tail, State1, [vary_response(Response0)|Acc]); true -> {Response, State} = gzip_response(Response0, State1), - fold(Tail, State, [Response|Acc]) + fold(Tail, State, [vary_response(Response)|Acc]) end end; %% Check headers and initiate compression... fold([Response0={headers, _, Headers}|Tail], State0, Acc) -> case check_resp_headers(Headers, State0) of State=#state{compress=undefined} -> - fold(Tail, State, [Response0|Acc]); + fold(Tail, State, [vary_headers(Response0)|Acc]); State1 -> {Response, State} = gzip_headers(Response0, State1), - fold(Tail, State, [Response|Acc]) + fold(Tail, State, [vary_headers(Response)|Acc]) end; %% then compress each data commands individually. fold([Data0={data, _, _}|Tail], State0=#state{compress=gzip}, Acc) -> @@ -164,6 +164,15 @@ fold([SetOptions={set_options, Opts}|Tail], State=#state{ fold([Command|Tail], State, Acc) -> fold(Tail, State, [Command|Acc]). +fold_vary_only([], State, Acc) -> + {lists:reverse(Acc), State}; +fold_vary_only([Response={response, _, _, _}|Tail], State, Acc) -> + fold_vary_only(Tail, State, [vary_response(Response)|Acc]); +fold_vary_only([Response={headers, _, _}|Tail], State, Acc) -> + fold_vary_only(Tail, State, [vary_headers(Response)|Acc]); +fold_vary_only([Command|Tail], State, Acc) -> + fold_vary_only(Tail, State, [Command|Acc]). + buffering_to_zflush(true) -> none; buffering_to_zflush(false) -> sync. @@ -183,10 +192,10 @@ gzip_response({response, Status, Headers, Body}, State) -> after zlib:close(Z) end, - {{response, Status, vary(Headers#{ + {{response, Status, Headers#{ <<"content-length">> => integer_to_binary(iolist_size(GzBody)), <<"content-encoding">> => <<"gzip">> - }), GzBody}, State}. + }, GzBody}, State}. gzip_headers({headers, Status, Headers0}, State) -> Z = zlib:open(), @@ -194,9 +203,15 @@ gzip_headers({headers, Status, Headers0}, State) -> %% @todo It might be good to allow them to be configured? zlib:deflateInit(Z, default, deflated, 31, 8, default), Headers = maps:remove(<<"content-length">>, Headers0), - {{headers, Status, vary(Headers#{ + {{headers, Status, Headers#{ <<"content-encoding">> => <<"gzip">> - })}, State#state{deflate=Z}}. + }}, State#state{deflate=Z}}. + +vary_response({response, Status, Headers, Body}) -> + {response, Status, vary(Headers), Body}. + +vary_headers({headers, Status, Headers}) -> + {headers, Status, vary(Headers)}. %% We must add content-encoding to vary if it's not already there. vary(Headers=#{<<"vary">> := Vary}) -> diff --git a/test/compress_SUITE.erl b/test/compress_SUITE.erl index 80f588d..75f6f5b 100644 --- a/test/compress_SUITE.erl +++ b/test/compress_SUITE.erl @@ -67,7 +67,7 @@ gzip_accept_encoding_malformed(Config) -> {200, Headers, _} = do_get("/reply/large", [{<<"accept-encoding">>, <<";">>}], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -76,7 +76,7 @@ gzip_accept_encoding_missing(Config) -> {200, Headers, _} = do_get("/reply/large", [], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -85,7 +85,7 @@ gzip_accept_encoding_no_gzip(Config) -> {200, Headers, _} = do_get("/reply/large", [{<<"accept-encoding">>, <<"compress">>}], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -94,7 +94,7 @@ gzip_accept_encoding_not_supported(Config) -> {200, Headers, _} = do_get("/reply/large", [{<<"accept-encoding">>, <<"application/gzip">>}], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -105,7 +105,7 @@ gzip_reply_content_encoding(Config) -> %% We set the content-encoding to compress; without actually compressing. {_, <<"compress">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), %% The reply didn't include a vary header. - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -116,7 +116,7 @@ gzip_reply_etag(Config) -> %% We set a strong etag. {_, <<"\"STRONK\"">>} = lists:keyfind(<<"etag">>, 1, Headers), %% The reply didn't include a vary header. - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -136,7 +136,7 @@ gzip_reply_sendfile(Config) -> {200, Headers, Body} = do_get("/reply/sendfile", [{<<"accept-encoding">>, <<"gzip">>}], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), ct:log("Body received:~n~p~n", [Body]), ok. @@ -145,7 +145,7 @@ gzip_reply_small_body(Config) -> {200, Headers, _} = do_get("/reply/small", [{<<"accept-encoding">>, <<"gzip">>}], Config), false = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, <<"100">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -181,7 +181,7 @@ gzip_stream_reply_content_encoding(Config) -> {200, Headers, Body} = do_get("/stream_reply/content-encoding", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"compress">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), 100000 = iolist_size(Body), ok. @@ -190,7 +190,7 @@ gzip_stream_reply_etag(Config) -> {200, Headers, Body} = do_get("/stream_reply/etag", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"\"STRONK\"">>} = lists:keyfind(<<"etag">>, 1, Headers), - false = lists:keyfind(<<"vary">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), 100000 = iolist_size(Body), ok. -- cgit v1.2.3