mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2024-12-31 09:02:07 +00:00
BUG/MEDIUM: cache: Fix hash collision in accept-encoding
handling for Vary
This patch fixes GitHub Issue #988. Commit ce9e7b2521
was not sufficient, because it fell back to a hash comparison if the bitmap
of known encodings was not acceptable instead of directly returning the the
cached response is not compatible.
This patch also extends the reg-test to test the hash collision that was
mentioned in #988.
Vary handling is 2.4, no backport needed.
This commit is contained in:
parent
64b6f36788
commit
dc38bc4a1a
56
reg-tests/cache/vary.vtc
vendored
56
reg-tests/cache/vary.vtc
vendored
@ -5,7 +5,8 @@ varnishtest "Vary support"
|
||||
feature ignore_unknown_macro
|
||||
|
||||
server s1 {
|
||||
# Response varying on "accept-encoding"
|
||||
# Response varying on "accept-encoding" with
|
||||
# an unacceptable content-encoding
|
||||
rxreq
|
||||
expect req.url == "/accept-encoding"
|
||||
txresp -hdr "Content-Encoding: gzip" \
|
||||
@ -16,6 +17,15 @@ server s1 {
|
||||
# Response varying on "accept-encoding"
|
||||
rxreq
|
||||
expect req.url == "/accept-encoding"
|
||||
txresp -hdr "Content-Encoding: gzip" \
|
||||
-hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-bodylen 45
|
||||
|
||||
# Response varying on "accept-encoding" with
|
||||
# no content-encoding
|
||||
rxreq
|
||||
expect req.url == "/accept-encoding"
|
||||
txresp -hdr "Content-Type: text/plain" \
|
||||
-hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
@ -57,7 +67,7 @@ server s1 {
|
||||
expect req.url == "/referer-accept-encoding"
|
||||
txresp -hdr "Vary: referer,accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: gzip" \
|
||||
-hdr "Content-Encoding: br" \
|
||||
-bodylen 54
|
||||
|
||||
rxreq
|
||||
@ -72,14 +82,14 @@ server s1 {
|
||||
expect req.url == "/multiple_headers"
|
||||
txresp -hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: gzip" \
|
||||
-hdr "Content-Encoding: br" \
|
||||
-bodylen 155
|
||||
|
||||
rxreq
|
||||
expect req.url == "/multiple_headers"
|
||||
txresp -hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: gzip" \
|
||||
-hdr "Content-Encoding: br" \
|
||||
-bodylen 166
|
||||
|
||||
|
||||
@ -163,6 +173,16 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.http.content-encoding == "gzip"
|
||||
expect resp.bodylen == 45
|
||||
|
||||
# The response for the first request had an unacceptable `content-encoding`
|
||||
# which might happen if that's the only thing the server supports, but
|
||||
# we must not cache that and instead defer to the server.
|
||||
txreq -url "/accept-encoding" -hdr "Accept-Encoding: first_value"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.http.content-encoding == "gzip"
|
||||
expect resp.bodylen == 45
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
txreq -url "/accept-encoding" -hdr "Accept-Encoding: second_value"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -170,11 +190,15 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.http.content-type == "text/plain"
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
# This request matches the cache entry for the request above, despite
|
||||
# matching the `accept-encoding` of the first request because the
|
||||
# request above only has the `identity` encoding which is implicitly
|
||||
# added, unless explicitely forbidden.
|
||||
txreq -url "/accept-encoding" -hdr "Accept-Encoding: first_value"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.bodylen == 45
|
||||
expect resp.http.content-encoding == "gzip"
|
||||
expect resp.bodylen == 48
|
||||
expect resp.http.content-type == "text/plain"
|
||||
expect resp.http.X-Cache-Hit == 1
|
||||
|
||||
txreq -url "/accept-encoding" -hdr "Accept-Encoding: second_value"
|
||||
@ -227,7 +251,7 @@ client c1 -connect ${h1_fe_sock} {
|
||||
|
||||
# Mixed Vary (Accept-Encoding + Referer)
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Accept-Encoding: first_value,second_value" \
|
||||
-hdr "Accept-Encoding: br, gzip" \
|
||||
-hdr "Referer: referer"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -235,7 +259,7 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Accept-Encoding: first_value" \
|
||||
-hdr "Accept-Encoding: br" \
|
||||
-hdr "Referer: other-referer"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -243,7 +267,7 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Accept-Encoding: second_value" \
|
||||
-hdr "Accept-Encoding: gzip" \
|
||||
-hdr "Referer: other-referer"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -252,14 +276,14 @@ client c1 -connect ${h1_fe_sock} {
|
||||
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Referer: referer" \
|
||||
-hdr "Accept-Encoding: second_value,first_value"
|
||||
-hdr "Accept-Encoding: gzip, br"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.bodylen == 51
|
||||
expect resp.http.X-Cache-Hit == 1
|
||||
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Accept-Encoding: first_value" \
|
||||
-hdr "Accept-Encoding: br" \
|
||||
-hdr "Referer: other-referer"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -267,7 +291,7 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.http.X-Cache-Hit == 1
|
||||
|
||||
txreq -url "/referer-accept-encoding" \
|
||||
-hdr "Accept-Encoding: second_value" \
|
||||
-hdr "Accept-Encoding: gzip" \
|
||||
-hdr "Referer: other-referer"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
@ -277,16 +301,16 @@ client c1 -connect ${h1_fe_sock} {
|
||||
|
||||
# Multiple Accept-encoding headers
|
||||
txreq -url "/multiple_headers" \
|
||||
-hdr "Accept-Encoding: first_encoding" \
|
||||
-hdr "Accept-Encoding: second_encoding,third_encoding"
|
||||
-hdr "Accept-Encoding: gzip" \
|
||||
-hdr "Accept-Encoding: br, deflate"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.bodylen == 155
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
txreq -url "/multiple_headers" \
|
||||
-hdr "Accept-Encoding: third_encoding" \
|
||||
-hdr "Accept-Encoding: second_encoding,first_encoding"
|
||||
-hdr "Accept-Encoding: deflate" \
|
||||
-hdr "Accept-Encoding: br,gzip"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.bodylen == 155
|
||||
|
32
reg-tests/cache/vary_accept_encoding.vtc
vendored
32
reg-tests/cache/vary_accept_encoding.vtc
vendored
@ -73,6 +73,21 @@ server s1 {
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: unknown_encoding" \
|
||||
-bodylen 119
|
||||
|
||||
|
||||
rxreq
|
||||
expect req.url == "/hash-collision"
|
||||
txresp -hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: br" \
|
||||
-bodylen 129
|
||||
|
||||
rxreq
|
||||
expect req.url == "/hash-collision"
|
||||
txresp -hdr "Vary: accept-encoding" \
|
||||
-hdr "Cache-Control: max-age=5" \
|
||||
-hdr "Content-Encoding: gzip" \
|
||||
-bodylen 139
|
||||
} -start
|
||||
|
||||
|
||||
@ -300,4 +315,21 @@ client c1 -connect ${h1_fe_sock} {
|
||||
expect resp.bodylen == 119
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
#
|
||||
# Hash collision (https://github.com/haproxy/haproxy/issues/988)
|
||||
#
|
||||
# crc32(gzip) ^ crc32(br) ^ crc32(xxx) ^ crc32(jdcqiab) == crc32(gzip)
|
||||
txreq -url "/hash-collision" -hdr "Accept-Encoding: br,gzip,xxx,jdcqiab"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.http.content-encoding == "br"
|
||||
expect resp.bodylen == 129
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
|
||||
txreq -url "/hash-collision" -hdr "Accept-Encoding: gzip"
|
||||
rxresp
|
||||
expect resp.status == 200
|
||||
expect resp.http.content-encoding == "gzip"
|
||||
expect resp.bodylen == 139
|
||||
expect resp.http.X-Cache-Hit == 0
|
||||
} -run
|
||||
|
29
src/cache.c
29
src/cache.c
@ -2398,19 +2398,28 @@ static int accept_encoding_hash_cmp(const void *ref_hash, const void *new_hash,
|
||||
/* All the bits set in the reference bitmap correspond to the
|
||||
* stored response' encoding and should all be set in the new
|
||||
* encoding bitmap in order for the client to be able to manage
|
||||
* the response. */
|
||||
if ((ref.encoding_bitmap & new.encoding_bitmap) == ref.encoding_bitmap) {
|
||||
/* The cached response has encodings that are accepted
|
||||
* by the client, it can be served directly by the cache
|
||||
* (as far as the accept-encoding part is concerned). */
|
||||
return 0;
|
||||
}
|
||||
* the response.
|
||||
*
|
||||
* If this is the case the cached response has encodings that
|
||||
* are accepted by the client. It can be served directly by
|
||||
* the cache (as far as the accept-encoding part is concerned).
|
||||
*/
|
||||
|
||||
return (ref.encoding_bitmap & new.encoding_bitmap) != ref.encoding_bitmap;
|
||||
}
|
||||
else {
|
||||
/* We must compare hashes only when the the response contains
|
||||
* unknown encodings.
|
||||
* Otherwise we might serve unacceptable responses if the hash
|
||||
* of a client's `accept-encoding` header collides with a
|
||||
* known encoding.
|
||||
*/
|
||||
|
||||
ref.hash = read_u32(ref_hash+sizeof(ref.encoding_bitmap));
|
||||
new.hash = read_u32(new_hash+sizeof(new.encoding_bitmap));
|
||||
ref.hash = read_u32(ref_hash+sizeof(ref.encoding_bitmap));
|
||||
new.hash = read_u32(new_hash+sizeof(new.encoding_bitmap));
|
||||
|
||||
return ref.hash != new.hash;
|
||||
return ref.hash != new.hash;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user