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

Race condition in HeaderWriterFilter when using asynchronous processing #15510

Open
chschu opened this issue Aug 2, 2024 · 0 comments
Open
Labels
status: waiting-for-triage An issue we've not yet triaged type: bug A general bug

Comments

@chschu
Copy link

chschu commented Aug 2, 2024

Describe the bug

When using HeaderWriterFilter and asynchronous processing (e. g. a Spring MVC controller method that returns a StreamingResponseBody), headers may be set twice from different threads, potentially leading to race conditions and undefined behaviour.

The following two things happen in parallel:

  • The asynchronous processing writes to the response body. After data has been written, the response is committed. This causes the HeaderWriterFilter to write headers from the asynchronous task's thread.
  • The execution of the HeaderWriterFilter on the way back through the filter chain. This will write headers from the original request handling thread.

Both threads may successfully pass the isDisableOnResponseCommitted() check in HeaderWriterFilter.HeaderWriterResponse#writeHeaders, causing all HeaderWriters to be invoked twice. This may cause duplicate headers, because even if the HeaderWriter checks if the header is already present, this check-and-add is usually not atomic.

It's getting even worse: The implementation of HttpServletResponse is not guaranteed to be thread safe according to the Servlet Spec.

When using Apache Tomcat, adding headers from different threads causes nasty race conditions that (because of Tomcat's instance recycling) can durably break the inner workings of their org.apache.tomcat.util.http.MimeHeaders class, potentially leading to errors in completely unrelated requests. I already experienced duplicate and missing response headers. As with most race conditions, these specific issues are almost impossible to reproduce.

Because other parts of the application may also add headers, this issue cannot probably be solved inside the HeaderWriterFilter alone.

To make things even worse, Tomcat itself also manipulates response headers internally: For example, the Vary response headers created by the application are combined into one if response compression is enabled. This happens in the same thread where the response is committed, but is probably out of Spring Security's reach.

To Reproduce

Create a Spring MVC (or Spring Boot) application that uses HeaderWriterFilter and has a @Controller method that returns a StreamingResponseBody. This is already sufficient to reproduce the duplicate headers.

Adding a HeaderWriter that adds a lot of headers helps reproducing the errors caused by adding headers from different threads.

Expected behavior

HeaderWriterFilter doesn't invoke its HeaderWriters from different threads at the same time. However, this might not be sufficient, as described above.

Sample

https://github.com/chschu/spring-security-header-writer-filter-async-bug

The sample also includes a workaround: Flush the response programmatically before starting the asynchronous processing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged type: bug A general bug
Projects
None yet
Development

No branches or pull requests

1 participant