[issue32657] Mutable Objects in SMTP send_message Signature

[Inada Naoki]

Change by Inada Naoki :


--
versions: +Python 3.8 -Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Inada Naoki]

Change by Inada Naoki :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

See also issue34246.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Chason Chaffin]

Chason Chaffin  added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[R. David Murray]

R. David Murray  added the comment:

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.

--
versions:  -Python 2.7, Python 3.4, Python 3.5, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Chason Chaffin]

Change by Chason Chaffin :


--
keywords: +patch
pull_requests: +5180
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Chason Chaffin]

Chason Chaffin  added the comment:

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 #32663 where I found the tests for the UTF8 code weren't being run.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[R. David Murray]

R. David Murray  added the comment:

On Thu, 25 Jan 2018 15:18:01 +, Steven D'Aprano  
wrote:
> On Thu, Jan 25, 2018 at 02:28:17PM +, R. David Murray wrote:
> The docstring for send_message does say 
> 
> If the sender or any of the recipient addresses contain non-ASCII
> and the server advertises the SMTPUTF8 capability, the policy is
> cloned with utf8 set to True for the serialization, and SMTPUTF8
> and BODY=8BITMIME are asserted on the send.

"Asserted" means sent with the SMTP commands.  It could be reworded to
be clearer.

> which I don't really understand, but I thought that perhaps it was a 
> typo for *inserted* on the send, in the sense of inserted into the mail 
> options:
> 
> mail_options += ['SMTPUTF8', 'BODY=8BITMIME']

Even if that had been true, it would still be a bug to do it to the
mutable argument :)

> So are we agreed this is a bug? What about the default for rcpt_options 
> being a dict?

I didn't look at that, but it probably is.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Steven D'Aprano]

Steven D'Aprano  added the comment:

On Thu, Jan 25, 2018 at 02:28:17PM +, R. David Murray wrote:

> obviously I missed that mutation of the value 
> in the code review of the patch that added those lines :(

The docstring for send_message does say 

If the sender or any of the recipient addresses contain non-ASCII
and the server advertises the SMTPUTF8 capability, the policy is
cloned with utf8 set to True for the serialization, and SMTPUTF8
and BODY=8BITMIME are asserted on the send.

which I don't really understand, but I thought that perhaps it was a 
typo for *inserted* on the send, in the sense of inserted into the mail 
options:

mail_options += ['SMTPUTF8', 'BODY=8BITMIME']

So are we agreed this is a bug? What about the default for rcpt_options 
being a dict?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[R. David Murray]

R. David Murray  added the comment:

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 :(

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Chason Chaffin]

Chason Chaffin  added the comment:

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.

--
nosy: +chason

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Steven D'Aprano]

Steven D'Aprano  added the comment:

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.)

--
nosy: +steven.daprano

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32657] Mutable Objects in SMTP send_message Signature

[Kenny Trytek]

New submission from Kenny Trytek :

Mutable arguments in signature of send_message:
https://github.com/python/cpython/blob/master/Lib/smtplib.py#L892-L893

More Information:
http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments

--
components: Library (Lib)
messages: 310641
nosy: Kenny Trytek
priority: normal
severity: normal
status: open
title: Mutable Objects in SMTP send_message Signature
versions: Python 2.7, Python 3.4, Python 3.5, Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com