Skip to content

Add ability to avoid "no-store" in default Cache-Control header #6476

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

Closed
mickaeltr opened this issue Jan 23, 2019 · 6 comments
Closed

Add ability to avoid "no-store" in default Cache-Control header #6476

mickaeltr opened this issue Jan 23, 2019 · 6 comments
Assignees

Comments

@mickaeltr
Copy link

Summary

By default, Spring Security only allows turning off the default Cache-Control header. I would like to be able to customize it in order to avoid the "no-store" instruction.

Actual Behavior

Default Cache-Control header is: no-cache, no-store, max-age=0, must-revalidate

That does not allow storing the HTTP response (in any browser or proxy cache). Thus, HTTP bodies must always be downloaded, even if they have not changed.

Expected Behavior

I would like to always revalidate HTTP requests and get a 304 Not Modified when relevant, instead of downloading an HTTP body that could have been stored in a browser or proxy cache.

My ideal default Cache-Control header would be: no-cache, max-age=0, must-revalidate

Version

Spring Security 5.1.3

Thanks :)

@rwinch
Copy link
Member

rwinch commented Jan 23, 2019

If you want to do this you can disable the default cache control and then add static headers.

@rwinch rwinch closed this as completed Jan 23, 2019
@rwinch rwinch self-assigned this Jan 23, 2019
@mickaeltr
Copy link
Author

Hi @rwinch,

I tried your suggestion unfortunately I cannot get the expected behavior.

First I tried to use the CacheControlHeadersWriter class, but it isn't customizable.

Then I tried a basic StaticHeadersWriter:

http
      .headers()
      .cacheControl().disable()
      .addHeaderWriter(new StaticHeadersWriter(HttpHeaders.CACHE_CONTROL, "max-age=0, no-cache, must-revalidate"));

That works fine for most resources… but those with custom Cache-Control headers are not well-treated (values are concatenated):

return ResponseEntity.ok()
      .cacheControl(CacheControl.maxAge(1, TimeUnit.HOURS))
      .body(…);

give me this: Cache-Control: max-age=3600, max-age=0, no-cache, must-revalidate

Eventually I tried to extend the StaticHeadersWriter but it does not help because the headers list is private.

Any suggestion is welcome, thanks.

@rwinch
Copy link
Member

rwinch commented Jan 24, 2019

Thanks for the follow up @mickaeltr!

It appears the reason is that StaticHeadersWriter is adding the header without checking to see if it exists. We should add a boolean property to allow for that behavior. I created gh-6478 for it. Would you be interested in submitting a Pull Request for that? If so please claim it by commenting on the issue.

In the meantime you can do something like this:

class CustomStaticHeadersWriter implements HeaderWriter {
    private HeaderWriter delegate = new StaticHeadersWriter(HttpHeaders.CACHE_CONTROL, "max-age=0, no-cache, must-revalidate");
    public void writeHeaders(HttpServletRequest request, HttpServletResponse response) {
        if (response.containsHeader(HttpHeaders.CACHE_CONTROL)) {
            return;
        }
        delegate.writeHeaders(request, response);
    }
}

@mickaeltr
Copy link
Author

Thanks @rwinch, I'll let you know if I manage to setup Spring Security on my machine.

@mickaeltr
Copy link
Author

This is what my workaround looks like for the moment:

  private void allowHttpResponseStore(HttpSecurity http) throws Exception {
    // Default Cache-Control header set by Spring Security is: no-cache, no-store, max-age=0, must-revalidate
    // We removed the "no-store" so browsers and proxy caches can store the data, however they always must revalidate it
    // Thus when the response has not changed, we get a 304 Not Modified and we don't need to download it again
    http
      .headers()
      .cacheControl().disable()
      .addHeaderWriter(new DefaultCacheControlHeadersWriter("max-age=0", "no-cache", "must-revalidate"));
  }

  // Workaround for
  // - Add ability to avoid "no-store" in default Cache-Control header: https://github.com/spring-projects/spring-security/issues/6476
  // - Add StaticHeadersWriter.setIgnoreIfContainsHeader(boolean): https://github.com/spring-projects/spring-security/issues/6478
  private static class DefaultCacheControlHeadersWriter implements HeaderWriter {
    private final HeaderWriter delegate;

    private DefaultCacheControlHeadersWriter(String... cacheControlValues) {
      this.delegate = new StaticHeadersWriter(HttpHeaders.CACHE_CONTROL, cacheControlValues);
    }

    public void writeHeaders(HttpServletRequest request, HttpServletResponse response) {
      if (!response.containsHeader(HttpHeaders.CACHE_CONTROL)) {
        delegate.writeHeaders(request, response);
      }
    }
  }

@mickaeltr
Copy link
Author

Hello,
I've been asked how this could be done now, so here is my final solution:

private void allowHttpResponseStore(HttpSecurity http) throws Exception {
  http
    .headers()
    .cacheControl()
    .disable()
    .addHeaderWriter(new StaticHeadersWriter("no-cache", "max-age=0", "must-revalidate", "private"));
}

Hope this helps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants