MINOR: compression: Improve the way Vary header is added

When a message is compressed, A "Vary" header is added with
"accept-encoding" value. However, a new header is always added, regardless
there is already a Vary header or not. In addition, if there is already a
Vary header, there is no check on values to be sure "accept-encoding" value
is not already there. So it is possible to have it twice.

To improve this part, we now test Vary header values and "accept-encoding"
is only added if it was not found. In addition, "accept-encoding" value is
appended to the last Vary header found, if any. Otherwise, a new header is
added.

diff --git a/reg-tests/compression/vary.vtc b/reg-tests/compression/vary.vtc
index 34d8976c09c2b29fecdda5a7e05efd79738497be..489de30f64f62f974c9eb2d43b3ce388854fa49f 100644 (file)
--- a/reg-tests/compression/vary.vtc
+++ b/reg-tests/compression/vary.vtc
@@ -59,6 +59,56 @@ server s1 {
 } -repeat 2 -start
 
 
+server s2 {
+        rxreq
+        expect req.url == "/vary/no-vary"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -bodylen 100
+
+        rxreq
+        expect req.url == "/vary/accept-encoding"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -hdr "Vary: Accept-Encoding" \
+          -bodylen 100
+
+        rxreq
+        expect req.url == "/vary/other"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -hdr "Vary: Other" \
+          -bodylen 100
+
+        rxreq
+        expect req.url == "/vary/accept-encoding-and-other"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -hdr "Vary: Accept-Encoding,Other" \
+          -bodylen 100
+
+        rxreq
+        expect req.url == "/vary/other-and-accept-encoding"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -hdr "Vary: Other,Accept-Encoding" \
+          -bodylen 100
+
+        rxreq
+        expect req.url == "/vary/empty"
+        expect req.http.accept-encoding == "gzip"
+        txresp \
+          -hdr "Content-Type: text/plain" \
+          -hdr "Vary: " \
+          -bodylen 100
+} -start
+
+
 haproxy h1 -conf {
     global
         # WT: limit false-positives causing "HTTP header incomplete" due to
@@ -87,6 +137,16 @@ haproxy h1 -conf {
 
     backend be-nothing
         server www ${s1_addr}:${s1_port}
+
+    frontend fe-vary
+        bind "fd@${fe_vary}"
+        default_backend be-vary
+
+    backend be-vary
+        compression algo gzip
+        compression type text/plain
+        server www ${s2_addr}:${s2_port}
+
 } -start
 
 client c1 -connect ${h1_fe_gzip_sock} {
@@ -189,3 +249,60 @@ client c2 -connect ${h1_fe_nothing_sock} {
         expect resp.http.vary == "<undef>"
         expect resp.bodylen == 100
 } -run
+
+
+client c3 -connect ${h1_fe_vary_sock} {
+        txreq -url "/vary/no-vary" \
+          -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 "/vary/accept-encoding" \
+          -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 "/vary/other" \
+          -hdr "Accept-Encoding: gzip"
+        rxresp
+        expect resp.status == 200
+        expect resp.http.content-encoding == "gzip"
+        expect resp.http.vary == "Other,Accept-Encoding"
+        gunzip
+        expect resp.bodylen == 100
+
+        txreq -url "/vary/accept-encoding-and-other" \
+          -hdr "Accept-Encoding: gzip"
+        rxresp
+        expect resp.status == 200
+        expect resp.http.content-encoding == "gzip"
+        expect resp.http.vary == "Accept-Encoding,Other"
+        gunzip
+        expect resp.bodylen == 100
+
+        txreq -url "/vary/other-and-accept-encoding" \
+          -hdr "Accept-Encoding: gzip"
+        rxresp
+        expect resp.status == 200
+        expect resp.http.content-encoding == "gzip"
+        expect resp.http.vary == "Other,Accept-Encoding"
+        gunzip
+        expect resp.bodylen == 100
+
+        txreq -url "/vary/empty" \
+          -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
+} -run

diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 3b939e1890f9c75800925dae6cd82a11aad49253..d34b56a86de8e3ebb0ba4c41fd6dee8ca62018d1 100644 (file)
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -408,7 +408,7 @@ set_compression_header(struct comp_state *st, struct stream *s, struct http_msg
 {
        struct htx *htx = htxbuf(&msg->chn->buf);
        struct htx_sl *sl;
-       struct http_hdr_ctx ctx;
+       struct http_hdr_ctx ctx, last_vary;
        struct comp_algo *comp_algo;
        int comp_index;
 
@@ -454,8 +454,29 @@ set_compression_header(struct comp_state *st, struct stream *s, struct http_msg
                }
        }
 
-       if (!http_add_header(htx, ist("Vary"), ist("Accept-Encoding")))
-               goto error;
+       /* Add "Vary: Accept-Encoding" header but only if it is not found. */
+       ctx.blk = NULL;
+       last_vary.blk = NULL;
+       while (http_find_header(htx, ist("Vary"), &ctx, 0)) {
+               if (isteqi(ctx.value, ist("Accept-Encoding")))
+                       break;
+               last_vary = ctx;
+       }
+       /* No "Accept-Encoding" value found. */
+       if (ctx.blk == NULL) {
+               if (last_vary.blk == NULL) {
+                       /* No Vary header found at all. Add our header */
+                       if (!http_add_header(htx, ist("Vary"), ist("Accept-Encoding")))
+                               goto error;
+               }
+               else  {
+                       /* At least one Vary header found. Append the value to
+                        * the last one.
+                        */
+                       if (!http_append_header_value(htx, &last_vary, ist("Accept-Encoding")))
+                               goto error;
+               }
+       }
 
        /*
         * Add Content-Encoding header when it's not identity encoding.