Issue
Currently, when returning a subclass of InputStreamResource
from a controller, ResourceHttpMessageWriter
from the Spring Web module doesn't consider subclasses of InputStreamResource
when deciding whether or not to call the contentLength
method on the given Resource
, it only considers the own InputStreamResource
class.
The result is: subclasses of InputStreamResource
must provide an override for the given method or risk having the whole input stream read by the default implementation of Resource#contentLength
, which could be very undesirable and lead to errors such as:
java.lang.IllegalStateException: InputStream has already been read - do not use InputStreamResource if a stream needs to be read multiple times
This issue doesn't happen if you return a ResponseEntity<Resource>
from your controller and make sure to set the Content-Length
header because there's no need to check for the length of the resource. But that's not always the case, there are cases where the final size of the input stream is undetermined, and for these cases, Spring already does handle these cases and properly sets and uses chunked
as the transfer-encoding
header, but only when returning an actual InputStreamResource
. This is the point I'm going to propose a change.
Affects: All versions up to Spring Framework 6.1.10 / Spring Boot 3.3.1 (and possibility newer versions too).
Proposed Solution
To fix this, I propose a simple, yet significant change in the Spring Web module, changing the check being made in this line:
https://github.com/spring-projects/spring-framework/blob/9b58e1ff71eeb6e47d3ab8dbedc1f796cf3eb17c/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java#L191
to this
InputStreamResource.class.isAssignableFrom(resource.getClass())
Here's a simple test code I ran to double-check that the current check doesn't support subclasses of InputStreamResource
.
public class JavaClass {
public static void evaluate() {
InputStreamResource normalInputStream = new InputStreamResource(InputStream.nullInputStream());
InputStreamResource customInputStream = new MyCustomInputStreamResource();
boolean normalInputStreamWithCurrentCheck = InputStreamResource.class == normalInputStream.getClass();
boolean customInputStreamWithCurrentCheck = InputStreamResource.class == customInputStream.getClass();
boolean normalInputStreamWithProposedCheck = InputStreamResource.class.isAssignableFrom(normalInputStream.getClass());
boolean customInputStreamWithProposedCheck = InputStreamResource.class.isAssignableFrom(customInputStream.getClass());
System.out.println("Normal input stream with current check: " + normalInputStreamWithCurrentCheck); // prints true
System.out.println("Custom input stream with current check: " + customInputStreamWithCurrentCheck); // prints false
System.out.println("Normal input stream with proposed check: " + normalInputStreamWithProposedCheck); // prints true
System.out.println("Custom input stream with proposed check: " + customInputStreamWithProposedCheck); // prints true
}
public static class MyCustomInputStreamResource extends InputStreamResource {
public MyCustomInputStreamResource() {
super(InputStream.nullInputStream());
}
}
}
Comment From: jhoeller
This is by design: We only exclude our well-known InputStreamResource
implementation itself there - since we know it is never working for that purpose due to the inherited contentLength()
method which consumes the stream. For custom subclasses of InputStreamResource
, we effectively expect and highly recommend an overridden contentLength()
implementation. Point taken that this is not as obvious as it should be, so I'll turn this into a documentation issue.
FWIW, InputStreamResource
is generally a last resort and not fully compliant with the Resource
abstraction. If there is any chance to provide a proper Resource
implementation that freshly obtains the stream for every getInputStream()
call, that's very much recommended. In particular if you are providing a custom Resource
class anyway, you should aim for such on-demand stream retrieval there.
Overall, there is only really one reason to build a custom subclass of InputStreamResource
: providing a concrete content length for a given single-use InputStream
in a custom contentLength()
implementation. For every other purpose, you are better off extending from AbstractResource
and encapsulating your resource specifics in an entirely custom resource class with built-in stream retrieval.
On a related recent note: https://github.com/spring-projects/spring-framework/issues/32802#issuecomment-2110414842