From 5ffb4f98e0a8be09675ca508c269b71654294d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= Date: Fri, 4 Oct 2019 13:32:35 +0200 Subject: Make cowboy_compress_h add vary: accept-encoding --- src/cowboy_compress_h.erl | 24 +++++++++++++++---- test/compress_SUITE.erl | 56 ++++++++++++++++++++++++++++++++++++++++++++ test/handlers/compress_h.erl | 3 +++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/cowboy_compress_h.erl b/src/cowboy_compress_h.erl index f4ee7b9..e9824ba 100644 --- a/src/cowboy_compress_h.erl +++ b/src/cowboy_compress_h.erl @@ -176,10 +176,10 @@ gzip_response({response, Status, Headers, Body}, State) -> after zlib:close(Z) end, - {{response, Status, Headers#{ + {{response, Status, vary(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(), @@ -187,9 +187,25 @@ 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, Headers#{ + {{headers, Status, vary(Headers#{ <<"content-encoding">> => <<"gzip">> - }}, State#state{deflate=Z}}. + })}, State#state{deflate=Z}}. + +%% We must add content-encoding to vary if it's not already there. +vary(Headers=#{<<"vary">> := Vary}) -> + try cow_http_hd:parse_vary(iolist_to_binary(Vary)) of + '*' -> Headers; + List -> + case lists:member(<<"accept-encoding">>, List) of + true -> Headers; + false -> Headers#{<<"vary">> => [Vary, <<", accept-encoding">>]} + end + catch _:_ -> + %% The vary header is invalid. Probably empty. We replace it with ours. + Headers#{<<"vary">> => <<"accept-encoding">>} + end; +vary(Headers) -> + Headers#{<<"vary">> => <<"accept-encoding">>}. %% It is not possible to combine zlib and the sendfile %% syscall as far as I can tell, because the zlib format diff --git a/test/compress_SUITE.erl b/test/compress_SUITE.erl index 2fa006a..8891e42 100644 --- a/test/compress_SUITE.erl +++ b/test/compress_SUITE.erl @@ -67,6 +67,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), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -75,6 +76,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), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -84,6 +86,8 @@ gzip_reply_content_encoding(Config) -> [{<<"accept-encoding">>, <<"gzip">>}], 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), {_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -92,6 +96,7 @@ gzip_reply_large_body(Config) -> {200, Headers, GzBody} = do_get("/reply/large", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), {_, Length} = lists:keyfind(<<"content-length">>, 1, Headers), ct:log("Original length: 100000; compressed: ~s.", [Length]), _ = zlib:gunzip(GzBody), @@ -102,6 +107,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), ct:log("Body received:~n~p~n", [Body]), ok. @@ -110,6 +116,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), {_, <<"100">>} = lists:keyfind(<<"content-length">>, 1, Headers), ok. @@ -118,6 +125,7 @@ gzip_stream_reply(Config) -> {200, Headers, GzBody} = do_get("/stream_reply/large", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), _ = zlib:gunzip(GzBody), ok. @@ -126,6 +134,7 @@ gzip_stream_reply_sendfile(Config) -> {200, Headers, GzBody} = do_get("/stream_reply/sendfile", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), _ = zlib:gunzip(GzBody), ok. @@ -134,6 +143,7 @@ gzip_stream_reply_sendfile_fin(Config) -> {200, Headers, GzBody} = do_get("/stream_reply/sendfile_fin", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), _ = zlib:gunzip(GzBody), ok. @@ -142,6 +152,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), 100000 = iolist_size(Body), ok. @@ -164,6 +175,7 @@ opts_compress_buffering_false(Config0) -> [{<<"accept-encoding">>, <<"gzip">>}]), {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), Z = zlib:open(), zlib:inflateInit(Z, 31), {data, nofin, Data1} = gun:await(ConnPid, Ref, 500), @@ -195,6 +207,7 @@ opts_compress_buffering_true(Config0) -> [{<<"accept-encoding">>, <<"gzip">>}]), {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), Z = zlib:open(), zlib:inflateInit(Z, 31), %% The data gets buffered because it is too small. @@ -229,6 +242,7 @@ set_options_compress_buffering_false(Config0) -> [{<<"accept-encoding">>, <<"gzip">>}]), {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), Z = zlib:open(), zlib:inflateInit(Z, 31), {data, nofin, Data1} = gun:await(ConnPid, Ref, 500), @@ -260,6 +274,7 @@ set_options_compress_buffering_true(Config0) -> [{<<"accept-encoding">>, <<"gzip">>}]), {response, nofin, 200, Headers} = gun:await(ConnPid, Ref), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), Z = zlib:open(), zlib:inflateInit(Z, 31), %% The data gets buffered because it is too small. @@ -281,5 +296,46 @@ set_options_compress_threshold_0(Config) -> {200, Headers, GzBody} = do_get("/reply/set_options_threshold0", [{<<"accept-encoding">>, <<"gzip">>}], Config), {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), _ = zlib:gunzip(GzBody), ok. + +vary_accept(Config) -> + doc("Add accept-encoding to vary when the response has a 'vary: accept' header."), + {200, Headers, _} = do_get("/reply/vary", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-vary">>, <<"accept">>} + ], Config), + {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept, accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), + ok. + +vary_accept_accept_encoding(Config) -> + doc("Don't change the vary value when the response has a 'vary: accept, accept-encoding' header."), + {200, Headers, _} = do_get("/reply/vary", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-vary">>, <<"accept, accept-encoding">>} + ], Config), + {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept, accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), + ok. + +vary_empty(Config) -> + doc("Add accept-encoding to vary when the response has an empty vary header."), + {200, Headers, _} = do_get("/reply/vary", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-vary">>, <<>>} + ], Config), + {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"accept-encoding">>} = lists:keyfind(<<"vary">>, 1, Headers), + ok. + +vary_wildcard(Config) -> + doc("Don't change the vary value when the response has a 'vary: *' header."), + {200, Headers, _} = do_get("/reply/vary", [ + {<<"accept-encoding">>, <<"gzip">>}, + {<<"x-test-vary">>, <<"*">>} + ], Config), + {_, <<"gzip">>} = lists:keyfind(<<"content-encoding">>, 1, Headers), + {_, <<"*">>} = lists:keyfind(<<"vary">>, 1, Headers), + ok. diff --git a/test/handlers/compress_h.erl b/test/handlers/compress_h.erl index 32830d9..76c2db1 100644 --- a/test/handlers/compress_h.erl +++ b/test/handlers/compress_h.erl @@ -11,6 +11,9 @@ init(Req0, State=reply) -> cowboy_req:reply(200, #{}, lists:duplicate(100, $a), Req0); <<"large">> -> cowboy_req:reply(200, #{}, lists:duplicate(100000, $a), Req0); + <<"vary">> -> + Vary = cowboy_req:header(<<"x-test-vary">>, Req0), + cowboy_req:reply(200, #{<<"vary">> => Vary}, lists:duplicate(100000, $a), Req0); <<"over-threshold">> -> cowboy_req:reply(200, #{}, lists:duplicate(200, $a), Req0); <<"content-encoding">> -> -- cgit v1.2.3