Add support for unknown content lengths.
One use case is: Currently there is no way to ignore Range requests for Resources that are not specifically InputStreamResource (not even its subclasses). The crux of the matter is that we are trying to ignore Range requests for streams whose content lengths cannot be determined, instead we do it in a roundabout way by excluding specifically InputStreamResource.
For instance: - I have a custom InputStream that needs to ignore Range requests. It additionally needs to be lazily initialized (i.e. only when the Resource.getInputStream() is called). Its content length is unknown. - Cannot use any other Resource class other than InputStreamResource because they all will try accepting Range requests and when the content length is unknown, will throw 416s. - Cannot put the stream in InputStreamResource because of the lazy instantiation requirement - Cannot subclass InputStreamResource and override the getInputStream() method because the subclass will now accept Range requests and throw a 416
This PR excludes Resources with unknown content lengths from trying to do Partial Range support (which only results in 416s, instead of just a 200 with a full resource)
A more complete redesign would allow the unknown lengths partial support, perhaps using "*" in the Content-Range header
Comment From: rstoyanchev
The idea of excluding resources with unknown content-length from range requests seems fine, but the "Accept-Ranges" that is set for the full resource should also be excluded in that case.
Note that the change on Resource#contentLenth() from long to Long is changing a public API that has existed since 2003. Any code that calls it now has to anticipate the possibility of null whereas previously it could check for -1. This has no chance of being accepted and it is not necessary for this change as far as I can tell.
Comment From: randomnicode
The idea of excluding resources with unknown content-length from range requests seems fine, but the "Accept-Ranges" that is set for the full resource should also be excluded in that case.
Hmm, I think you're right? I assumed it was only setting it for appropriate resource types here since that's gated by the isResourceType(): https://github.com/spring-projects/spring-framework/blob/6c2cb8ecf5d1d755f09aff80489aa8b6e49d70b1/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java#L204-L205
Does that mean that currently InputStreamResources get an "Accept-Ranges" response headers even though we explicitly exclude them from Ranges?
Note that the change on Resource#contentLenth() from long to Long is changing a public API that has existed since 2003. Any code that calls it now has to anticipate the possibility of null whereas previously it could check for -1. This has no chance of being accepted and it is not necessary for this change as far as I can tell.
Yes, it is changing the public API, I thought it made for better semantics. I assumed it was something to consider at major revision change, but okay to not accept it. We can test for negative content length values, that will work just fine too. Some of this stuff might need to change to account for negative values: https://github.com/spring-projects/spring-framework/blob/6c2cb8ecf5d1d755f09aff80489aa8b6e49d70b1/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java#L686-L692
Comment From: randomnicode
I also think we can take out InputStreamResource from the exclusion list here: https://github.com/spring-projects/spring-framework/pull/24314/files#diff-24571dc5c7743b2a69f7a5df4f2109b2R333-R335
That way, only Resources with unknown lengths are excluded from Range requests. If the length of the InputStreamResource is known, then it should be okay to include it in the Range requests?
Comment From: rstoyanchev
Feel free to re-work the pull request along those lines.
Comment From: rstoyanchev
Closing as outdated. Most of the changes are to use Long instead of long in Resource#contentLength and that is not relevant to the actual request. It would be fine to just ignore range requests for resources without a known content length.