bneradt commented on PR #13913: URL: https://github.com/apache/trafficserver/pull/13913#issuecomment-6608584417
Thanks for the careful review, @bryancall. I pushed 791b0964f7, rebased on master after #13912 merged. **Blocking: must-revalidate:** I took the second option. must-revalidate and proxy-revalidate are no longer kept when their element is malformed, so they're ignored the same way they are on master today. A malformed 302 with `must-revalidate; max-age=0` stays uncacheable. The replay test now has that 302 case, and the unit tests have must-revalidate and proxy-revalidate rows. Treating invalid freshness information as stale is the more complete fix, but it's a larger change in HttpTransact, so I left it for a follow-up. The same goes for must-revalidate counting as "cacheable" on well-formed headers. I agree that belongs in its own PR. **Should fix: restrictive directive later in the element:** A malformed element is now scanned from start to end for no-cache, no-store and private tokens. `max-age=600; private`, `s-maxage=60; no-store` and `public; private` keep the restriction, while a token like `x-no-store-private` does not match. There are unit rows for each, and the replay test has a two-user `max-age=600; private` case. **Non-blocking: qualified forms:** Since #13912 merged, `no-cache="Set-Cookie"` and `private="Set-Cookie"` parse as well-formed, so they no longer reach this code. Its unit rows and the `cache-control-qualified` replay test cover them, and both still pass with this change. **Nits:** I fixed both. The comment no longer says every bit is cleared, and the quote now cites RFC 9111 Section 5.2.3. @HiLleywyn, thanks for testing and for the follow-up. The 302 and later-token regressions are both covered now. -- 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]
