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

Possibility to configure encoding other than ascii in FormHttpMessageConverter.MultipartHttpOutputMessage [SPR-15396] #19959

Closed
spring-projects-issues opened this issue Mar 29, 2017 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 29, 2017

Anders Båtstrand opened SPR-15396 and commented

For some cases, it is useful to allow non-ascii characters in the mime headers in multipart requests. For example when using Zuul to proxy file uploads with non-ascii filenames.

The encoding used is hard-coded in FormHttpMessageConverter.MultipartHttpOutputMessage#getAsciiBytes(String).

Although that seems correct, I guess most browsers does not respect this. It would be really useful if that encoding was possible to disable/configure in some way.

I would be happy to provide a pull request if this is something you would consider.

The only work-around for me now, is to duplicate FormHttpMessageConverter in my code, with a slightly modified inner class MultipartHttpOutputMessage.


Issue Links:

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I'm currently revising a few spots in terms of configurable character encodings already, so I'll have a look at this one for 4.3.8 still. At a minimum, we should be able to provide an overridable protected template method or a configurable property for the multipart encoding here.

@spring-projects-issues
Copy link
Collaborator Author

Anders Båtstrand commented

@juergen.hoeller: That is excellent news! Please let me know if I can help, be it a pull request, writing some tests or testing the solution.

For my team, replacing FormHttpMessageConverter.MultipartHttpOutputMessage#getAsciiBytes(String) with a protected method in FormHttpMessageConverter would be nice. Recent releases of Spring Cloud Netflix exposes a constructor in org.springframework.cloud.netflix.zuul.filters.pre.FormBodyWrapperFilter that allows setting the FormHttpMessageConverter.

On the other hand, having it possible to configure using a property would make the fix usable for users of older versions of Spring Cloud Netflix.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Apr 3, 2017

Juergen Hoeller commented

On review, it looks like we addressed a similar requirement through #16724 back in 4.1.1 already.

Rossen Stoyanchev, do you see a need for yet another configurable charset variant here, or is the existing multipartCharset for filenames good enough?

@spring-projects-issues
Copy link
Collaborator Author

Anders Båtstrand commented

Just some input:

Using multipartCharset is not an option for me, as it gives NPE. It is used in org.springframework.http.converter.FormHttpMessageConverter#getFilename, and that method returns null for me (the resource stream does not have a filename). Note that org.springframework.core.io.Resource#getFilename allows for returning null.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Indeed, there's a filename != null check missing there before we try the encoding! I've added that right away... However, I suppose that's still not going to help you since you don't expose a filename from the resource to begin with? Is your filename header manually set?

I'm wondering whether the multipartCharset semantics should be extended to custom filename headers or even all part headers. I'd primarily like to avoid yet another configurable charset when we've got two of them on FormHttpMessageConverter already.

@spring-projects-issues
Copy link
Collaborator Author

Anders Båtstrand commented

I use Zuul, but I have not looked into exactly how the header is set.

A solution for my team would be to use multipartCharset, if set, insetad of ASCII in FormHttpMessageConverter.MultipartHttpOutputMessage#getAsciiBytes(String) (after a rename of the method, obviously).

@spring-projects-issues
Copy link
Collaborator Author

Anders Båtstrand commented

Any news on this? Should I attempt a pull request?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 1, 2017

Rossen Stoyanchev commented

This should be addressed in 5.0 with #19769. The ticket is quite long so the resulting commit might be more helpful to see. Effectively we now use the charset property (UTF-8 by default) to encode the filename unless multpartCharset is set (null by default) in which case we use RFC 2047 encoding.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 1, 2017

Rossen Stoyanchev commented

Marking as duplicate of #19769 and resolving but feel free to comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants