Skip to content

ContentLength for InputStreamResource subclasses #24522

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

Closed
admoca60 opened this issue Feb 14, 2020 · 1 comment
Closed

ContentLength for InputStreamResource subclasses #24522

admoca60 opened this issue Feb 14, 2020 · 1 comment
Assignees
Labels
status: invalid An issue that we don't feel is valid

Comments

@admoca60
Copy link

Affects: spring-web-5.2.3


Dear all

I could see that the treatment of subclasses of InputStreamResource is not really good in some converters or codecs for Resource instances. Exactly, I could see that there are some classes that they don't take into account that InputStreamResource could have subclasses (as my case). These examples classes are org.springframework.http.codec.ResourceHttpMessageWriter (method lengthOf(Resource) line if (InputStreamResource.class != resource.getClass()) {) or org.springframework.http.converter.ResourceHttpMessageConverter (method getContentLength and line if (InputStreamResource.class == resource.getClass()) {

I could see another opened issue about it (#20990) and in it, you talk about the possibility to override the contentLength method for all subclasses, but in my opinion, this is a workaround because the right fix should be changing the above lines for something like this: if (!(resource instanceof InputStreamResource)) {. In that way, all of subclasses of InputStreamResource will have the same treatment, because by definition, all of these subclasses are based on an opened InputStream so the treatment should be the same for all cases.

I'm open to collaborate and send you a PR to fix both cases.

I await your reply!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 14, 2020
@poutsma poutsma self-assigned this Mar 23, 2020
@poutsma
Copy link
Contributor

poutsma commented Mar 23, 2020

The current behavior you are seeing (i.e. not checking against subclasses with instanceof) is by design. As @jhoeller states in said issue, the alternative would be for InputStreamResource and all subclasses to fail, because of the way AbstractResource::contentLength() is implemented and because an input stream can only be read once.

@jhoeller also writes:

Unfortunately there is no sensible way to determine the content length for a plain InputStreamResource. And for custom subclasses thereof, we expect contentLength() to be overridden.

The current behavior enables users to override contentLength in subclasses; it would not work otherwise.

@poutsma poutsma added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 23, 2020
@poutsma poutsma closed this as completed Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants