traeak commented on code in PR #13611:
URL: https://github.com/apache/trafficserver/pull/13611#discussion_r4018769932
##########
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:
Comment from claude:
- ATS 9.2 keyed `/foo` and `/foo;` identically. Every 9.2 cache this feature
exists to migrate already conflated the two, in both directions, for its
whole life.
- Master conflates them today without any setting. A request for `/foo;`
reads the current key for `/foo;`, which is precisely where every legacy
`/foo` object lives. That direction stays until the legacy copy ages out,
and this PR does not touch it.
- This PR adds only the reverse direction, `/foo` reading a post-upgrade
`/foo;` object, and only while the migration switch is on.
- The collision is exactly a path ending in a bare semicolon with nothing
after it. A path like `/foo;a` hashes differently under both schemes and
never collides.
--
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]
