From 5b2f600036145653c48a7e8a60853e4a0ecc770b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Hoguin?= <essen@ninenines.eu>
Date: Fri, 5 Jan 2024 15:53:42 +0100
Subject: Don't automatically compress when response has etag

In the cowboy_compress_h stream handler.

Otherwise this could cause issues with caching, with the
etag being the same for compressed/uncompressed content.

Users that wish to send etags AND compress will have to
do it manually for the time being.
---
 doc/src/manual/cowboy_compress_h.asciidoc |  4 +++-
 src/cowboy_compress_h.erl                 |  3 +++
 test/compress_SUITE.erl                   | 20 ++++++++++++++++++++
 test/handlers/compress_h.erl              |  5 +++++
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/doc/src/manual/cowboy_compress_h.asciidoc b/doc/src/manual/cowboy_compress_h.asciidoc
index f6a45a9..048a4ac 100644
--- a/doc/src/manual/cowboy_compress_h.asciidoc
+++ b/doc/src/manual/cowboy_compress_h.asciidoc
@@ -9,7 +9,7 @@ cowboy_compress_h - Compress stream handler
 The module `cowboy_compress_h` compresses response bodies
 automatically when the client supports it. It will not
 try to compress responses that already have a content
-encoding.
+encoding or that have an etag header defined.
 
 Normal responses will only be compressed when their
 size is lower than the configured threshold. Streamed
@@ -55,6 +55,8 @@ The compress stream handler does not produce any event.
 
 == Changelog
 
+* *2.11*: Compression is now disabled when the etag
+  header is in the response headers.
 * *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 374cb6a..bb4a265 100644
--- a/src/cowboy_compress_h.erl
+++ b/src/cowboy_compress_h.erl
@@ -96,6 +96,9 @@ check_req(Req) ->
 %% Do not compress responses that contain the content-encoding header.
 check_resp_headers(#{<<"content-encoding">> := _}, State) ->
 	State#state{compress=undefined};
+%% Do not compress responses that contain the etag header.
+check_resp_headers(#{<<"etag">> := _}, State) ->
+	State#state{compress=undefined};
 check_resp_headers(_, State) ->
 	State.
 
diff --git a/test/compress_SUITE.erl b/test/compress_SUITE.erl
index a25c427..80f588d 100644
--- a/test/compress_SUITE.erl
+++ b/test/compress_SUITE.erl
@@ -109,6 +109,17 @@ gzip_reply_content_encoding(Config) ->
 	{_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers),
 	ok.
 
+gzip_reply_etag(Config) ->
+	doc("Reply with etag header; get an uncompressed response."),
+	{200, Headers, _} = do_get("/reply/etag",
+		[{<<"accept-encoding">>, <<"gzip">>}], 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),
+	{_, <<"100000">>} = lists:keyfind(<<"content-length">>, 1, Headers),
+	ok.
+
 gzip_reply_large_body(Config) ->
 	doc("Reply a large body; get a gzipped response."),
 	{200, Headers, GzBody} = do_get("/reply/large",
@@ -174,6 +185,15 @@ gzip_stream_reply_content_encoding(Config) ->
 	100000 = iolist_size(Body),
 	ok.
 
+gzip_stream_reply_etag(Config) ->
+	doc("Stream reply with etag header; get an uncompressed response."),
+	{200, Headers, Body} = do_get("/stream_reply/etag",
+		[{<<"accept-encoding">>, <<"gzip">>}], Config),
+	{_, <<"\"STRONK\"">>} = lists:keyfind(<<"etag">>, 1, Headers),
+	false = lists:keyfind(<<"vary">>, 1, Headers),
+	100000 = iolist_size(Body),
+	ok.
+
 opts_compress_buffering_false(Config0) ->
 	doc("Confirm that the compress_buffering option can be set to false, "
 		"which is the default."),
diff --git a/test/handlers/compress_h.erl b/test/handlers/compress_h.erl
index 27edbd3..658c834 100644
--- a/test/handlers/compress_h.erl
+++ b/test/handlers/compress_h.erl
@@ -19,6 +19,9 @@ init(Req0, State=reply) ->
 		<<"content-encoding">> ->
 			cowboy_req:reply(200, #{<<"content-encoding">> => <<"compress">>},
 				lists:duplicate(100000, $a), Req0);
+		<<"etag">> ->
+			cowboy_req:reply(200, #{<<"etag">> => <<"\"STRONK\"">>},
+				lists:duplicate(100000, $a), Req0);
 		<<"sendfile">> ->
 			AppFile = code:where_is_file("cowboy.app"),
 			Size = filelib:file_size(AppFile),
@@ -34,6 +37,8 @@ init(Req0, State=stream_reply) ->
 			stream_reply(#{}, Req0);
 		<<"content-encoding">> ->
 			stream_reply(#{<<"content-encoding">> => <<"compress">>}, Req0);
+		<<"etag">> ->
+			stream_reply(#{<<"etag">> => <<"\"STRONK\"">>}, Req0);
 		<<"sendfile">> ->
 			Data = lists:duplicate(10000, $a),
 			AppFile = code:where_is_file("cowboy.app"),
-- 
cgit v1.2.3