-
Notifications
You must be signed in to change notification settings - Fork 38.5k
IllegalStateException: InputStream has already been read when returning ResponseEntity<InputStreamResource> [SPR-16754] #21295
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
Comments
Stéphane Nicoll commented
Can you please share a sample we can run ourselves? Just to make sure we can eliminate a maximum of potential sources. Writing code in a comment is not what I've had in mind. If you see "frequently" those errors, you should be able to provide a sample that can reproduce it. Thank you. |
Jessy James commented I will provide an SSCCE in the hope that this will give any insights. However don't expect that to be any different from that sample code and also don't expect that to fail. I was not able to reproduce that issue locally. I can see those errors when I deploy the Boot-2 upgrade to production traffic and wait a few seconds to see some requests to fail. The thing is, this error clearly happens outside of client code. The last line of the sample is creating that InputStreamResource() and anything after that is framework land, which calls getInputStream() for a second time. |
Jessy James commented I think I have something more useful now. This is the stack trace of a first getInputStream() (which doesn't fail):
You see that this is a very different stack trace than the one for the second failing call. Also everything is related to AbstractResource.contentLength(). During my pre ticket research I noticed related tickets also discussing contentLength(). So was there a recent change in that space which could have introduced this regression? |
Jessy James commented I can now successfully reproduce the issue. It happens only when continuing a download with a Range header. With this information I can happily create a SSCCE which will fail for ranged requests. |
Jessy James commented Sorry, I was too curious, so I debugged it directly. There is HttpRange.toResourceRegion() which even says in the comment don't call InputStreamResource.contentLenght(), then the assertion fails and gets catched in AbstractMessageConverterMethodProcessor.writeWithMessageConverters(). To me it seems that Spring starts trying to handle range requests on InputStreamResource which it didn't before. Generally I would welcome such a change, but this has to happen in collaboration with the client code. The framework can't just assume that |
Jessy James commented And here is your SSCCE: spring-attic/spring-framework-issues#178 |
Jessy James commented I think 582014e944002609b562f6e4998935c06bbe6922 is the offending commit which references also #20344, #18407. So it really seems the framework now wants to support range requests on purpose. Then I suggest revising those tasks and addressing those three points I mentioned earlier:
|
Zhang Jie commented Spring will handle range request if only the request contains a valid Range header. And else, InputStreamResource Should only be used if no other specific Resource implementation |
Jessy James commented
I looked into that as I was looking for a workaround. I failed to see how:
|
Zhang Jie commented Yes I agree with you, maybe it would be better that made range request handling configurable(default is "not ignore"). And also obey several MUST ignore(e.g. A server MUST ignore a Range header field received with a request method other than GET) |
Brian Clozel commented I've just pushed a fix that should be shortly available in This fix disables completely HTTP Range support if the return type is an
Now I feel that we got a little sidetracked here in the issue comments. I think that supporting HTTP Range requests when the return type is a Please try the latest SNAPSHOT with your application and check back here with your findings. I'm closing this issue for now but I'm happy to discuss further improvements. Thanks for the report! |
Jessy James commented The issue is fixed for me in 5.0.6.BUILD-SNAPSHOT. Thanks for the quick fix.
I sure would love to have the framework handle all the ranged request logic. My use case is serving a stream from an origin server (think of a proxy). For that I have to handle ranged requests in the same way as the origin does. Some don't have a content length, some don't support ranged requests at all. I would of course appreciate any framework related support in this space. |
Jessy James opened SPR-16754 and commented
With the recent upgrade to Spring-Boot-2, also Spring-5 came in and I guess it is related to the upgrade from 4 to 5. So here's the issue. I have a controller which provides a download for the user by returning a ResponseEntity<InputStreamResource>. After the recent upgrade I now see frequently these errors in the log:
I tried some scenarios locally and couldn't reproduce it. Also it happens not all the time, just sometimes it fails in said ways. The stack trace doesn't contain anything from my code. With Spring-Boot-1 I never had those errors.
This is an exemplary extract of my Controller:
Affects: 5.0.5
Reference URL: spring-projects/spring-boot#12939
Issue Links:
Referenced from: commits 72cfe41, e9a8a50
The text was updated successfully, but these errors were encountered: