bneradt commented on code in PR #13776:
URL: https://github.com/apache/trafficserver/pull/13776#discussion_r4160998320
##########
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:
[P2] Preserve weak validators when only the client has the compressed copy
This early return leaves client-side revalidation unprotected. For example,
let the origin send a compressible `200` with `Cache-Control: private,
max-age=1` and `ETag: "v1"`. ATS sends the client gzip with `W/"v1"` but stores
no cache entry. The client's next `GET` with `Accept-Encoding: gzip` and
`If-None-Match: W/"v1"` can now successfully validate at the origin, which
returns `304` with `ETag: "v1"`. `TSHttpTxnCachedRespGet` fails here,
`is_content_compressible` also returns without a cached response, and the
bodyless 304 never reaches `etag_header`. The client therefore receives the
origin's strong validator for its transformed representation. The corresponding
gzip 200 would carry a weak validator; [RFC 9110
ยง15.4.5](https://www.rfc-editor.org/rfc/rfc9110.html#section-15.4.5) requires
the 304's ETag to be the one that would be sent with a 200 for that request.
Please cover this client-only cache case in the replay and preserve the
outgoing weak validator without depending on an encoded ATS cache entry. The
related `cache false` case also needs coverage: ATS stores identity bytes with
a strong tag, so a fresh conditional cache hit can generate a strong-tagged 304
without running the body transform. The identity cache metadata should remain
strong, while the client's compressed representation keeps its weak validator.
--
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]