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

Use UTF-8 by default for JSON multipart content in ContentRequestMatchers #31924

Conversation

azdanov
Copy link
Contributor

@azdanov azdanov commented Dec 29, 2023

This should resolve #31923

I've added default UTF-8 charset to be used in DiskFileItemFactory.

This should allow ContentRequestMatchers to correctly handle multipartData which contains unicode.

@pivotal-cla
Copy link

@azdanov Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@azdanov Thank you for signing the Contributor License Agreement!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 29, 2023
@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) labels Dec 30, 2023
@sbrannen sbrannen changed the title Add default UTF-8 charset to ContentRequestMatchers for multipartData handling Add default UTF-8 charset to ContentRequestMatchers for multipart data handling Dec 30, 2023
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you agree with the proposed change below and whether you'd like to do it?

fileUpload.setFileItemFactory(new DiskFileItemFactory());
DiskFileItemFactory factory = new DiskFileItemFactory();
factory.setDefaultCharset(DEFAULT_ENCODING);
fileUpload.setFileItemFactory(factory);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the general default could break existing tests, and that should wait for a major or minor release. In the meantime, we could narrow the scope and default to UTF-8 specifically for JSON. That would be safer and addresses the original issue.

@azdanov
Copy link
Contributor Author

azdanov commented Jan 15, 2024

I agree, but I don't have time now. So feel free to add.

@rstoyanchev rstoyanchev self-assigned this Jan 15, 2024
@rstoyanchev rstoyanchev added this to the 6.1.4 milestone Jan 15, 2024
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 15, 2024
@rstoyanchev rstoyanchev changed the title Add default UTF-8 charset to ContentRequestMatchers for multipart data handling Use UTF-8 by default for JSON multipart content in ContentRequestMatchers Jan 15, 2024
@rstoyanchev
Copy link
Contributor

Moving to 6.2 to avoid any regressions in a maintenance release.

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

Successfully merging this pull request may close these issues.

ContentRequestMatchers.MultipartHelper default charset
5 participants