Resource writer doesn't consider subclasses of InputStreamResource for content length bypass

[SecretX33]

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:

spring-framework/spring-web/src/main/java/org/springframework/http/codec/ResourceHttpMessageWriter.java

Line 191 in 9b58e1f

if (InputStreamResource.class != resource.getClass()) {

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());
        }
    }
}

[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: #32802 (comment)