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

Pass filename hint to the ResourceDecoder #22267

Closed
filiphr opened this issue Jan 16, 2019 · 4 comments
Closed

Pass filename hint to the ResourceDecoder #22267

filiphr opened this issue Jan 16, 2019 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@filiphr
Copy link
Contributor

filiphr commented Jan 16, 2019

Currently the ResourceDecoder uses the ByteArrayResource(byte[]) constructor. However, usually when working with Resource the Content-Disposition header can be used to send the filename of the resource. I think that in this case it would be helpful if that can be passed as the description.

@bclozel bclozel added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 17, 2019
@sdeleuze sdeleuze self-assigned this Jan 18, 2019
@sdeleuze
Copy link
Contributor

Thanks for your proposal.

ResourceDecoder lives in spring-core and has no knowledge of HTTP headers. Leveraging Content-Disposition header would require implementing and using a dedicated ResourceHttpMessageReader instead of the generic DecoderHttpMessageReader. Implementing your proposal would require additional infrastructure, so I would like to have more detail about your use case to evaluate if your need is common enough.

Also, if we decide to do something here, are we talking only about description or also about filename Resource property (currently not accessible from ByteArrayResource)?

@rstoyanchev
Copy link
Contributor

FWIW there is similar logic in the converter equivalent:

else if (Resource.class == clazz || ByteArrayResource.class.isAssignableFrom(clazz)) {
byte[] body = StreamUtils.copyToByteArray(inputMessage.getBody());
return new ByteArrayResource(body) {
@Override
@Nullable
public String getFilename() {
return inputMessage.getHeaders().getContentDisposition().getFilename();
}
};
}

I suppose we could pass a filename hint without having the ResourceDecoder deal with an HTTP header.

@sdeleuze sdeleuze removed their assignment Jan 18, 2019
@sdeleuze
Copy link
Contributor

Feature parity between converters and codecs would be nice, so I add this improvement request in our 5.2 bucket.

@filiphr If you need that with 5.1, you can probably wrap the decoder and decorate the resource as a temporary workaround.

@sdeleuze sdeleuze added this to the 5.2 RC1 milestone Jan 18, 2019
@sdeleuze sdeleuze added in: web Issues in web modules (web, webmvc, webflux, websocket) in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 18, 2019
@filiphr
Copy link
Contributor Author

filiphr commented Jan 18, 2019

You were faster than me to resolve everything 😄.

I suppose we could pass a filename hint without having the ResourceDecoder deal with an HTTP header.

I was indeed thinking about passing the filename in the hints, not making the ResourceDecoder aware about the HTTP headers, sorry about not being more specific, I thought the name of the issue would imply that.

Also, if we decide to do something here, are we talking only about description or also about filename Resource property (currently not accessible from ByteArrayResource)?

I was thinking about description as I saw that the filename is not exposed in the ByteArrayResource. However, pointing into the ResourceHttpMessageConverter indeed shows that one can set the filename properly.

I actually need the filename as that is more descriptive and something that I am looking for in the Resource.

Feature parity between converters and codecs would be nice, so I add this improvement request in our 5.2 bucket.

Thanks for adding it to the 5.2 bucket.

Just my 2 cents, you might want to support passing the filename to a ByteArrayResource in order not to need to provide something like in the ResourceHttpMessageConverter.

@filiphr If you need that with 5.1, you can probably wrap the decoder and decorate the resource as a temporary workaround.

I'll look into on our side to see what would be the best approach. Thanks for the hint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants