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
Mutable Objects in SMTP send_message Signature #76838
Comments
Mutable arguments in signature of send_message: More Information: |
Hi Kenny, and thanks, but I'm not sure what your point is. Are you claiming this is a bug in the code or the documentation? For what it is worth... mutable defaults *are* a "gotcha", so the documentation isn't wrong. And mutable defaults are *not* illegal nor a bug and are occasionally useful. So I'm not sure why you are reporting this? Have you found a problem? (I see that send_message gives rcpt_options a default of {} (empty dict) and then passes it to sendmail, which expects rcpt_options to be a list. I don't know if that matters.) |
Hi Steven, I have nothing to do with the original submitter but this bug did catch my eye. Does using mutable variables in the SMTP library serve a purpose? Looking through the code, I wasn't able to spot anything obvious, and I did spot a place with a potential bug: https://github.com/python/cpython/blob/master/Lib/smtplib.py#L960 Here mail_options is mutated in the case of a non-ASCII email address. Subsequent calls to send_message, even to a different SMTP server (if I'm not mistaken) would also send these headers along. |
Chason: that does look like a bug. Mutable defaults are best to avoid, but if they are used read-only and not passed down further it isn't a problem. send_message was modeled on sendmail, and so copied it's use of defaults (which date from quite some time back :), but obviously I missed that mutation of the value in the code review of the patch that added those lines :( |
On Thu, Jan 25, 2018 at 02:28:17PM +0000, R. David Murray wrote:
The docstring for send_message does say
which I don't really understand, but I thought that perhaps it was a
So are we agreed this is a bug? What about the default for rcpt_options |
On Thu, 25 Jan 2018 15:18:01 +0000, Steven D'Aprano <report@bugs.python.org> wrote:
"Asserted" means sent with the SMTP commands. It could be reworded to
Even if that had been true, it would still be a bug to do it to the
I didn't look at that, but it probably is. |
I'm writing a PR for this right now. Is it worth it to write a test to make sure that mail_options isn't being mutated or is it enough just to update the code to use non-mutable defaults? Interesting side effect of looking into this is bpo-32663 where I found the tests for the UTF8 code weren't being run. |
It's probably enough to fix it. I fear that if we also change them in sendmail we'll break someone's code, but maybe we should do that anyway, for 3.7 only. |
Should I make a separate issue for that, or attach it to this PR? |
See also bpo-34246. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: