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

ResourceRegions converter opens and closes InputStream multiple times for multiple HttpRanges #24210

Closed
randomnicode opened this issue Dec 14, 2019 · 1 comment
Labels
status: superseded An issue that has been superseded by another

Comments

@randomnicode
Copy link

randomnicode commented Dec 14, 2019

Affects: Spring Web 5.2.2

Trying to utilize the Range header in an HTTP request to download a (portion of a) dynamically generated resource. The operative code isn't too exciting:

@GetMapping
public ResponseEntity<Resource> handleRequest(...) {
  CustomInputStream is = ... //dynamically generated input stream with known fixed length
  return ResponseEntity.ok().headers(...).body(new CustomResource(is)); //InputStreamResource not used so Range requests may be accomodated
}

The issue is that for multiple byte ranges, the CustomInputStream underlying the Resource wrapper is closed multiple times: https://github.com/spring-projects/spring-framework/blob/master/spring-web/src/main/java/org/springframework/http/converter/ResourceRegionHttpMessageConverter.java#L182-L211. The CustomResource is provided to multiple ResourceRegion objects, each of which tries to open and close the underlying CustomInputStream separately. It also throws the following Exception (due to the InputStreamResource being opened again and again for multiple byte ranges):

java.lang.IllegalStateException: InputStream has already been read - do not use InputStreamResource if a stream needs to be read multiple times
	at org.springframework.core.io.InputStreamResource.getInputStream(InputStreamResource.java:97) ~[spring-core-5.2.2.RELEASE.jar:5.2.2.RELEASE]
	at org.springframework.http.converter.ResourceRegionHttpMessageConverter.writeResourceRegionCollection(ResourceRegionHttpMessageConverter.java:185) ~[spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
	at org.springframework.http.converter.ResourceRegionHttpMessageConverter.writeInternal(ResourceRegionHttpMessageConverter.java:139) ~[spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]
	at org.springframework.http.converter.AbstractGenericHttpMessageConverter.write(AbstractGenericHttpMessageConverter.java:104) ~[spring-web-5.2.2.RELEASE.jar:5.2.2.RELEASE]

This wouldn't be a big problem for static assets (streams can be opened and closed), but because the stream is being generated dynamically, it cannot be regenerated (for context, it is using a circular buffer where another stream is writing to its read buffer).

Overriding the close() method of the (Custom)InputStream to ignore and not actually close the stream is not a good solution, because then the stream remains open and does not know when to actually close (i.e., after the request has been serviced and all the HttpRange regions have been written).

Overriding the CustomResource may get rid of the exception, but the underlying problem remains. The underlying stream is being closed again and again at every iteration of the loop, and the next iteration tries opening it again.

For now a decent solution may be to reuse the InputStream if available and not close it at the end of every loop iteration. Only call close() on all used InputStreams once all the iterations are done.

Additionally, perhaps the concept of a ResourceRegion needs to be reimplemented? I would expect it to perhaps contain the one Resource (that all the ranges are built on) and one or more ranges. Otherwise, we seem to be creating multiple ResourceRegion objects each containing the same Resource object, and expecting it to be okay when all the different objects open and close the underlying stream of the same object.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 14, 2019
randomnicode added a commit to randomnicode/spring-framework that referenced this issue Dec 14, 2019
Dynamically generated input streams cannot be closed and reopened
multiple times. This fix reuses the same InputStream across multiple
ResourceRegion writes.

Closes spring-projects#24210
randomnicode added a commit to randomnicode/spring-framework that referenced this issue Dec 15, 2019
Dynamically generated input streams cannot be closed and reopened
multiple times. This fix reuses the same InputStream across multiple
ResourceRegion writes.

This is a proposal for and closes spring-projects#24210
randomnicode added a commit to randomnicode/spring-framework that referenced this issue Dec 15, 2019
Dynamically generated input streams cannot be closed and reopened
multiple times. This fix reuses the same InputStream across multiple
ResourceRegion writes.

This is a proposal for and closes spring-projects#24210
@rstoyanchev
Copy link
Contributor

Superseded by #24214.

@rstoyanchev rstoyanchev added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
3 participants