Commit Graph

31 Commits

Author SHA1 Message Date
Remi Tricot-Le Breton ca4fd73938 BUG/MINOR: cache: A 'max-age=0' cache-control directive can be overriden by a s-maxage
When a s-maxage cache-control directive is present, it overrides any
other max-age or expires value (see section 5.2.2.9 of RFC7234). So if
we have a max-age=0 alongside a strictly positive s-maxage, the response
should be cached.

This bug was raised in GitHub issue #2203.
The fix can be backported to all stable branches.
2023-07-04 22:15:00 +02:00
Christopher Faulet 34cffede3a REGTESTS: cache: Use rxresphdrs to only get headers for 304 responses
304 responses contains "Content-length" or "Transfer-encoding"
headers. rxresp action expects to get a payload in this case, even if 304
reponses must not have any payload. A workaround was added to remove these
headers from the 304 responses. However, a better solution is to only get
the response headers from clients using rxresphdrs action.

If a payload is erroneously added in these reponses, the scripts will fail
the same way. So it is safe.
2023-02-22 16:12:45 +01:00
Remi Tricot-Le Breton 879debeecb BUG/MINOR: cache: Cache response even if request has "no-cache" directive
Since commit cc9bf2e5f "MEDIUM: cache: Change caching conditions"
responses that do not have an explicit expiration time are not cached
anymore. But this mechanism wrongly used the TX_CACHE_IGNORE flag
instead of the TX_CACHEABLE one. The effect this had is that a cacheable
response that corresponded to a request having a "Cache-Control:
no-cache" for instance would not be cached.
Contrary to what was said in the other commit message, the "checkcache"
option should not be impacted by the use of the TX_CACHEABLE flag
instead of the TX_CACHE_IGNORE one. The response is indeed considered as
not cacheable if it has no expiration time, regardless of the presence
of a cookie in the response.

This should fix GitHub issue #2048.
This patch can be backported up to branch 2.4.
2023-02-21 18:35:41 +01:00
Tim Duesterhus 2a5fb62ad1 REGTESTS: Remove REQUIRE_VERSION=1.9 from all tests
HAProxy 2.0 is the lowest supported version, thus this always matches.

see 1b095cac94
2023-01-05 09:11:38 +01:00
Christopher Faulet a0e1a87948 REG-TESTS: cache: Remove T-E header for 304-Not-Modified responses
VTEST does not properly handle 304-Not-Modified responses. If a
Transfer-Encoding header (and probably a Content-Lenght header too), it
waits for a body. Waiting for a fix, the Transfer-Encoding encoding of
cached responses in 2 VTEST scripts are removed.

Note it is now an issue because of a fix in the H1 multiplexer :

  * 226082d13a "BUG/MINOR: mux-h1: Do not send a last null chunk on body-less answers"

This patch must be backported with the above commit.
2022-11-16 17:19:43 +01:00
Willy Tarreau f673923629 REGTESTS: extend the default I/O timeouts and make them overridable
With the CI occasionally slowing down, we're starting to see again some
spurious failures despite the long 1-second timeouts. This reports false
positives that are disturbing and doesn't provide as much value as this
could. However at this delay it already becomes a pain for developers
to wait for the tests to complete.

This commit adds support for the new environment variable
HAPROXY_TEST_TIMEOUT that will allow anyone to modify the connect,
client and server timeouts. It was set to 5 seconds by default, which
should be plenty for quite some time in the CI. All relevant values
that were 200ms or above were replaced by this one. A few larger
values were left as they are special. One test for the set-timeout
action that used to rely on a fixed 1-sec value was extended to a
fixed 5-sec, as the timeout is normally not reached, but it needs
to be known to compare the old and new values.
2021-11-18 17:57:11 +01:00
Tim Duesterhus 3bc6af417d BUG/MINOR: cache: Correctly handle existing-but-empty 'accept-encoding' header
RFC 7231#5.3.4 makes a difference between a completely missing
'accept-encoding' header and an 'accept-encoding' header without any values.

This case was already correctly handled by accident, because an empty accept
encoding does not match any known encoding. However this resulted in the
'other' encoding being added to the bitmap. Usually this also succeeds in
serving cached responses, because the cached response likely has no
'content-encoding', thus matching the identity case instead of not serving the
response, due to the 'other' encoding. But it's technically not 100% correct.

Fix this by special-casing 'accept-encoding' values with a length of zero and
extend the test to check that an empty accept-encoding is correctly handled.
Due to the reasons given above the test also passes without the change in
cache.c.

Vary support was added in HAProxy 2.4. This fix should be backported to 2.4+.
2021-06-18 15:48:20 +02:00
Tim Duesterhus a9334df5a9 CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests
The legacy HTTP subsystem has been removed. HTX is always enabled.
2021-06-04 15:41:21 +02:00
Willy Tarreau e1465c1e46 REGTESTS: disable inter-thread idle connection sharing on sensitive tests
Some regtests involve multiple requests from multiple clients, which can
be dispatched as multiple requests to a server. It turns out that the
idle connection sharing works so well that very quickly few connections
are used, and regularly some of the remaining idle server connections
time out at the moment they were going to be reused, causing those random
"HTTP header incomplete" traces in the logs that make them fail often. In
the end this is only an artefact of the test environment.

And indeed, some tests like normalize-uri which perform a lot of reuse
fail very often, about 20-30% of the times in the CI, and 100% of the
time in local when running 1000 tests in a row. Others like ubase64,
sample_fetches or vary_* fail less often but still a lot in tests.

This patch addresses this by adding "tune.idle-pool.shared off" to all
tests which have at least twice as many requests as clients. It proves
very effective as no single error happens on normalize-uri anymore after
10000 tests. Also 100 full runs of all tests yield no error anymore.

One test is tricky, http_abortonclose, it used to fail ~10 times per
1000 runs and with this workaround still fails once every 1000 runs.
But the test is complex and there's a warning in it mentioning a
possible issue when run in parallel due to a port reuse.
2021-05-09 14:41:41 +02:00
Christopher Faulet 85a813676f REGTESTS: Fix required versions for several scripts
The following scripts require HAProxy 2.4 :

 * cache/caching_rules.vtc
 * cache/post_on_entry.vtc
 * cache/vary.vtc
 * checks/1be_40srv_odd_health_checks.vtc
 * checks/40be_2srv_odd_health_checks.vtc
 * checks/4be_1srv_health_checks.vtc
 * converter/fix.vtc
 * converter/mqtt.vtc
 * http-messaging/protocol_upgrade.vtc
 * http-messaging/websocket.vtc
 * http-set-timeout/set_timeout.vtc
 * log/log_uri.vtc

However it may change is features are backported.
2021-01-28 16:37:14 +01:00
Remi Tricot-Le Breton 6ca89162dc MINOR: cache: Do not store responses with an unknown encoding
If a server varies on the accept-encoding header and it sends a response
with an encoding we do not know (see parse_encoding_value function), we
will not store it. This will prevent unexpected errors caused by
cache collisions that could happen in accept_encoding_hash_cmp.
2021-01-15 22:33:05 +01:00
Ilya Shipitsin 1e9a66603f CLEANUP: assorted typo fixes in the code and comments
This is 14th iteration of typo fixes
2021-01-06 16:26:50 +01:00
Tim Duesterhus dc38bc4a1a 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.
2020-12-31 09:39:08 +01:00
Remi Tricot-Le Breton e6cc5b5974 MINOR: cache: Replace the "process-vary" option's expected values
Replace the <0/1> expected values of the process-vary option by a more
usual <on/off> pair.
2020-12-24 17:18:00 +01:00
Remi Tricot-Le Breton 14df1e1526 REGTESTS: cache: Add a specific test for the accept-encoding normalizer
Those tests check the explicit support of a subset of encodings added to
the accept-encoding normalizer.
2020-12-24 17:18:00 +01:00
Remi Tricot-Le Breton b054b6dec6 REGTESTS: cache: Simplify vary.vtc file
This test used chunked responses but it is not needed. All the chunked
responses are replaced by more simple 'bodylen' directives.
2020-12-24 17:18:00 +01:00
Remi Tricot-Le Breton e4421dec7e BUG/MINOR: cache: Manage multiple headers in accept-encoding normalization
The accept-encoding part of the secondary key (vary) was only built out
of the first occurrence of the header. So if a client had two
accept-encoding headers, gzip and br for instance, the key would have
been built out of the gzip string. So another client that only managed
gzip would have been sent the cached resource, even if it was a br resource.
The http_find_header function is now called directly by the normalizers
so that they can manage multiple headers if needed.
A request that has more than 16 encodings will be considered as an
illegitimate request and its response will not be stored.

This fixes GitHub issue #987.

It does not need any backport.
2020-12-24 17:18:00 +01:00
Remi Tricot-Le Breton 72cffaf440 MEDIUM: cache: Remove cache entry in case of POST on the same resource
In case of successful unsafe method on a stored resource, the cached entry
must be invalidated (see RFC7234#4.4).
A "non-error response" is one with a 2xx (Successful) or 3xx (Redirection)
status code.
This implies that the primary hash must now be calculated on requests
that have an unsafe method (POST or PUT for instance) so that we can
disable the corresponding entries when we process the response.
2020-12-04 10:21:56 +01:00
Remi Tricot-Le Breton 795e1412b0 MINOR: cache: Do not store stale entry
When a response has an Age header (filled in by another cache on the
message's path) that is greater than its defined maximum age (extracted
either from cache-control directives or an expires header), it is
already stale and should not be cached.
2020-12-04 10:21:56 +01:00
Remi Tricot-Le Breton 8bb72aa82f MINOR: cache: Improve accept_encoding_normalizer
Turn the "Accept-Encoding" value to lower case before processing it.
Calculate the CRC on every token instead of a sorted concatenation of
them all (in order to avoir copying them) then XOR all the CRCs into a
single hash (while ignoring duplicates).
2020-12-02 16:32:54 +01:00
Remi Tricot-Le Breton 754b2428d3 MINOR: cache: Add a process-vary option that can enable/disable Vary processing
The cache section's process-vary option takes a 0 or 1 value to disable
or enable the vary processing.
When disabled, a response containing such a header will never be cached.
When enabled, we will calculate a preliminary hash for a subset of request
headers on all the incoming requests (which might come with a cpu cost) which
will be used to build a secondary key for a given request (see RFC 7234#4.1).
The default value is 0 (disabled).
2020-11-24 16:52:57 +01:00
Remi Tricot-Le Breton 1785f3dd96 MEDIUM: cache: Add the Vary header support
Calculate a preliminary secondary key for every request we see so that
we can have a real secondary key if the response is cacheable and
contains a manageable Vary header.
The cache's ebtree is now allowed to have multiple entries with the same
primary key. Two of those entries will be distinguished thanks to
secondary keys stored in the cache_entry (based on hashes of a subset of
their headers).
When looking for an entry in the cache (cache_use), we still use the
primary key (built the same way as before), but in case of match, we
also need to check if the entry has a vary signature. If it has one, we
need to perform an extra check based on the newly built secondary key.
We will only be able to forge a response out of the cache if both the
primary and secondary keys match with one of our entries. Otherwise the
request will be forwarder to the server.
2020-11-24 16:52:57 +01:00
Remi Tricot-Le Breton cc9bf2e5fe MEDIUM: cache: Change caching conditions
Do not cache responses that do not have an explicit expiration time
(s-maxage or max-age Cache-Control directives or Expires header) or a
validator (ETag or Last-Modified headers) anymore, as suggested in
RFC 7234#3.
The TX_FLAG_IGNORE flag is used instead of the TX_FLAG_CACHEABLE so as
not to change the behavior of the checkcache option.
2020-11-12 11:22:05 +01:00
Remi Tricot-Le Breton a6476114ec MINOR: cache: Add Expires header value parsing
When no Cache-Control max-age or s-maxage information is present in a
cached response, we need to parse the Expires header value (RFC 7234#5.3).
An invalid Expires date value or a date earlier than the reception date
will make the cache_entry stale upon creation.
For now, the Cache-Control and Expires headers are parsed after the
insertion of the response in the cache so even if the parsing of the
Expires results in an already stale entry, the entry will exist in the
cache.
2020-10-30 11:08:38 +01:00
Remi Tricot-Le Breton bf97121f1c MINOR: cache: Create res.cache_hit and res.cache_name sample fetches
Res.cache_hit sample fetch returns a boolean which is true when the HTTP
response was built out of a cache. The cache's name is returned by the
res.cache_name sample_fetch.

This resolves GitHub issue #900.
2020-10-27 18:25:43 +01:00
Remi Tricot-Le Breton 53161d81b8 MINOR: cache: Process the If-Modified-Since header in conditional requests
If a client sends a conditional request containing an If-Modified-Since
header (and no If-None-Match header), we try to compare the date with
the one stored in the cache entry (coming either from a Last-Modified
head, or a Date header, or corresponding to the first response's
reception time). If the request's date is earlier than the stored one,
we send a "304 Not Modified" response back. Otherwise, the stored is sent
(through a 200 OK response).

This resolves GitHub issue #821.
2020-10-27 18:10:25 +01:00
Remi Tricot-Le Breton 5bbdc81cf1 REGTEST: cache: Add if-none-match test case
Test that if-none-match header is properly taken into account and that
when the conditions are fulfilled, a "304 Not Modified" response can be
sent to the client.

Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
2020-10-22 16:10:20 +02:00
Frédéric Lécaille d7a8f14145 REGTEST: rename the reg test files.
We rename all the VTC files to avoid name collisions when importing/backporting.
2019-04-23 15:37:03 +02:00
Christopher Faulet 8f16148df7 REGTEST: Add option to use HTX prefixed by the macro 'no-htx'
So some tests have been removed.
2018-12-20 10:37:32 +01:00
Willy Tarreau 86ff9c8f3a REGTEST: the cache regtest requires haproxy 1.9
The chunks and the max-object-size features require version 1.9.
2018-12-12 15:21:56 +01:00
Frédéric Lécaille a3fe1bb901 REGTEST: Add a basic test for the cache.
The client makes the same HTTP request four times.
The varnishtest HTTP server serves the first client request and quits.
So, the three last requests are handled by the haproxy cache.
2018-11-29 08:42:01 +01:00