Copilot commented on code in PR #13776:
URL: https://github.com/apache/trafficserver/pull/13776#discussion_r4162933547
##########
src/iocore/cache/HttpTransactCache.cc:
##########
@@ -86,10 +86,10 @@ do_strings_match_strongly(const char *raw_tag_field, int
raw_tag_field_len, cons
const char *etag_start;
int n, etag_length;
- // Can never match a weak tag with a strong compare
- if ((raw_tag_field_len >= 2) && (raw_tag_field[0] == 'W' && raw_tag_field[1]
== '/')) {
- return false;
- }
+ // A missing or weak tag never matches a specific tag under a strong
compare, but "*" matches any
+ // current representation.
+ bool const only_star = (raw_tag_field_len <= 0) || (raw_tag_field_len >= 2
&& raw_tag_field[0] == 'W' && raw_tag_field[1] == '/');
+
// Find the unalterated tag
etag_start = find_etag(raw_tag_field, raw_tag_field_len, &etag_length);
Review Comment:
The new missing-ETag path still calls `find_etag()` unconditionally. An
absent field is returned as a default empty `std::string_view`
(`include/proxy/hdrs/MIME.h:1321-1328`), so `raw_tag_field` can be null;
`find_etag()` then evaluates `raw_tag_field + 0`, which is undefined behavior.
Avoid parsing the response tag when its length is zero—the request list can
still be scanned for `*`.
##########
src/iocore/cache/HttpTransactCache.cc:
##########
@@ -1384,12 +1388,8 @@
HttpTransactCache::match_response_to_request_conditionals(HTTPHdr *request, HTTP
// If-Match: must match strongly //
if (request->presence(MIME_PRESENCE_IF_MATCH)) {
- auto
raw_etags{response->value_get(static_cast<std::string_view>(MIME_FIELD_ETAG))};
- std::string_view comma_sep_tag_list{};
-
- if (!raw_etags.empty()) {
- comma_sep_tag_list =
request->value_get(static_cast<std::string_view>(MIME_FIELD_IF_MATCH));
- }
+ auto
raw_etags{response->value_get(static_cast<std::string_view>(MIME_FIELD_ETAG))};
+ auto
comma_sep_tag_list{request->value_get(static_cast<std::string_view>(MIME_FIELD_IF_MATCH))};
if (do_strings_match_strongly(raw_etags.data(),
static_cast<int>(raw_etags.length()), comma_sep_tag_list.data(),
Review Comment:
This now treats `If-Match: *` as successful for every cached response
without an ETag, including a negative-cached 404. `build_response_from_cache()`
passes arbitrary cached statuses into this matcher
(`src/proxy/http/HttpTransact.cc:3146-3169`), and 404 is explicitly eligible
for negative caching; however, `*` must fail when the target has no current
representation. Gate the wildcard result on representation existence (and add a
negative-cache regression) rather than inferring existence solely from having a
cached response object.
--
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]
