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

Regression: ResourceHandlerRegistration setCachePeriod doesn't set the correct response header anymore [SPR-14005] #18577

Closed
spring-issuemaster opened this issue Mar 1, 2016 · 6 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Mar 1, 2016

Erko Hansar opened SPR-14005 and commented

We have a Spring MVC application with Spring Security around it. In the MVC config we add resourceHandlers for static files together with cache periods. And Spring Security applies the default header configuration.

@Configuration
public class MvcConfig extends DelegatingWebMvcConfiguration {

    @Override
    protected void addResourceHandlers(ResourceHandlerRegistry registry) {
        registry.addResourceHandler("/images/**").addResourceLocations("/images/").setCachePeriod(60 * 60 * 8);
    }

...

This has been working fine with Spring versions 4.2.1, 4.2.2, 4.2.3 and 4.2.4 combined with either Spring Security 4.0.2 or 4.0.4. The response contained the correct
Cache-Control:max-age=28800
header (and no Pragma header).

But after upgrading Spring to 4.2.5 it doesn't work anymore. Now the response contains:
Cache-Control:no-cache, no-store, max-age=0, must-revalidate
Pragma:no-cache

I also tried 4.2.6 and 4.3.0 BUILD-SNAPSHOTs, but the result was the same as with 4.2.5. There are a couple of issues related to Cache in 4.2.5 release notes, like #18390 and #18440, but I'm not sure if these are directly related.


Affects: 4.2.5

Issue Links:

  • #18440 ResponseEntity CacheControl ignored / extended by RequestMappingHandlerAdapter
  • #18625 Reset Expires header in WebContentGenerator when caching resources

Referenced from: commits 0ef90df, 50bcd67

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 1, 2016

Brian Clozel commented

This is probably related to #18440.
Can you share the relevant part of your Spring Security configuration so I can create a repro project? Especially the http part.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 1, 2016

Erko Hansar commented

Very straightforward config, I dropped some non-related parts:

        http
            //.headers()
            //    .and()
            .authorizeRequests()
                // allow public static resources
                .antMatchers("/css/**").permitAll()
                .antMatchers("/js/**").permitAll()
                .antMatchers("/images/**").permitAll()
                .antMatchers("/robots.txt").permitAll()
                .antMatchers("/sitemap.xml").permitAll()

                // allow public pages
                .antMatchers("/").permitAll()
                .antMatchers("/features").permitAll()
                .antMatchers("/pricing").permitAll()

                ...

                // deny everything else
                .anyRequest().denyAll()

                .and()
            .exceptionHandling()
                .authenticationEntryPoint(new LoginUrlAuthenticationEntryPoint("/signin"))
                .and()
            .addFilterBefore(createSmartCardAuthenticationFilter(), UsernamePasswordAuthenticationFilter.class)
            .sessionManagement()
                .and()
            .logout()
                .logoutUrl("/signout")
                .logoutSuccessUrl("/signin");

There is no difference if the .headers() part is commented out or in.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 1, 2016

Brian Clozel commented

Additional question, can you confirm that completely ignoring the resource paths in your Spring Security configuration reverts back to the previous behavior? Probably something like http.ignoringAntMatchers("/images/**")

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 1, 2016

Erko Hansar commented

Added the following code to our security config:

@Override
public void configure(WebSecurity web) throws Exception {
    web.ignoring()
            .antMatchers("/css/**")
            .antMatchers("/js/**")
            .antMatchers("/images/**");
}

Now spring 4.2.5 correctly outputs the
Cache-Control:max-age=28800
header. All other Spring Security headers are gone (as expected).

It would be helpful to mention this option somewhere in the Spring Security Reference docs.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 2, 2016

Brian Clozel commented

I've relaxed #18440 to only consider responses coming from Controller handlers. This should be available shortly in 4.2.6.BUILD-SNAPSHOT and 4.3.0.BUILD-SNAPSHOT.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 15, 2016

Rob Winch commented

This relates to spring-projects/spring-security#2953. In particular refer to the workaround that is listed (the one on this page is not advised).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.