This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2011年03月15日 15:34 by michael.henry, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| test_email_attach_to_string.py | michael.henry, 2011年03月15日 15:34 | Demonstrates confusing exception returned from attach() after set_payload("some string") | ||
| string_payload_to_attach.patch | varun, 2014年03月01日 12:18 | review | ||
| test_email_attach_to_string.patch | varun, 2014年03月02日 19:21 | review | ||
| test_email_attach_to_string_11558.patch | varun, 2014年03月02日 22:06 | review | ||
| test_email_attach_to_string_11558.patch | varun, 2014年03月03日 02:42 | review | ||
| Messages (10) | |||
|---|---|---|---|
| msg130984 - (view) | Author: Michael Henry (michael.henry) * | Date: 2011年03月15日 15:34 | |
The attached test program, test_email_attach_to_string.py, demonstrates the desire for email.message.Message.attach to raise a more helpful exception when the user incorrectly invokes attach() after setting the payload to a string. |
|||
| msg212498 - (view) | Author: Varun Sharma (varun) * | Date: 2014年03月01日 12:18 | |
I have made a patch which raises TypeError whenever a string type payload is attached to message using email.Message.attach() method.I have also added a unit test for the same. Need review. |
|||
| msg212501 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年03月01日 14:20 | |
Thanks, Varun. Your patch addresses an issue with the current API, but it doesn't address the problem raised in this issue. The problem in this issue is what happens when the *payload* is a string, and you call attach (to attach a message object) on that message. Your fix addresses what happens if you pass a string to attach itself. This also results in an invalid message (it ends up with a payload that contains a list consisting of a single string). Thinking about changing this raises some interesting questions, though. For one, the problem isn't that the argument to attach was a string, but that it was not an object that generator knows how to handle (that is, it wasn't a Message object). For another, is it possible someone is using the email package in a weird context where attaching non-message objects is actually useful? If so, and we disallow it, then we break someones code. Since no one has reported calling attach with a non-message object as an actual bug, I'm inclined not to make this change. Python is a "consenting adults" language, so unless there is a specific reason to disallow something, we generally allow it. To work on a fix for the reported issue, you should start by turning the code in test_email_attach_to_string into a unit test. |
|||
| msg212581 - (view) | Author: Varun Sharma (varun) * | Date: 2014年03月02日 19:21 | |
Afer getting a lot of help from david on irc, i finally improved the patch. Now it catches AttributeError and raises TypeError for non-object types in attach function. |
|||
| msg212594 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年03月02日 21:30 | |
Yes, this is the right idea. A couple of comments: I'm a bit concerned that the AttributeError might catch something other than the specific error generated by this case, but since that is unlikely and in any case would be revealed by the chained traceback, I think this is OK, and not worth the effort it would take to narrow it. The test, while correct, is IMO overbroad. I prefer to only look for the essential parts of the message, which in this case would be mention of 'attach' and 'non-multipart'. Thus the regex I suggested on irc for use in the test: 'attach.*non-multipart'. Also, you should wrap all lines to less than 79 characters, per PEP8. |
|||
| msg212596 - (view) | Author: Varun Sharma (varun) * | Date: 2014年03月02日 22:06 | |
I have fixed the pep8 voilations and shortened the regex as you suggested in the new patch. |
|||
| msg212600 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年03月02日 23:06 | |
You missed a PEP8 line length fix. |
|||
| msg212610 - (view) | Author: Varun Sharma (varun) * | Date: 2014年03月03日 02:42 | |
Sorry :) |
|||
| msg212814 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2014年03月06日 16:44 | |
New changeset 302c8fdb17e3 by R David Murray in branch 'default': #11558: Better message if attach called on non-multipart. http://hg.python.org/cpython/rev/302c8fdb17e3 |
|||
| msg212815 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2014年03月06日 16:46 | |
Committed, thanks Varun. Note that I made two changes to your patch: added a missing space in the folded message (your second PEP8 change), and I renamed the test method to make it clearer what was being tested. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:14 | admin | set | github: 55767 |
| 2014年03月06日 16:46:52 | r.david.murray | set | status: open -> closed versions: + Python 3.4, Python 3.5 messages: + msg212815 type: enhancement stage: resolved |
| 2014年03月06日 16:44:32 | python-dev | set | nosy:
+ python-dev messages: + msg212814 |
| 2014年03月03日 02:42:11 | varun | set | files:
+ test_email_attach_to_string_11558.patch messages: + msg212610 |
| 2014年03月02日 23:06:10 | r.david.murray | set | messages: + msg212600 |
| 2014年03月02日 22:06:51 | varun | set | files:
+ test_email_attach_to_string_11558.patch messages: + msg212596 |
| 2014年03月02日 21:30:25 | r.david.murray | set | messages: + msg212594 |
| 2014年03月02日 19:21:40 | varun | set | files:
+ test_email_attach_to_string.patch messages: + msg212581 |
| 2014年03月01日 14:20:33 | r.david.murray | set | nosy:
+ barry messages: + msg212501 components: + email |
| 2014年03月01日 12:18:46 | varun | set | files:
+ string_payload_to_attach.patch nosy: + varun messages: + msg212498 keywords: + patch |
| 2011年03月15日 15:34:15 | michael.henry | create | |