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

feat: default to charset=utf-8 for text content type #554

Merged
merged 3 commits into from
Nov 25, 2021

Conversation

PureWhiteWu
Copy link
Sponsor Contributor

Motivation

When using axum, I found that if I returned content in non-English(e.g. Chinese), then all browsers (include Chrome, Safari, Firefox) will display unreadable chars.

After investigating, I found that this is because axum only returns Content-Type: text/plain instead of Content-Type: text/plain; charset=utf-8.

I think adding charset=utf-8 as the default charset for text content type makes sense.

Solution

I added charset=utf-8 as the default charset for text content type and used the mime crate to avoid repeat and hard code of header values.

Copy link
Member

@davidpdrsn davidpdrsn left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me.

@jplatte Do you wanna take a quick look as well?

Copy link
Member

@jplatte jplatte left a comment

Choose a reason for hiding this comment

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

Looks good to me as well. I was a little worried about the extra dependency at first but I've had a quick skim of its docs & source and it really seems minimal (nothing like mime guessing based on magic bytes or file extensions in there) and is maintained by seanmonstar so doesn't even increase the potential for supply-chain attacks either.

axum/src/extract/form.rs Outdated Show resolved Hide resolved
@davidpdrsn
Copy link
Member

davidpdrsn commented Nov 24, 2021

One more thing I thought off: We should probably mention this in the changelog.

@PureWhiteWu
Copy link
Sponsor Contributor Author

I've fixed the comment and added the changelog

@davidpdrsn davidpdrsn enabled auto-merge (squash) November 25, 2021 07:58
@PureWhiteWu
Copy link
Sponsor Contributor Author

@davidpdrsn Hi, thanks for your review!
You may need to approve the workflow for this pr~

auto-merge was automatically disabled November 25, 2021 08:17

Head branch was pushed to by a user without write access

@PureWhiteWu
Copy link
Sponsor Contributor Author

@davidpdrsn
Sorry for the mistake. Should be fine now.
I've tested locally.

@davidpdrsn davidpdrsn enabled auto-merge (squash) November 25, 2021 08:23
@davidpdrsn davidpdrsn merged commit 5a5800c into tokio-rs:main Nov 25, 2021
@PureWhiteWu PureWhiteWu deleted the feat/charset_utf8 branch November 25, 2021 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants