Open SpComb opened 4 years ago
Also just to confirm that it's the same jetty Response
being called from the two threads: the _fields: instance of org.eclipse.jetty.http.HttpFields(id=13856)
matches across both threads
Breakpoint hit: "thread=task-24", org.eclipse.jetty.server.Response.addHeader(), line=627 bci=0
task-24[1] where
[1] org.eclipse.jetty.server.Response.addHeader (Response.java:627)
[2] javax.servlet.http.HttpServletResponseWrapper.addHeader (HttpServletResponseWrapper.java:212)
[3] org.springframework.security.web.firewall.FirewalledResponse.addHeader (FirewalledResponse.java:55)
[4] javax.servlet.http.HttpServletResponseWrapper.addHeader (HttpServletResponseWrapper.java:212)
[5] org.springframework.security.web.util.OnCommittedResponseWrapper.addHeader (OnCommittedResponseWrapper.java:63)
[6] org.springframework.http.server.ServletServerHttpResponse.lambda$writeHeaders$0 (ServletServerHttpResponse.java:104)
[7] org.springframework.http.server.ServletServerHttpResponse$$Lambda$1241.488706756.accept (null)
[8] java.util.Map.forEach (Map.java:661)
[9] org.springframework.http.server.ServletServerHttpResponse.writeHeaders (ServletServerHttpResponse.java:102)
[10] org.springframework.http.server.ServletServerHttpResponse.getBody (ServletServerHttpResponse.java:83)
[11] org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitterReturnValueHandler$StreamingServletServerHttpResponse.getBody (ResponseBodyEmitterReturnValueHandler.java:278)
[12] org.springframework.http.converter.StringHttpMessageConverter.writeInternal (StringHttpMessageConverter.java:122)
[13] org.springframework.http.converter.StringHttpMessageConverter.writeInternal (StringHttpMessageConverter.java:44)
[14] org.springframework.http.converter.AbstractHttpMessageConverter.write (AbstractHttpMessageConverter.java:227)
[15] org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitterReturnValueHandler$HttpMessageConvertingHandler.sendInternal (ResponseBodyEmitterReturnValueHandler.java:210)
[16] org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitterReturnValueHandler$HttpMessageConvertingHandler.send (ResponseBodyEmitterReturnValueHandler.java:203)
[17] org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitter.sendInternal (ResponseBodyEmitter.java:189)
[18] org.springframework.web.servlet.mvc.method.annotation.ResponseBodyEmitter.send (ResponseBodyEmitter.java:183)
[19] org.springframework.web.servlet.mvc.method.annotation.SseEmitter.send (SseEmitter.java:126)
[20] org.springframework.web.servlet.mvc.method.annotation.ReactiveTypeHandler$SseEmitterSubscriber.send (ReactiveTypeHandler.java:365)
[21] org.springframework.web.servlet.mvc.method.annotation.ReactiveTypeHandler$AbstractEmitterSubscriber.run (ReactiveTypeHandler.java:308)
[22] java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1,128)
[23] java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:628)
[24] java.lang.Thread.run (Thread.java:834)
task-24[1] dump this
this = {
LOG: instance of org.eclipse.jetty.util.log.Slf4jLog(id=13840)
__MIN_BUFFER_SIZE: 1
__EXPIRES_01JAN1970: instance of org.eclipse.jetty.http.PreEncodedHttpField(id=13841)
SET_INCLUDE_HEADER_PREFIX: "org.eclipse.jetty.server.include."
_channel: instance of org.eclipse.jetty.server.HttpChannelOverHttp(id=13855)
_fields: instance of org.eclipse.jetty.http.HttpFields(id=13856)
_errorSentAndIncludes: instance of org.eclipse.jetty.util.AtomicBiInteger(id=13857)
_out: instance of org.eclipse.jetty.server.HttpOutput(id=13858)
_status: 200
_reason: null
_locale: null
_mimeType: null
_characterEncoding: null
_encodingFrom: instance of org.eclipse.jetty.server.Response$EncodingFrom(id=13846)
_contentType: null
_outputType: instance of org.eclipse.jetty.server.Response$OutputType(id=13847)
_writer: null
_contentLength: -1
_trailers: null
__localeOverride: instance of java.util.RegularEnumSet(id=13848)
__explicitCharset: instance of java.util.RegularEnumSet(id=13849)
}
task-24[1] cont
Breakpoint hit: "thread=qtp761680655-29", org.eclipse.jetty.server.Response.setHeader(), line=577 bci=0
qtp761680655-29[1] dump this
this = {
LOG: instance of org.eclipse.jetty.util.log.Slf4jLog(id=13840)
__MIN_BUFFER_SIZE: 1
__EXPIRES_01JAN1970: instance of org.eclipse.jetty.http.PreEncodedHttpField(id=13841)
SET_INCLUDE_HEADER_PREFIX: "org.eclipse.jetty.server.include."
_channel: instance of org.eclipse.jetty.server.HttpChannelOverHttp(id=13855)
_fields: instance of org.eclipse.jetty.http.HttpFields(id=13856)
_errorSentAndIncludes: instance of org.eclipse.jetty.util.AtomicBiInteger(id=13857)
_out: instance of org.eclipse.jetty.server.HttpOutput(id=13858)
_status: 200
_reason: null
_locale: null
_mimeType: null
_characterEncoding: null
_encodingFrom: instance of org.eclipse.jetty.server.Response$EncodingFrom(id=13846)
_contentType: "text/event-stream"
_outputType: instance of org.eclipse.jetty.server.Response$OutputType(id=13860)
_writer: null
_contentLength: -1
_trailers: null
__localeOverride: instance of java.util.RegularEnumSet(id=13848)
__explicitCharset: instance of java.util.RegularEnumSet(id=13849)
}
qtp761680655-29[1] cont
Looking at the code, the
ResponseBodyEmitter
created by theResponseBodyEmitterReturnValueHandler
/ReactiveTypeHandler
should not be writing any HTTP response headers outside of theextendResponse
called in the servlet thread, but theReactiveTypeHandler
subscription'sSseEmitter.send
call in theTaskExecutor
thread is also invokingServletServerHttpResponse.writeHeaders
via theHttpMessageConvertingHandler.sendInternal
.
Looking a little further, that analysis of how the async servlet request/filter response header processing works isn't exactly correct. The ResponseBodyEmitterReturnValueHandler.handleReturnValue
-> SseEmitter.extendResponse
isn't writing any response headers, they're just being passed to the HttpMessageConvertingHandler
in the outputMessage
, and I suppose the expected behavior is for the servlet request to be handed off to the async handler thread for writing?
I suppose the key to reproducing this issue is probably our use of spring-security, and the HeaderWriterFilter in particular... the problematic code path involves doHeadersAfter conditionally invoking writeHeaders
based on the OnCommittedResponseWrapper's onResponseCommitted()
state... I guess that's what actually racing here, causing both the HeaderWriterFilter
running in the jetty servlet thread's filter chain as well as the ServletServerHttpResponse
used in the async handler thread to race and both set the servlet response headers concurrently?
java.lang.NullPointerException: null
at org.eclipse.jetty.http.HttpFields.put(HttpFields.java:630) ~[jetty-http-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.http.HttpFields.put(HttpFields.java:658) ~[jetty-http-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.Response.setHeader(Response.java:592) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at javax.servlet.http.HttpServletResponseWrapper.setHeader(HttpServletResponseWrapper.java:203) ~[javax.servlet-api-3.1.0.jar!/:3.1.0]
at org.springframework.security.web.firewall.FirewalledResponse.setHeader(FirewalledResponse.java:49) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.header.writers.frameoptions.XFrameOptionsHeaderWriter.writeHeaders(XFrameOptionsHeaderWriter.java:99) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.header.HeaderWriterFilter.writeHeaders(HeaderWriterFilter.java:100) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.header.HeaderWriterFilter$HeaderWriterResponse.writeHeaders(HeaderWriterFilter.java:139) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:94) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:77) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:105) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:334) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:215) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:178) ~[spring-security-web-5.2.2.RELEASE.jar!/:5.2.2.RELEASE]
at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:358) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:271) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1604) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1604) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1604) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:109) ~[spring-boot-actuator-2.2.6.RELEASE.jar!/:2.2.6.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1604) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119) ~[spring-web-5.2.5.RELEASE.jar!/:5.2.5.RELEASE]
at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1604) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:545) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:590) ~[jetty-security-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1610) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1300) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:485) ~[jetty-servlet-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1580) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1215) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.StatisticsHandler.handle(StatisticsHandler.java:173) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.Server.handle(Server.java:500) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:383) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:547) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:375) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:273) ~[jetty-server-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311) ~[jetty-io-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:103) ~[jetty-io-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.io.ChannelEndPoint$2.run(ChannelEndPoint.java:117) ~[jetty-io-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:336) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:313) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:171) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:129) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:375) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:806) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:938) ~[jetty-util-9.4.27.v20200227.jar!/:9.4.27.v20200227]
at java.base/java.lang.Thread.run(Thread.java:834) ~[na:na]
@SpComb we faced the same issue recently. What I found was that we had two web configurations one for @EnableWebSecurity and one for @EnableWebMvc which were interfering with each other. The solution that worked for us was to headers().disable() in @EnableWebSecurity
@Override
protected void configure(HttpSecurity http) throws Exception{
http.headers().disable()...;
Hope this helps
I'm a little confused. On the one hand you say:
We recently converted our SSE controller from returning a
SseEmitter
to returning aFlux<ServerSentEvent<?>>
, whereupon we started seeing sporadic NullPointerExceptions
On the other hand, the stacktraces and the explanations are about SseEmitter
.
Never mind, I see now.
So we subscribe immediately to the Flux and start writing from a scheduled thread (to avoid blocking the Flux). Meanwhile, as the main thread is on its way out of the filter chain, Spring Security also tries to write to the headers, is this correct?
@rwinch can you take a look at the stacktrace for Spring Security. What I don't understand is how it could write a header reliably after the DispatcherServlet
given that the response may have been committed? Perhaps there are some checks or guards missing.
Never mind, I see now.
Yes, the ReactiveTypeHandler
adapts the Flux<ServerSentEvent<?>>
into a SseEmitter
.
So we subscribe immediately to the Flux and start writing from a scheduled thread (to avoid blocking the Flux). Meanwhile, as the main thread is on its way out of the filter chain, Spring Security also tries to write to the headers, is this correct?
That is my interpretation. My understanding is that this issue is down to how async servlet responses and servlet filter chains interact, but my knowledge of async servlet processing is too weak to say how it should be fixed.
@rwinch can you take a look at the stacktrace for Spring Security. What I don't understand is how it could write a header reliably after the DispatcherServlet given that the response may have been committed? Perhaps there are some checks or guards missing.
The org.springframework.security.web.util.OnCommittedResponseWrapper
used by the spring security header filter shows up in the ReactiveTypeHandler
's flux thread, so I suppose it's attempting to intercept the response-committing, but that logic is also racing between the jetty servlet filter thread and the flux thread?
@rstoyanchev It looks like @SpComb analysis is correct. Spring Security's OnComittedResponseWrapper
is detecting that the response is about to be committed, so it then tries to write the headers before allowing the operation that commits the response.
I'm curious what happens if you set HeaderWriterFilter.shouldWriteHeadersEagerly = true
using something like this:
@Override
protected void configure(HttpSecurity http) throws Exception {
http
.headers(headers -> headers
.withObjectPostProcessor(new ObjectPostProcessor<HeaderWriterFilter>() {
@Override
public HeaderWriterFilter postProcess(HeaderWriterFilter headerWriterFilter) {
headerWriterFilter.setShouldWriteHeadersEagerly(true);
return headerWriterFilter;
}
})
)
...
This ensures the headers are written immediately. However, this can cause problems with cache control. In particular if someone overrides the cache control header to anything but Cache-Control: no-cache, no-store, max-age=0, must-revalidate Pragma: no-cache
it can cause issues since Spring Security will also attempt to set other cache related headers like Expires: 0
. The problem is that servlet spec allows the value of a header to be overridden, but not removed (which is what should happen).
@rwinch so if I understand correctly what happens is this. For a Flux<ServerSentEvent>
we subscribe immediately and when values are published, we switch to a different thread to absorb the blocking write. The first write commits the response and Spring Security's FirewalledResponse
gets involved via OnCommittedResponseWrapper
. In the mean time we are also exiting the Servlet container thread and in the filter chain FirewalledResponse
gets involved as well, possibly in parallel.
Assuming the cause for the NPE is this attempt to set the same header from two threads, I'm not quite sure what we can do in the Spring Framework to protect. Synchronizing in ServletServerHttpResponse#writeHeaders
won't help against races with any other uses of the HttpServletResponse
that don't go through the ServletServerHttpResponse
wrapper. It seems that such synchronization would be more effective in Spring Security which aims to intercept both the case where a commit occurs and also the case of exiting the filter chain. As is the case here those two could compete with each other.
@SpComb I wonder if you've tried again after the fix for https://github.com/eclipse/jetty.project/issues/4860, has that changed anything for your case?
At a minimum, the FirewalledResponse
should be aware of the value on HttpServletResponse.isCommitted()
not attempt to do anything if that returns true (indicating the response is committed and no header manipulation is now possible)
Any new information about this issue? We're affected by this as well and we can't use the mentioned fix by @rwinch because we use cache-control.
The problematic spring backend in our case did not need to override any Cache-Control headers, so we were able to deploy the headerWriterFilter.setShouldWriteHeadersEagerly(true)
workaround. We can confirm that we are no longer seeing these NPEs as a result.
We also encountered this sporadically when utilizing ResponseEntity<StreamingResponseBody>
. Can confirm that headerWriterFilter.setShouldWriteHeadersEagerly(true)
fixes it for us, too.
We have encountered this, however headerWriterFilter.setShouldWriteHeadersEagerly(true)
does not appear to solve the problem on our end.
Also faced with the same problem, when using ResponseEntity<StreamingResponseBody>
.
And headerWriterFilter.setShouldWriteHeadersEagerly(true)
just reduce error rate.
My team has seen sporadic test failures for all async endpoints that use StreamingResponseBody
or, more generally, any async endpoints that use the WebAsyncManager.startCallableProcessing
code path. All of these failures are due to ConcurrentModificationException
in Spring Security’s HeaderWriterFilter.writeHeaders
. Some occur in the original dispatch thread, while unwinding the filter chain; the rest occur in the async dispatch thread when our code begins writing to the response.
The crux of the problem seems to be that OnCommittedResponseWrapper
is used in multiple threads, but isn’t thread-safe. One simple solution would be to make it thread-safe. Its boolean disableOnCommitted
field that’s used here:
private void doOnResponseCommitted() {
if (!this.disableOnCommitted) {
onResponseCommitted();
disableOnResponseCommitted();
}
}
…could be changed to final AtomicBoolean
and used thus:
private void doOnResponseCommitted() {
if (!this.disableOnCommitted.getAndSet(true)) {
onResponseCommitted();
}
}
An even simpler solution would be for HeaderWriterFilter
to stop trying to write to the response from two different threads. Application code normally commits any given HTTP response from exactly one thread. If HeaderWriterFilter
just trusted OnCommittedResponseWrapper
to do its thing, there would be no race condition. That would mean deleting the finally
block below:
try {
filterChain.doFilter(headerWriterRequest, headerWriterResponse);
}
finally {
headerWriterResponse.writeHeaders();
}
Any reason not to?
For the time being, the workaround I’ve adopted is adding a response.flushBuffer();
call just before the return
statement in each affected async endpoint. That call commits the response synchronously in the main dispatch thread before async processing begins—before the race condition can develop.
We are also affected by this bug when returning ResponseEntity<StreamingResponseBody>
.
We are also affected via https://github.com/spring-projects/spring-framework/issues/31543.
Is there any idea how to fix this? We are seeing flaky tests on CI.
@sbrannen Is there anything you could think of that we can do in out spring integration tests to work-around that issue until this underlying issue resolved?
Help is appreciated!
We're also affected. Just seen this case yesterday on our production logs. We're not using the HttpServletResponse in our endpoints but returning ResponseEntity<StreamingResponseBody>
, so we cannot apply the flushBuffer workaround without some re-doing (unless I misundertood and the workaround is still returning the responsenentity but response.flushbuffer anyway before that return without a finally, a code example would be nice). We wanted to set the cache control for some of the endpoints but not all of them plus adding always some custom headers, so we're not sure what to apply here in the mean time.
Like the previous post to this one says, help here will be appreciated.
Edit: in case it helps here's the stacktrace in our case, we can see that it's not related to cache control but Mime and Tomcat:
java.io.IOException: Issue when transferring file
at com.custom.package.TransferDto.lambda$toResponse$0(TransferDto.java:27)
at org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBodyReturnValueHandler$StreamingResponseBodyTask.call(StreamingResponseBodyReturnValueHandler.java:110)
at org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBodyReturnValueHandler$StreamingResponseBodyTask.call(StreamingResponseBodyReturnValueHandler.java:97)
at org.springframework.web.context.request.async.WebAsyncManager.lambda$startCallableProcessing$4(WebAsyncManager.java:341)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at io.opentelemetry.context.Context.lambda$wrap$1(Context.java:212)
at com.custom.package.config.AsyncConfig$MdcTaskDecorator.lambda$decorate$0(AsyncConfig.java:123)
at io.opentelemetry.javaagent.bootstrap.executors.ContextPropagatingRunnable.run(ContextPropagatingRunnable.java:37)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.lang.NullPointerException: Cannot invoke "org.apache.tomcat.util.http.MimeHeaderField.getName()" because "this.headers[i]" is null
at org.apache.tomcat.util.http.MimeHeaders.getValue(MimeHeaders.java:322)
at org.apache.coyote.CompressionConfig.useCompression(CompressionConfig.java:198)
at org.apache.coyote.http11.AbstractHttp11Protocol.useCompression(AbstractHttp11Protocol.java:395)
at org.apache.coyote.http11.Http11Processor.prepareResponse(Http11Processor.java:920)
at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:377)
at org.apache.coyote.Response.action(Response.java:208)
at org.apache.coyote.http11.Http11OutputBuffer.doWrite(Http11OutputBuffer.java:187)
at org.apache.coyote.Response.doWrite(Response.java:616)
at org.apache.catalina.connector.OutputBuffer.realWriteBytes(OutputBuffer.java:331)
at org.apache.catalina.connector.OutputBuffer.flushByteBuffer(OutputBuffer.java:773)
at org.apache.catalina.connector.OutputBuffer.append(OutputBuffer.java:676)
at org.apache.catalina.connector.OutputBuffer.writeBytes(OutputBuffer.java:379)
at org.apache.catalina.connector.OutputBuffer.write(OutputBuffer.java:357)
at org.apache.catalina.connector.CoyoteOutputStream.write(CoyoteOutputStream.java:97)
at org.springframework.security.web.util.OnCommittedResponseWrapper$SaveContextServletOutputStream.write(OnCommittedResponseWrapper.java:639)
at java.base/java.io.InputStream.transferTo(InputStream.java:783)
at com.custom.package.TransferDto.lambda$toResponse$0(TransferDto.java:24)
... 11 common frames omitted
Here's our spring security dependencies:
[INFO] +- org.springframework.boot:spring-boot-starter-security:jar:3.1.8:compile
[INFO] | +- org.springframework:spring-aop:jar:6.0.16:compile
[INFO] | +- org.springframework.security:spring-security-config:jar:6.1.6:compile
[INFO] | \- org.springframework.security:spring-security-web:jar:6.1.6:compile
Another one with the same issue here. Getting the exception sporadically from controllers which return ResponseEntity<StreamingResponseBody>
. And yes I'm writing Cache-Control headers. (Exception example at the end of the post).
My setup is Spring Boot: 3.2.3 with embedded Tomcat.
I recently migrated from Spring Boot 2 + Jetty, and I have never seen this exception before migration. It only started with SB3 + Tomcat. But the exception source is clearly Tomcat in my case. I can see that this issue persists for a long time already, and even though exception occurs once in 2-3 days, it gets me alerted every time.
Does anybody experience the same issue with Jetty (and SpringBoot 3) ?
Also, I don't know if this is connected, but I've recently noticed that images were loading slower. I checked CDN statuses and was surprised to see BYPASS
statuses. My ResponseEntity<StreamingResponseBody>
image controllers were setting Cache-Control: public
headers, but for some reason, there was a 2nd pair of Cache-Control
header with a private
value. Of course, private
is what was causing BYPASS
and slow loading. When I debugged the app, my code was not setting private
anywhere, it was added somewhere implicitly. So the way I fixed it, is by how I add this header.
Previously I did it as follows:
final HttpHeaders headers = new org.springframework.http.HttpHeaders();
headers.setCacheControl(<my public value>);
...
return new ResponseEntity<>(stream, headers, HttpStatus.OK);
But now directly on instance of HttpServletResponse
:
response.setHeader("Cache-Control", "<my public value>");
and the problem with CDN is solved.
I still use HttpHeaders
instance for other headers, but Cache-Control
I set explicitly on the HttpServletResponse
instance.
So, I wonder, what happens if I set all headers directly on the HttpServletResponse
and not via org.springframework.http.HttpHeaders
...
Here's the relevant part of the stacktrace:
java.lang.NullPointerException: Cannot invoke "org.apache.tomcat.util.http.MimeHeaderField.getName()" because "this.headers[i]" is null
at org.apache.tomcat.util.http.MimeHeaders.getValue(MimeHeaders.java:322)
at org.apache.tomcat.util.http.MimeHeaders.getHeader(MimeHeaders.java:354)
at org.apache.coyote.Response.containsHeader(Response.java:365)
at org.apache.catalina.connector.Response.containsHeader(Response.java:1082)
at org.apache.catalina.connector.ResponseFacade.containsHeader(ResponseFacade.java:309)
at jakarta.servlet.http.HttpServletResponseWrapper.containsHeader(HttpServletResponseWrapper.java:65)
at jakarta.servlet.http.HttpServletResponseWrapper.containsHeader(HttpServletResponseWrapper.java:65)
at org.springframework.security.web.header.writers.HstsHeaderWriter.writeHeaders(HstsHeaderWriter.java:156)
at org.springframework.security.web.header.HeaderWriterFilter.writeHeaders(HeaderWriterFilter.java:99)
at org.springframework.security.web.header.HeaderWriterFilter$HeaderWriterResponse.writeHeaders(HeaderWriterFilter.java:132)
at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:93)
at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75)
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:116)
Should probably change the title of this issue as it is not unique to Jetty or Jetty's HttpFields
object anymore.
A comment above says it only happened to them after upgrading to Spring Boot 3, but we've been experiencing the same issue in Spring Boot 2.7.18 (and many earlier versions)
StreamingResponseBody
shouldWriteHeadersEagerly = true
seems to reduce the probability of the error occurring, but doesn't eliminate itClientAbortException
s
Affects: 5.2.5.RELEASE
We have a Spring Web MVC + jetty application using SSE for notifications. We recently converted our SSE controller from returning a
SseEmitter
to returning aFlux<ServerSentEvent<?>>
, whereupon we started seeing sporadicNullPointerException
s from jettyResponse.setHeader
calls for something on the order of 0.02% of requests for these SSE endpoints:Looking at the jetty
HttpFields
implementation, these NPEs seem to be caused by a corrupted internal_fields
state containingnulls
.With jetty 9.4.20 these exceptions cause the connection to be aborted, but with newer version of jetty (at least 9.4.27) these corrupted
HttpFields
put the jettyHttpChannel
into a busyloop that crashes the entire server: https://github.com/eclipse/jetty.project/issues/4860Investigating the underlying NPE issue together with the jetty maintainers we were unable to find any direct calls to the jetty APIs that would introduce
null
values in theHttpFields
state, but looking at the implementation of the jettyHttpFields
structure used inResponse.setHeaders/addHeaders
etc, it is suspectible to internal state corruption if called concurrently from multiple threads, and is documented as non-threadsafe:This kind of corruption caused by concurrent multi-threaded accesses also matches up with our observation of very rarely occuring state corruption/crashes. And indeed, using
jdb
to trace calls to these header-related methods for a single HTTP request, I can see calls from multiple different threads!Some of the calls are from the jetty server thread, via the servlet filter chain, ~which is the correct usage~:
However, there are also calls to
addHeader
from theThreadPoolExecutor
used in the spring frameworkResponseBodyEmitterReturnValueHandler
implementation, via theorg.springframework.web.servlet.mvc.method.annotation.ReactiveTypeHandler$SseEmitterSubscriber
:~Looking at the code, the
ResponseBodyEmitter
created by theResponseBodyEmitterReturnValueHandler
/ReactiveTypeHandler
should not be writing any HTTP response headers outside of theextendResponse
called in the servlet thread, but theReactiveTypeHandler
subscription'sSseEmitter.send
call in theTaskExecutor
thread is also invokingServletServerHttpResponse.writeHeaders
via theHttpMessageConvertingHandler.sendInternal
.~ EDIT: see comment below on spring-security HeaderWriterFilter behavior insteadComplete error log and
jdb
traces atttached:EDIT: edited stack traces to censor custom package names; use the first
task-9
thread stack from the jdb traces as the example