https://github.com/spring-projects/spring-framework/blame/f85d5bd84a7e7bc810bb5a8179fc2fc130affc89/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java#L889
Why do we override response with async one always, it's breaks back compatibility for non async requests.
Latest working for me code was in 6.1.4 version (Spring Boot 3.2.3)
Comment From: bclozel
Can you elaborate on what is being broken for your application? Maybe share a minimal application showing the issue?
Comment From: ghost
Can you elaborate on what is being broken for your application? Maybe share a minimal application showing the issue?
Yes, to reproduce you can simply create servlet filter and create custom response class that will extend HttpServletResponseWrapper. This custom object of response we need to pass through filter chain. So previously in Spring controller if I inject HttpServletResponse as method parameter I will receive instance of that response object, but now it's wrappped with Lifecycle... wrapper that breaks my code because I expect original instance of response that I pass from filter. And this is regular request, so I don't understand why it's forced to be async. Since 6.1.4 I see that in 6.1.8 in invokation handler method code was reorganized and async is now enforced.
Comment From: jhoeller
@rstoyanchev this looks a side effect of your https://github.com/spring-projects/spring-framework/issues/32340 efforts in 6.1.5 - anything we can do about this regression for regular filter dispatching?
Comment From: rstoyanchev
The nature of request and response wrapping is such that it has to be done ahead of time in order to cover request handling, and can't be done on demand deeper within the callstack, and the wrapper becomes a pass through if not needed.
Request or response wrapping is common, and typically done by extending ServletResponseWrapper
, so you can always unwrap or find the instance you need. We have a couple of methods in WebUtils
called getNativeRequests
and getNativeResponse
that could help with that.
Comment From: ghost
The nature of request and response wrapping is such that it has to be done ahead of time in order to cover request handling, and can't be done on demand deeper within the callstack, and the wrapper becomes a pass through if not needed.
Request or response wrapping is common, and typically done by extending
ServletResponseWrapper
, so you can always unwrap or find the instance you need. We have a couple of methods inWebUtils
calledgetNativeRequests
andgetNativeResponse
that could help with that.
Yes, I can unwrap it, but I mean that it breaks existing code. If it's possible to have back compatibility it would be great.
Comment From: rstoyanchev
We could avoid the wrapping if request.isAsyncSupported()
returns false, but otherwise we just don't know if processing will switch to async mode.
Comment From: ghost
We could avoid the wrapping if
request.isAsyncSupported()
returns false, but otherwise we just don't know if processing will switch to async mode.
I think checking whether the request is currently asynchronous at the point of invocation handling using request.isAsyncStarted() could be a good compromise. If the request hasn't been switched to async mode by this stage, then ideally, we should not wrap it, as it would ensure that synchronous operations are not unnecessarily affected.
Comment From: rstoyanchev
You're mixing up the order of things. At the point of invocation it is not yet known whether async processing will be started. After the point of invocation it is too late to wrap.
Comment From: ghost
Ok, thx for clarification. I will change my code to unwrap to what I need.