Copilot commented on code in PR #13611: URL: https://github.com/apache/trafficserver/pull/13611#discussion_r4018683489
########## src/proxy/hdrs/URL.cc: ########## @@ -1908,8 +1913,16 @@ url_CryptoHash_get_general_92(const URLImpl *url, CryptoContext &ctx, CryptoHash ends[7] = strs[7] + 1; ends[8] = strs[8] + url->m_len_path; - strs[9] = ";"; - strs[10] = url->m_ptr_params; + // ATS 9.2 split "/path;params" into separate path and params components and + // hashed them as path + ";" + params. That parsing was removed, so ";params" + // now stays inside the path and already spells the same byte sequence. Adding + // the separator again would append a ";" that 9.2 never emitted, so only add + // it when the path does not carry one. The params component itself is always + // empty now; it is left out rather than read back as an empty string. + bool const path_has_params = url->has_path_params(); + + strs[9] = path_has_params ? nullptr : ";"; Review Comment: This makes the compatibility key for `/foo` identical to the current key for `/foo;`. Cache reads are selected by that hash without validating the stored request URL, so with this fallback enabled a current `/foo;` object can be served for a request to `/foo` as though it were a legacy object (the new replay primes exactly this collision). Please reject compatibility hits whose stored URL does not match the requested URL, and add a regression test for the cross-URI case; this needs changes in the hit path, not just this hash expression. -- 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]
