Copilot commented on code in PR #13776:
URL: https://github.com/apache/trafficserver/pull/13776#discussion_r4162620160
##########
plugins/compress/compress.cc:
##########
@@ -811,6 +810,190 @@ 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;
+ }
+ ts::PostScript cached_defer([&]() -> void { TSHandleMLocRelease(cached_buf,
TS_NULL_MLOC, cached_loc); });
+
+ TSMLoc cached_ce = TSMimeHdrFieldFind(cached_buf, cached_loc,
TS_MIME_FIELD_CONTENT_ENCODING, TS_MIME_LEN_CONTENT_ENCODING);
+
+ if (cached_ce == TS_NULL_MLOC) {
+ return;
+ }
+ TSHandleMLocRelease(cached_buf, cached_loc, cached_ce);
+
+ TSMLoc srv_etag = TSMimeHdrFieldFind(srv_buf, srv_loc, TS_MIME_FIELD_ETAG,
TS_MIME_LEN_ETAG);
+
+ if (srv_etag == TS_NULL_MLOC) {
+ return;
+ }
+ ts::PostScript srv_etag_defer([&]() -> void { TSHandleMLocRelease(srv_buf,
srv_loc, srv_etag); });
+
+ TSMLoc cached_etag = TSMimeHdrFieldFind(cached_buf, cached_loc,
TS_MIME_FIELD_ETAG, TS_MIME_LEN_ETAG);
+
+ if (cached_etag == TS_NULL_MLOC) {
+ return;
+ }
+ ts::PostScript cached_etag_defer([&]() -> void {
TSHandleMLocRelease(cached_buf, cached_loc, cached_etag); });
+
+ int srv_len;
+ const char *srv_str = TSMimeHdrFieldValueStringGet(srv_buf, srv_loc,
srv_etag, -1, &srv_len);
+ int cached_len;
+ const char *cached_str = TSMimeHdrFieldValueStringGet(cached_buf,
cached_loc, cached_etag, -1, &cached_len);
+ std::string_view srv_value{srv_str, static_cast<size_t>(srv_len)};
+ std::string_view cached_value{cached_str, static_cast<size_t>(cached_len)};
+
+ if (cached_value.starts_with("W/") && cached_value.substr(2) == srv_value) {
+ TSMimeHdrFieldValueStringSet(srv_buf, srv_loc, srv_etag, -1,
cached_value.data(), cached_value.size());
+ }
+}
+
+// No transform runs on a bodyless 304, so it goes out with the origin's or
the cached identity
+// copy's strong ETag. A 304 must carry the ETag its 200 would (RFC 9110
15.4.5), and this
+// request's 200 would have been compressed, so weaken it to match.
+static void
+weaken_client_not_modified_etag(TSHttpTxn txnp)
+{
+ TSMBuffer resp_buf;
+ TSMLoc resp_loc;
+
+ if (TS_SUCCESS != TSHttpTxnClientRespGet(txnp, &resp_buf, &resp_loc)) {
+ return;
+ }
+ ts::PostScript resp_defer([&]() -> void { TSHandleMLocRelease(resp_buf,
TS_NULL_MLOC, resp_loc); });
+
+ if (TSHttpHdrStatusGet(resp_buf, resp_loc) == TS_HTTP_STATUS_NOT_MODIFIED) {
+ etag_header(resp_buf, resp_loc);
+ }
+}
+
+static int
+not_modified_etag_plugin(TSCont contp, TSEvent event, void *edata)
+{
+ TSHttpTxn txnp = static_cast<TSHttpTxn>(edata);
+
+ switch (event) {
+ case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
+ weaken_client_not_modified_etag(txnp);
+ break;
+
+ case TS_EVENT_HTTP_TXN_CLOSE:
+ TSContDestroy(contp);
+ break;
+
+ default:
+ fatal("compress 304 ETag unknown event");
+ }
+
+ TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+
+ return 0;
+}
+
+// With nothing stored, an origin 304 leaves no representation to tell whether
this request's 200
+// would have been compressed. A client validating with the weak form of the
origin's strong ETag
+// can only hold a copy this plugin weakened, so answer with the tag it was
given.
+static void
+weaken_unjudged_origin_not_modified(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)) {
+ TSHandleMLocRelease(cached_buf, TS_NULL_MLOC, cached_loc);
+ return;
+ }
+
+ TSMBuffer req_buf;
+ TSMLoc req_loc;
+
+ if (TS_SUCCESS != TSHttpTxnClientReqGet(txnp, &req_buf, &req_loc)) {
+ return;
+ }
+ ts::PostScript req_defer([&]() -> void { TSHandleMLocRelease(req_buf,
TS_NULL_MLOC, req_loc); });
+
+ int method_len;
+ const char *method = TSHttpHdrMethodGet(req_buf, req_loc, &method_len);
+
+ if (!((method_len == TS_HTTP_LEN_GET && memcmp(method, TS_HTTP_METHOD_GET,
TS_HTTP_LEN_GET) == 0) ||
+ (method_len == TS_HTTP_LEN_HEAD && memcmp(method, TS_HTTP_METHOD_HEAD,
TS_HTTP_LEN_HEAD) == 0))) {
+ return;
+ }
+
+ TSMLoc srv_etag = TSMimeHdrFieldFind(srv_buf, srv_loc, TS_MIME_FIELD_ETAG,
TS_MIME_LEN_ETAG);
+
+ if (srv_etag == TS_NULL_MLOC) {
+ return;
+ }
+ ts::PostScript srv_etag_defer([&]() -> void { TSHandleMLocRelease(srv_buf,
srv_loc, srv_etag); });
+
+ TSMLoc inm = TSMimeHdrFieldFind(req_buf, req_loc,
TS_MIME_FIELD_IF_NONE_MATCH, TS_MIME_LEN_IF_NONE_MATCH);
+
+ if (inm == TS_NULL_MLOC) {
+ return;
+ }
+ ts::PostScript inm_defer([&]() -> void { TSHandleMLocRelease(req_buf,
req_loc, inm); });
+
+ int srv_len;
+ const char *srv_str = TSMimeHdrFieldValueStringGet(srv_buf, srv_loc,
srv_etag, -1, &srv_len);
+ std::string_view srv_value{srv_str, static_cast<size_t>(srv_len)};
+
+ if (srv_value.starts_with("W/") || srv_value.starts_with("w/")) {
+ return;
+ }
+
+ int const nvalues = TSMimeHdrFieldValuesCount(req_buf, req_loc, inm);
+
+ for (int i = 0; i < nvalues; i++) {
+ int len;
+ const char *str = TSMimeHdrFieldValueStringGet(req_buf, req_loc, inm, i,
&len);
+
+ if (str == nullptr) {
+ continue;
+ }
+
+ std::string_view tag{str, static_cast<size_t>(len)};
+
+ if (tag.starts_with("W/") && tag.substr(2) == srv_value) {
+ TSMimeHdrFieldValueStringSet(srv_buf, srv_loc, srv_etag, -1, tag.data(),
tag.size());
Review Comment:
This inference also fires when compression cannot be selected. For example,
a cacheless GET without `Accept-Encoding` can send `If-None-Match: W/"p1"`; if
the origin replies `304` with `ETag: "p1"`, this branch rewrites it to weak
even though the corresponding 200 would remain identity (and `transformable()`
would reject it). The same applies to an unsupported encoding. Gate this
fallback on the request accepting a configured compression algorithm, and add a
direct-origin-304 identity regression case; otherwise the plugin unnecessarily
discards a valid strong validator and prevents later `If-Range` use.
--
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]
