Cache needs invalidated for PATCH and unknown methods

[rob05c]

This is strictly violating the RFC.

does_method_require_cache_copy_deletion needs to return true for PATCH and unknown methods

https://datatracker.ietf.org/doc/html/rfc7234#section-4.4

A cache MUST invalidate the effective request URI (Section 5.5 of [RFC7230]) when it receives a non-error response to a request with a method whose safety is unknown.

And

https://datatracker.ietf.org/doc/html/rfc5789

PATCH is neither safe nor idempotent

The code is https://github.com/apache/trafficserver/blob/b3ef5a04/proxy/http/HttpTransact.cc#L750

does_method_require_cache_copy_deletion(const HttpConfigParams *http_config_param, const int method)
{
  return ((method != HTTP_WKSIDX_GET) &&
          (method == HTTP_WKSIDX_DELETE || method == HTTP_WKSIDX_PURGE || method == HTTP_WKSIDX_PUT ||
           (http_config_param->cache_post_method != 1 && method == HTTP_WKSIDX_POST)));

So it's only returning true specifically for DELETE,PURGE,PUT,(maybe)POST.

It needs changed to return true, causing ATS to delete (invalidate) cached objects, for unknown methods and PATCH.

[rob05c]

This is trivial to fix, but I'm only like 95% sure the above is all true and correct. Someone else confirming would be good.

[rob05c]

See also https://github.com/apache/trafficserver/pull/7999

[shinrich]

Agree. Would be better to list the good ones.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

[JosiahWI]

This is trivial to fix, but I'm only like 95% sure the above is all true and correct. Someone else confirming would be good.

[JosiahWI]

Whoops, I didn't mean to close this issue. I am confirming that RFC 9111, which updates RFC 7234, does indeed say that unknown methods should be treated as unsafe and therefore invalidate the cache on non-error responses. It should be trivial to fix, but I will write tests for it as well.