mail
mail copied to clipboard
Add configurable mime boundary generator
Hello!
This PR aims to add the ability to customize the generator used for mime-boundaries when creating mails.
Recently, after converting our application to use ActionMailer (and thus, Mail),
my team found that several of our customers stopped receiving our emails due to
"malformed structures". Upon digging in, it appears their systems may be
flagging our emails because of the new email's mime-boundary containing an equal
sign (=). While the RFC states that this character is valid in the boundary
(provided it's quoted), we have found a couples instances noting that it's
merely safer to not include them at all.
Rather than request a change to the boundary generation algorithm here, we instead make the algorithm configurable to hopefully prevent any impact to current consumers.
Feedback is, of course, welcome on the code structures presented here and/or any other alternative options that may help us diagnose the issue.
Thank you!
Tested on Ruby 3.0.2, 2.7.3, and 2.6.5
Hi there @dugancathal this is excellent, though I think we can make this change to the random tag generator to remove the = sign from it as well. Could you please modify the random_tag method to exclude = from it? Also, please add a changelog entry and I'll get this deployed in the next 2.9 release.
Hey @mikel! Thanks for the review.
I'm not 100% sure I follow, so I'd like to clarify. Are you looking for me to:
- Undo the change here and instead completely remove the
=from thegenerate_boundarymethod? - Update the
Mail.random_tagmethod to, for instance,gsub('=', '')(or similar)? - Something else?
Looking at the code, I'm not sure that [2] would do anything as all the components of that string are numeric and will not include equal signs (=).
Just lemme know which you're lookin' for and I'll get this updated for release.
Cheers!
Hey there @dugancathal - you mention the reason you created the custom mime boundry generator capability is because Mail is including a = in the mime boundry generated.
So what I'm proposing is:
- Keep these changes (allowing people to override the mime boundry generator is a good thing, maybe they want a custom boundry for some reason)
- ALSO modify the default mail mime boundry generator itself so that it does not include an
=in it to remove the need for 99% of people to ever need to implement their own custom one (including you) :)
Make sense?
Makes sense, @mikel. Made those changes. Lemme know if this needs any other work. :)
OK thanks @dugancathal - any chance you could look into the spec failures?
AFAICT the failures are all in the Ruby setup step. This should have been solved by the fix to install libyaml-dev (not needed for JRuby, hence they succeeded). Just trigger a re-run and tests should all succeed.
Hey folks, sorry for the radio silence on this. Is there anything y'all need from me? Or is it possible for me to rebase and re-run CI somehow to see if this is still an issue?
It would be helpful if you could trigger a CI rebuild by making a trivial change. Rebasing would probably also work, though I'm not sure it's necessary here.
I think there needs to be a test to show that the original boundary can still be generated and used.
Also: if certain characters need to be quoted, who is responsible for doing this? Seems to me that this is the responsibility of the mail Gem, so needs to be handled in the code that wraps the user-provided boundary.