Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The cache handler should not invalidate all requests to an origin #3806

Closed
ivan-tymoshenko opened this issue Nov 6, 2024 · 2 comments · Fixed by #3816
Closed

The cache handler should not invalidate all requests to an origin #3806

ivan-tymoshenko opened this issue Nov 6, 2024 · 2 comments · Fixed by #3816
Labels
bug Something isn't working

Comments

@ivan-tymoshenko
Copy link
Contributor

When the cache-handler recevies a succesful response from one of "unsafe" http methods such as POST/PUT/DELETE... it invalidates all other cached responses to this origin.

https://github.com/nodejs/undici/blob/main/lib/handler/cache-handler.js#L79

RFC says:

A cache MUST invalidate the target URI ([Section 7.1](https://www.rfc-editor.org/rfc/rfc9110#section-7.1) of [[HTTP](https://www.rfc-editor.org/rfc/rfc9111.html#HTTP)]) when it receives a non-error status code in response to an unsafe request method (including methods whose safety is unknown).

By "target URI" here I understand the the full URL and not just an origin (correct me here if I;m wrong). For example if a client receives a succesful response for the DELETE http://example.com/foo/bar the cache hanlder can delete the GET http://example.com/foo/bar, but not all other cached responses to the http://example.com.

IMHO With a current behaviour the cache seems useless, because a one regulary called POST requests keeps your cache almost always empty.

@ivan-tymoshenko ivan-tymoshenko added the bug Something isn't working label Nov 6, 2024
@mcollina
Copy link
Member

mcollina commented Nov 6, 2024

cc @flakey5

@Uzlopak
Copy link
Contributor

Uzlopak commented Nov 6, 2024

I agree, I also realized that like 2-3 weeks ago. I think I tried to solve this in my PR, which i did not touch since you commented on it.

flakey5 added a commit to flakey5/undici that referenced this issue Nov 8, 2024
Closes nodejs#3806

Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
flakey5 added a commit to flakey5/undici that referenced this issue Nov 8, 2024
Closes nodejs#3806

Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
flakey5 added a commit to flakey5/undici that referenced this issue Nov 8, 2024
Closes nodejs#3806

Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
flakey5 added a commit to flakey5/undici that referenced this issue Nov 8, 2024
Closes nodejs#3806

Signed-off-by: flakey5 <73616808+flakey5@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants