Gig-o-Matic / GO3

GNU General Public License v3.0
10 stars 8 forks source link

Unusual characters in username breaks email sending #549

Closed bklang closed 2 months ago

bklang commented 2 months ago

Error from exception:

[worker] [2024-09-25 03:34:31] POST Response: 400 b'{"errors":[{"message":"Does not contain a valid address.","field":"personalizations.0.to.0.email","help":"http://sendgrid.com/docs/API_Reference/Web_API_v3/Mail/errors.html#message.personalizations.to"}]}'

Relevant piece of JSON from the SendGrid API call:

"personalizations": [{"to": [{"email": ""}],

Issue was traced to a user who had their name set as Example Name :) (real name redacted). Removing the smiley made emails work.

aaronoppenheimer commented 2 months ago

Hm. It may be that we should just enclose the "descriptive name" part of the address in quotes - it looks to me like fastmail always does that. I don't have the sendgrid keys to test it, but the place to add the quotes is in lib/email.py line 30:

return f'{self.name} <{self.email}>' if self.name else self.email

should be

return f'"{self.name}" <{self.email}>' if self.name else self.email

and I think that'll work.

bklang commented 2 months ago

Thanks Aaron. Your suggestion is a good one, but I'm having trouble even reproducing the issue locally.

In production, if I add :) to my name, emails stop working and I see an empty to attribute in the API request body. In my local environment, everything works normally when I add :) to my name and I see an email address in the API request body.

I tried to find some reference to whether the name portion should be quoted.

We are using EmailMultiAlternatives which subclasses EmailMessage which is a wrapper for smtplib.send_message which is a convenience method for smtplib.sendmail. The sendmail method is the first place where I found any reference to the formatting of the to attribute, and it passes the buck to the SMTP RFC. RFC 822 section 6 specifies the format of the To address but gives no specification I can see on how to encode names.

I guess I can try quoting the string. Unless I can figure out a local reproduction, it'll mean testing in production, which I don't love.

bklang commented 2 months ago

Right after I sent that I remembered that RFC 822 was superseded by RFC 2822. And that one defines how to represent display names, which allows for double-quotes as you suggested. https://datatracker.ietf.org/doc/html/rfc2822#section-3.4

Still going to try a reproduction, but I have more confidence that double quotes won't hurt anything.

bklang commented 2 months ago

More evidence that this fix is correct: on the PR, with the quoted string, my name started to appear in the API request in addition to my email address, which was not the case before. I'm going to call that good and ship it, and watch the logs.

bklang commented 2 months ago

Issue is fixed and verified in production