classification
Title: mailbox's _become_message is very fragile
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: dlam, josemaria, kasun, me, python-dev, r.david.murray, thomas.holmes
Priority: normal Keywords: easy, patch

Created on 2011-07-11 18:31 by r.david.murray, last changed 2012-04-12 07:53 by dlam. This issue is now closed.

Files
File name Uploaded Description Edit
12537.patch dlam, 2011-09-19 10:31 review
12537.find.attribute.differences.patch dlam, 2011-09-19 10:35 What I used in test_mailbox.py to find special attribute differences between message formats
Messages (10)
msg140157 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-07-11 18:31
The mailbox module has a method _become_message that copies attributes from an object that is an email.message.Message subclass to the calling object (which is also a subclass of email.message.Message).  This method is very fragile in the face of any changes to the email.message.Message attribute set.

Instead it would be better to decouple the mailbox and email modules by copying *all* __dict__ attributes from the source message to the new object, and then rewrite the _explain_to methods to not only convert the 'special attributes' to the correct format for the new subclass, but also delete any leftover "special" attributes.
msg140163 - (view) Author: José María Ruiz Aguilera (josemaria) Date: 2011-07-11 21:11
Hi, I will be working on this issue. This is the first time I will work on a issue in Python so please patient.
msg144061 - (view) Author: David Lam (dlam) * Date: 2011-09-15 00:29
Hi hi, noob here. I found this today after clicking 'Easy issues' link. 

Would something like this work?  test_mailbox.py seems to pass. However, I'm not too sure what more needs to be done in the _explain_to. It seems like everything to convert headers/flags between message formats are already there ?


diff -r 63bf3bae20ef Lib/mailbox.py
--- a/Lib/mailbox.py    Wed Sep 14 11:46:17 2011 -0400
+++ b/Lib/mailbox.py    Wed Sep 14 17:12:51 2011 -0700
@@ -1467,8 +1467,7 @@
 
     def _become_message(self, message):
         """Assume the non-format-specific state of message."""
-        for name in ('_headers', '_unixfrom', '_payload', '_charset',
-                     'preamble', 'epilogue', 'defects', '_default_type'):
+        for name in message.__dict__:
             self.__dict__[name] = message.__dict__[name]
msg144063 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-09-15 01:27
That is the first step, yes.  In addition to that we need to have the various explain_to methods delete the special attributes that aren't valid for the new Message subtype.
msg144112 - (view) Author: David Lam (dlam) * Date: 2011-09-16 01:42
Hm, it seems right now that the only time that happens is when creating a message based on an mbox message.  The 'status' and 'x-status' attributes are deleted.

Any hints on what I could to do figure out what special attributes should be deleted?  I was thinking I might try to find and go through the mailbox specs, and find out what headers are mandatory for each of them.

I think I see some hints on... http://docs.python.org/library/mailbox.html#mailbox.mboxMessage.
msg144169 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-09-17 00:59
Unfortunately I don't think there is any way except going through each subclass to see what special attributes it creates.
msg144265 - (view) Author: David Lam (dlam) * Date: 2011-09-19 10:31
This patch deletes the leftover special attributes. 

Thanks for guiding me through my confusion and/or cluelessness!
msg157831 - (view) Author: Roundup Robot (python-dev) Date: 2012-04-09 02:36
New changeset 5c1c402a63e5 by R David Murray in branch 'default':
#12537: in mailbox avoid depending on knowledge of email package internals
http://hg.python.org/cpython/rev/5c1c402a63e5
msg157832 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-04-09 02:43
David, thanks for your assistance.  I didn't wind up using your patch, but the work you did was valuable in preparing the patch I committed.

What I did was turn your 'detect the attributes' recipe into a unit test.  I then applied your patch, but it didn't quite work.  I eventually figured out that the fix I suggested wasn't quite right, that in fact the right place to delete the attributes is in _become_message.  So I moved the list of attributes you developed in your patch into class attributes, so that what the final patch does is to copy everything *except* the attributes you found.

So, a successful resolution, thank you.
msg158112 - (view) Author: David Lam (dlam) * Date: 2012-04-12 07:53
Wow, cool!  Thanks for the update.
History
Date User Action Args
2012-04-12 07:53:11dlamsetmessages: + msg158112
2012-04-09 02:43:23r.david.murraysetstatus: open -> closed
type: behavior
messages: + msg157832

resolution: fixed
stage: resolved
2012-04-09 02:36:39python-devsetnosy: + python-dev
messages: + msg157831
2011-09-19 10:35:02dlamsetfiles: + 12537.find.attribute.differences.patch
2011-09-19 10:31:27dlamsetfiles: + 12537.patch
keywords: + patch
messages: + msg144265
2011-09-17 00:59:52r.david.murraysetmessages: + msg144169
2011-09-16 01:42:32dlamsetmessages: + msg144112
2011-09-15 01:27:09r.david.murraysetmessages: + msg144063
2011-09-15 00:29:48dlamsetnosy: + dlam
messages: + msg144061
2011-09-01 20:13:26mesetnosy: + me
2011-08-28 16:39:46kasunsetnosy: + kasun
2011-07-11 21:11:23josemariasetnosy: + josemaria
messages: + msg140163
2011-07-11 18:50:18thomas.holmessetnosy: + thomas.holmes
2011-07-11 18:31:14r.david.murraycreate