spring-projects / spring-projects/spring-security

Race condition in HeaderWriterFilter when using asynchronous processing

Open
#15,510 1 comment 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: waiting-for-triage type: bug
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with HeaderWriterFilter.HeaderWriterResponse#writeHeaders and reproduce the behavior using the linked Spring MVC sample with a StreamingResponseBody. Trace both the asynchronous response commit and the return through the filter chain, then verify that HeaderWriters are not invoked concurrently or twice under the reproduction.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, spring
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.