You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/06/26 17:37:36 UTC
[GitHub] [trafficserver] bneradt opened a new pull request #7999: Do not invalidate cached resources upon error responses to unsafe met…
bneradt opened a new pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999
…hods
Before this change, cached resources would be deleted when the server
replied to a request with an unsafe method (POST, PUT, etc.) with an
error response code. This patch changes that behavior so that it is only
deleted when the response has a successful response code.
This seeks to comply more narrowly with the wording of RFC 7234,
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.
Note that we were technically in compliance with this before since we
always invalidated, regardless of the error response code. This,
however, was too broad in that we invalidated when we didn't need to.
Now we will only invalidate when the response code indicates a
successful response to the request with an unsafe method.
This also fixes our feature that caches responses to POST requests.
---
This change is a reprisal of commit
c75c797410a8661f23ce1b9868b771457b1026f7. One of the ESI plugin
features, called packed-node, updated the cached resource for ESI
includes with a "packed" version which offers more efficient parsing.
This packing was achieved via an internal POST to the cache, effectively
replacing the resource with the packed version. The former commit,
however, broke this mechanism because the updated resource was
associated with a POST request, which differed from the subsequent GET
request for the resource. This difference in method, per the changes
made to HttpTransact for caching of POST requests, now results in the
cached resource no longer matching the requst, which caused the ESI
plugin to not find it, resulting eventually in assertions being tripped.
This latter patch fixes this by simply using a PUT to update the source
which achieves the same effect without mismatching the request methods.
This problem is exposed via the esi.test.py test and was caught in CI,
so that test functions as a regression test for this update to the
patch.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt removed a comment on pull request #7999: Do not invalidate cached resources upon error responses to unsafe met…
Posted by GitBox <gi...@apache.org>.
bneradt removed a comment on pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999#issuecomment-869045340
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7999: Do not invalidate cached resources upon error responses to unsafe met…
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999#issuecomment-869045340
[approve ci autest]
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shukitchan removed a comment on pull request #7999: Do not invalidate cached resources upon error responses to unsafe met…
Posted by GitBox <gi...@apache.org>.
shukitchan removed a comment on pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999#issuecomment-869203973
I think we need to change https://github.com/apache/trafficserver/blob/master/plugins/esi/esi.cc#L1346 as well.
The "post" request will be intercepted to update the cache. Changing it to PUT will make it not go through those code.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] shukitchan commented on pull request #7999: Do not invalidate cached resources upon error responses to unsafe met…
Posted by GitBox <gi...@apache.org>.
shukitchan commented on pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999#issuecomment-869203973
I think we need to change https://github.com/apache/trafficserver/blob/master/plugins/esi/esi.cc#L1346 as well.
The "post" request will be intercepted to update the cache. Changing it to PUT will make it not go through those code.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #7999: Do not invalidate cached resources upon error responses to unsafe methods
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999#issuecomment-870716123
> I think we need to change https://github.com/apache/trafficserver/blob/master/plugins/esi/esi.cc#L1346 as well.
>
> The "post" request will be intercepted to update the cache. Changing it to PUT will make it not go through those code.
Thank you, @shukitchan , for these observations. Updating this code again causes issues with the ESI plugin. I've now updated this PR to follow your initial advice and simply not exercise the packed-node feature.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt merged pull request #7999: Do not invalidate cached resources upon error responses to unsafe methods
Posted by GitBox <gi...@apache.org>.
bneradt merged pull request #7999:
URL: https://github.com/apache/trafficserver/pull/7999
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org