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

Request body as cache key issues #3

Closed
vytas7 opened this issue Dec 26, 2019 · 1 comment
Closed

Request body as cache key issues #3

vytas7 opened this issue Dec 26, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@vytas7
Copy link

vytas7 commented Dec 26, 2019

It is quite unusual for GET or HEAD requests to have a non-empty body at all, but even for cached responders that do handle the request body, the stream would have been emptied and left at EOF if completely consumed.

Therefore, either would these responders interfere with the cache key logic, or the cache middleware would consume the stream leaving nothing for requests.

Ref:

request_body = req.stream.read(req.content_length or 0)

@zoltan-fedor
Copy link
Owner

Thanks.
Yes, I was wondering about this part for a while, whether it is worth including or not.
Based on your insight regarding the emptied stream not being available for the request, it doesn't worth it, so for now I will just remove the request body from the cache key and leave only the path and the method.
If later a request comes up to include the request body, then I will reconsider how to include it while avoiding emptying the stream (maybe refill it?) or maybe by storing it on the req.context to make it available to the responder.

See 5591191

@zoltan-fedor zoltan-fedor added the bug Something isn't working label Dec 26, 2019
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

No branches or pull requests

2 participants