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]

Reply via email to