Willy,
read the cover letter of this thread before ignoring the first patch, just
because this one has a higher version number to avoid mistakes.
Apply with `git am --scissors` to automatically cut the commit message.
-- >8 --
Make HAProxy set the `Vary: Accept-Encoding` response header if the server
response would normally be compressed based off the current configuration.
Specifically make sure to:
1. Disregard the *request* headers ...
2. Disregard the current compression rate and other temporary conditions ...
... when determining whether the `Vary` header must be set.
The *response* headers generated by the upstream are allowed to be taken
into account. This patch *does not* though. Once compression is enabled
the `Vary` header will be set for *all* upstream responses.
A small issue remains: The User-Agent is not added to the `Vary` header,
despite being relevant to the response. Adding the User-Agent header would
make responses effectively uncacheable and it's unlikely to see a Mozilla/4
in the wild in 2019.
Add a reg-test to ensure this behaviour.
---
reg-tests/compression/vary.vtc | 187 +++++++++++++++++++++++++++++++++
src/flt_http_comp.c | 30 +++++-
2 files changed, 213 insertions(+), 4 deletions(-)
create mode 100644 reg-tests/compression/vary.vtc
diff --git a/reg-tests/compression/vary.vtc b/reg-tests/compression/vary.vtc
new file mode 100644
index 000000000..2f5d82d73
--- /dev/null
+++ b/reg-tests/compression/vary.vtc
@@ -0,0 +1,187 @@
+varnishtest "Compression sets Vary header"
+
+#REQUIRE_VERSION=1.9
+#REQUIRE_OPTION=ZLIB|SLZ
+
+feature ignore_unknown_macro
+
+server s1 {
+ rxreq
+ expect req.url == "/plain/accept-encoding-gzip"
+ expect req.http.accept-encoding == "gzip"
+ txresp \
+ -hdr "Content-Type: text/plain" \
+ -bodylen 100
+
+ rxreq
+ expect req.url == "/plain/accept-encoding-invalid"
+ expect req.http.accept-encoding == "invalid"
+ txresp \
+ -hdr "Content-Type: text/plain" \
+ -bodylen 100
+
+ rxreq
+ expect req.url == "/plain/accept-encoding-null"
+ expect req.http.accept-encoding == "<undef>"
+ txresp \
+ -hdr "Content-Type: text/plain" \
+ -bodylen 100
+
+ rxreq
+ expect req.url == "/html/accept-encoding-gzip"
+ expect req.http.accept-encoding == "gzip"
+ txresp \
+ -hdr "Content-Type: text/html" \
+ -bodylen 100
+
+ rxreq
+ expect req.url == "/html/accept-encoding-invalid"
+ expect req.http.accept-encoding == "invalid"
+ txresp \
+ -hdr "Content-Type: text/html" \
+ -bodylen 100
+
+
+ rxreq
+ expect req.url == "/html/accept-encoding-null"
+ expect req.http.accept-encoding == "<undef>"
+ txresp \
+ -hdr "Content-Type: text/html" \
+ -bodylen 100
+
+ rxreq
+ expect req.url == "/dup-etag/accept-encoding-gzip"
+ expect req.http.accept-encoding == "gzip"
+ txresp \
+ -hdr "Content-Type: text/plain" \
+ -hdr "ETag: \"123\"" \
+ -hdr "ETag: \"123\"" \
+ -bodylen 100
+} -repeat 2 -start
+
+
+haproxy h1 -conf {
+ defaults
+ mode http
+ ${no-htx} option http-use-htx
+ timeout connect 1s
+ timeout client 1s
+ timeout server 1s
+
+ frontend fe-gzip
+ bind "fd@${fe_gzip}"
+ default_backend be-gzip
+
+ backend be-gzip
+ compression algo gzip
+ compression type text/plain
+ server www ${s1_addr}:${s1_port}
+
+ frontend fe-nothing
+ bind "fd@${fe_nothing}"
+ default_backend be-nothing
+
+ backend be-nothing
+ server www ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_fe_gzip_sock} {
+ txreq -url "/plain/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.content-encoding == "gzip"
+ expect resp.http.vary == "Accept-Encoding"
+ gunzip
+ expect resp.bodylen == 100
+
+ txreq -url "/plain/accept-encoding-invalid" \
+ -hdr "Accept-Encoding: invalid"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+
+ txreq -url "/plain/accept-encoding-null"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-invalid" \
+ -hdr "Accept-Encoding: invalid"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-null"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+
+ txreq -url "/dup-etag/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "Accept-Encoding"
+ expect resp.bodylen == 100
+} -run
+
+# This Client duplicates c1, against the "nothing" frontend, ensuring no Vary
header is ever set.
+client c2 -connect ${h1_fe_nothing_sock} {
+ txreq -url "/plain/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/plain/accept-encoding-invalid" \
+ -hdr "Accept-Encoding: invalid"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/plain/accept-encoding-null"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-invalid" \
+ -hdr "Accept-Encoding: invalid"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/html/accept-encoding-null"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+
+ txreq -url "/dup-etag/accept-encoding-gzip" \
+ -hdr "Accept-Encoding: gzip"
+ rxresp
+ expect resp.status == 200
+ expect resp.http.vary == "<undef>"
+ expect resp.bodylen == 100
+} -run
diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index b04dcd145..d513257b7 100644
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -160,11 +160,11 @@ comp_http_headers(struct stream *s, struct filter
*filter, struct http_msg *msg)
if (!(msg->chn->flags & CF_ISRESP))
select_compression_request_header(st, s, msg);
else {
+ if (!set_compression_response_header(st, s, msg))
+ goto end;
/* Response headers have already been checked in
* comp_http_post_analyze callback. */
if (st->comp_algo) {
- if (!set_compression_response_header(st, s, msg))
- goto end;
register_data_filter(s, msg->chn, filter);
if (!IS_HTX_STRM(s))
st->hdrs_len = s->txn->rsp.sov;
@@ -476,6 +476,16 @@ http_set_comp_reshdr(struct comp_state *st, struct stream
*s, struct http_msg *m
struct http_txn *txn = s->txn;
struct hdr_ctx ctx;
+ if (http_header_add_tail2(msg, &txn->hdr_idx, "Vary: Accept-Encoding",
21) < 0)
+ goto error;
+
+ /*
+ * Break if no compression is actually happening.
+ */
+ if (!st->comp_algo) {
+ return 1;
+ }
+
/*
* Add Content-Encoding header when it's not identity encoding.
* RFC 2616 : Identity encoding: This content-coding is used only in
the
@@ -526,7 +536,8 @@ http_set_comp_reshdr(struct comp_state *st, struct stream
*s, struct http_msg *m
return 1;
error:
- st->comp_algo->end(&st->comp_ctx);
+ if (st->comp_algo)
+ st->comp_algo->end(&st->comp_ctx);
st->comp_algo = NULL;
return 0;
}
@@ -537,6 +548,16 @@ htx_set_comp_reshdr(struct comp_state *st, struct stream
*s, struct http_msg *ms
struct htx *htx = htxbuf(&msg->chn->buf);
struct http_hdr_ctx ctx;
+ if (!http_add_header(htx, ist("Vary"), ist("Accept-Encoding")))
+ goto error;
+
+ /*
+ * Break if no compression is actually happening.
+ */
+ if (!st->comp_algo) {
+ return 1;
+ }
+
/*
* Add Content-Encoding header when it's not identity encoding.
* RFC 2616 : Identity encoding: This content-coding is used only in the
@@ -580,7 +601,8 @@ htx_set_comp_reshdr(struct comp_state *st, struct stream
*s, struct http_msg *ms
return 1;
error:
- st->comp_algo->end(&st->comp_ctx);
+ if (st->comp_algo)
+ st->comp_algo->end(&st->comp_ctx);
st->comp_algo = NULL;
return 0;
}
--
2.21.0
