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

Fix that set not-accurate multipart header #1145

Conversation

kirikak2
Copy link
Contributor

@kirikak2 kirikak2 commented Sep 7, 2017

According to RFC2046, multipart/* content-type do not need to have charset parameter.
Currentry, When I create multipart mail, mail library append charset="UTF-8" in content-type header automatically.

This patch fixes 2 issues.

  1. When create multipart content-type header, mail library appends charset parameter.
  2. When call Message#convert_to_multipart, mail library ignores original header. I think Content-Type, Content-Disposition, Content-Transfer-Encoding andContent-Description should move to part header.

@kirikak2 kirikak2 force-pushed the should_not_have_charset_params_in_multipart_content_type branch from 7d8d1e2 to e10f4ac Compare September 7, 2017 05:16
@kirikak2 kirikak2 changed the title WIP: Fix that do not append charset parameter with content-type multipart/* WIP: Fix that set not-accurate multipart header Sep 7, 2017
@kirikak2 kirikak2 changed the title WIP: Fix that set not-accurate multipart header Fix that set not-accurate multipart header Sep 7, 2017
@jeremy
Copy link
Collaborator

jeremy commented Sep 7, 2017

Agreed on both points.

@jeremy jeremy self-assigned this Sep 7, 2017
@jeremy jeremy added this to the 2.8.0 milestone Nov 4, 2017
@jeremy jeremy closed this in 404ecdd Nov 4, 2017
@ahorek ahorek mentioned this pull request Dec 19, 2018
mikel pushed a commit that referenced this pull request Nov 30, 2022
…ts to a Mail resets the default charset to nil.

This PR restores the behaviour from 2-7-stable and allows us to use the edge version of mail with rails 7 without any patch.

Originally from @railsbob on #1470
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

2 participants