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.

Sign up for GitHub

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

Jump to bottom

Don't touch HTTP headers where possible #616

Open
nate-at-gusto opened this issue Jun 6, 2024 · 2 comments · May be fixed by #620
Open

Don't touch HTTP headers where possible #616

nate-at-gusto opened this issue Jun 6, 2024 · 2 comments · May be fixed by #620

Comments

@nate-at-gusto
Copy link
Contributor

Even if disable_caching is on, we still overwrite the applications HTTP cache headers.

This can make HTTP caching difficult to observe in development mode. I understand why we have hacks and workarounds here, but it would be better if we could avoid touching this stuff and let the app do it's thing.
@suryanarayanan035
Copy link

Hello @nate-at-gusto , we are modifying the HTTP header to prevent browser from storing cache only if disable_caching is on. So, I don't find any problems with adding no-store to HTTP header if disable caching is on and feel like its the right thing to do.

If you still find that we need to prevent modifying HTTP headers, then can you please explain me the problem we will be facing here if we just let it be?

@glaszig glaszig linked a pull request Aug 16, 2024 that will close this issue
Open
@glaszig
Copy link
Contributor

glaszig commented Aug 16, 2024 •
edited
Loading

adding no-store to HTTP header if disable caching is on

disable_caching controls whether no-store is added, yes, but still the header will be overridden, regardless, with must-revalidate, private, max-age=0:

rack-mini-profiler/lib/mini_profiler.rb

Lines 432 to 435 in 5e42a57

headers['X-MiniProfiler-Original-Cache-Control'] = headers['Cache-Control'] unless headers['Cache-Control'].nil?
headers['Cache-Control'] = "#{"no-store, " if config.disable_caching}must-revalidate, private, max-age=0"

i think there should also be a big fat warning at boot time when disable_caching == true. i just spent an hour or two digging around rails and rack to see wth i'm doing wrong.

please explain me the problem we will be facing here if we just let it be

the headers which I MYSELF have set are silently overridden and have no effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fully respect disable_caching option glaszig/rack-mini-profiler
3 participants
@glaszig @suryanarayanan035 @nate-at-gusto