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

All non 200 responses get cache-control=no-cache header #1984

Closed
candid82 opened this issue Sep 27, 2014 · 7 comments
Closed

All non 200 responses get cache-control=no-cache header #1984

candid82 opened this issue Sep 27, 2014 · 7 comments
Assignees
Labels
Milestone

Comments

@candid82
Copy link

@candid82 candid82 commented Sep 27, 2014

https://github.com/hapijs/hapi/blob/master/lib/response/headers.js#L56-L58

Nowhere RFC 2616 prescribes this behavior and the framework should not enforce it.
The use case in mind is to be able to cache 404.

@kpdecker
Copy link
Contributor

@kpdecker kpdecker commented Sep 29, 2014

301, 302, etc are also of concern here.

This is pretty big for us as there are a number of not found cases and redirects that we want to push our caching out to but with the way the header method is setup we can't even do an explicit header set to get the behavior that we need.

@kpdecker
Copy link
Contributor

@kpdecker kpdecker commented Oct 4, 2014

This does not feel like it should be a server level property, but the
route level. Doesn't it? If the original concern was that we want to be
sure that the user is very aware of what they are doing when caching it
seems logical to have that level of granularity (perhaps in addition to
server-wide)

On Saturday, October 4, 2014, Eran Hammer notifications@github.com wrote:

Closed #1984 #1984 via 32a287b
32a287b
.


Reply to this email directly or view it on GitHub
#1984 (comment).

Sent from my mobile device

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Oct 5, 2014

Feel free to open another issue for route level override.

@candid82
Copy link
Author

@candid82 candid82 commented Oct 6, 2014

I think what we really need is to have full control over cache-control header when we need it. cacheControlStatus option is not very useful, because once we add, say, 404 to cacheControlStatus array, all 404 responses will now be cached if cache attribute is set on a route config, which is definitely not what we want. How about hapi only applies its cache logic if cache-control is not already set by user's code explicitly? That is, if I do response.header('cache-control', myCacheControl);, this will be exactly what is returned to the client (right now hapi will override this if status is not in cacheControlStatus).
Sorry for not being clear in the original request.

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Oct 6, 2014

Open a new issue. Overriding existing cache-control is a bug.

@candid82
Copy link
Author

@candid82 candid82 commented Oct 6, 2014

Done. #2011

@lock
Copy link

@lock lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants