Timing for HeaderWriterFilter to writes headers changed in 4.2.5 breaks existing codes.

[tan9]

Summary

In Spring Security 4.2.4 or earlier, the HeaderWriterFilter writes headers before filter chin was processed.
However, commit f81b581#diff-57c0f670220b7f4e45a0d1252a99b482 in 4.2.5 changed the timing of header writing to response.onResponseCommitted phase. And this will break existing code which writes custom headers other than those defined in HeaderWriters. For example:

I have some URL intend to be embed in frames. In 4.2.4 or earlier, I can overwrite the default value from XFrameOptionsHeaderWriter as following :

    public ModelAndView relogin(HttpServletResponse response) {
        ModelAndView mav = new ModelAndView();
        mav.setViewName("security/relogin");

        response.setHeader("X-Frame-Options", "SAMEORIGIN"); // overwrite `DENY` in XFrameOptionsHeaderWriter
        return mav;
    }

Now I have no easy way to set X-Frame-Options to SAMEORIGIN in dedicate URLs while applying DENY to rest or the system.

Call sequence illustrated for above code snippet in 4.2.5.RELEASE:

HeaderWriterFilter.doFilterInternal
  filterChain.doFilter
    response.setHeader in contorller     <- manual header writing here (X-Frame-Options=SAMEORIGIN)
  response.onResponseCommitted           <- HeaderWriters writes header here (X-Frame-Options=DENY)

Resulting X-Frame-Options=DENY.

Call sequence illustrated for 4.2.4.RELEASE and earlier:

HeaderWriterFilter.doFilterInternal      <- HeaderWriters writes header here (X-Frame-Options=DENY)
  filterChain.doFilter
    response.setHeader in contorller     <- manual header writing here (X-Frame-Options=SAMEORIGIN)
  response.onResponseCommitted

Resulting X-Frame-Options=SAMEORIGIN as expected.

Actual Behavior

response.setHeader in controller code will not take effect as before.

Expected Behavior

Manual response.setHeader in controller code overwrites headers wrote by HeaderWriters.

Configuration

N/A

Version

4.2.5.RELEASE

Sample

N/A

[tan9]

Hi @rwinch , seems this issue is related to #5004, #5005, #4307, #3975, #2953, it really wired and confusing.

If the behavior is by design, should we document it well for programmer to follow

[jazdw]

@tan9 I agree with what you are saying, it definitely seems more obvious to me that the security headers should be set earlier and then the controller should be able to override them.

Another issue with it is that the OnCommittedResponseWrapper just doesn't seem to do its job reliably as I posted here - #3975 (comment)

[rwinch]

This does appear to be an issue. I think we need to modify it to check to see if the header is already set and if so not to override it. If anyone is interested in submitting a PR that would help get this issue resolved faster as we are focused on some other tasks at the moment.

[tan9]

@rwinch thanks for your response, I am willing to contribute. Would you please provide some direction how to do it right. Use another responseWrapper within HeaderWriterResponse to prevent headers wrote in filterChain been overwritten by headerWriters?

[rwinch]

Thanks for the fast response! I think we should just update the HeaderWriter implementations to only write if the value isn't present.

[koju]

Workarounds are mentioned in #2953.
JavaConfig: #2953 (comment)
XmlConfig: #2953 (comment)

[rwinch]

Closing this as a duplicate of gh-2953