ezelkow1 commented on code in PR #13776:
URL: https://github.com/apache/trafficserver/pull/13776#discussion_r4161266995
##########
plugins/compress/compress.cc:
##########
@@ -811,6 +810,65 @@ add_vary_header_to_client_response(TSHttpTxn txnp)
TSHandleMLocRelease(resp_buf, TS_NULL_MLOC, resp_loc);
}
+// A 304 carries the origin's strong ETag, which the cache would merge over
the weakened ETag of a
+// stored compressed copy. Keep it weak when both name the same entity and the
stored copy is
+// encoded; an identity copy's ETag is the origin's own, so it may
legitimately become strong.
+static void
+keep_cached_etag_weak(TSHttpTxn txnp)
+{
+ TSMBuffer srv_buf;
+ TSMLoc srv_loc;
+
+ if (TS_SUCCESS != TSHttpTxnServerRespGet(txnp, &srv_buf, &srv_loc)) {
+ return;
+ }
+ ts::PostScript srv_defer([&]() -> void { TSHandleMLocRelease(srv_buf,
TS_NULL_MLOC, srv_loc); });
+
+ if (TSHttpHdrStatusGet(srv_buf, srv_loc) != TS_HTTP_STATUS_NOT_MODIFIED) {
+ return;
+ }
+
+ TSMBuffer cached_buf;
+ TSMLoc cached_loc;
+
+ if (TS_SUCCESS != TSHttpTxnCachedRespGet(txnp, &cached_buf, &cached_loc)) {
+ return;
Review Comment:
Thanks, confirmed all of these. One nuance on the private case: on the
default config ATS strips the client's conditionals on a cache miss for a
likely-cacheable request, so the origin returns a 200 and ATS generates the 304
itself. The conditional only reaches the origin when the request isn't likely
cacheable (e.g. `Authorization`) or with `when_to_revalidate=4`. Either way the
client got the strong tag.
Fixed with a `SEND_RESPONSE_HDR` hook (registered only for requests with
`If-None-Match`): when the outgoing response is a 304 with strong ETag `"x"`,
the client accepts an encoding the remap compresses with, and the client's
`If-None-Match` includes `W/"x"`, the 304 carries `W/"x"`. This only touches
the client response, so with `cache false` the stored identity copy keeps its
strong tag. The replay now covers the private response (both the ATS-generated
and origin 304 paths), the `cache false` fresh hit, a strong conditional
against the `cache false` copy (stays strong), and a client without
`Accept-Encoding` (stays strong).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
