classification
Title: email.parser.BytesParser().parse() closes file argument
Type: Stage:
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: r.david.murray, sdaoden
Priority: normal Keywords: patch

Created on 2011-03-28 12:30 by sdaoden, last changed 2011-09-17 16:26 by sdaoden. This issue is now closed.

Files
File name Uploaded Description Edit
11701.1.diff sdaoden, 2011-04-13 11:27 review
11701.2.diff sdaoden, 2011-04-13 13:52 review
Messages (6)
msg132397 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-03-28 12:30
... and that closes the original file, too!
(I've also opened #11700 due to the mailbox.py
side of this.)


static PyObject *
textiowrapper_close(textio *self, PyObject *args)
        return PyObject_CallMethod(self->buffer, "close", NULL);


>>> mb = mailbox.Maildir('sdaoden', create=False)
>>> mbf = mb.get_file(mb.keys()[0])
>>> msg = email.parser.BytesParser().parse(mbf, headersonly=True)
>>> mbf.close()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/steffen/usr/opt/py3k/lib/python3.3/mailbox.py", line 1922, in close
    if hasattr(self._file, 'close'):
AttributeError: '_ProxyFile' object has no attribute '_file'
msg133659 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-04-13 11:27
This patch fixes the problem.
It was introduced in e727cf35.

David Murray, i've looked into test_email.py to add tests
for this, and i found TestIdempotent().
What i would do is try to split that thing up so that it covers
str() as well as bytes(), rewriting ._msgobj() to use a xy
parser instance so that the file-argument-is-not-closed test
could be embedded in there as a side-effect.

This is however your area, so please let me know if i may
touch this or if you have local modifications to this one
which will make my snail-slow work on it pointless.
Thanks.
msg133664 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-04-13 13:07
For easy reference, here's a hash for that changeset that roundup will turn into a link: e727cf354720.

TestIdempotent is already run for both the bytes and str cases (they are widely separated in the test file...I'll fix that at some point).
msg133665 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-04-13 13:34
On Wed, Apr 13, 2011 at 01:07:28PM +0000, R. David Murray wrote:
> TestIdempotent is already run for both the bytes and str cases

(*^.^*)
Ouch.  Searching is only for the experienced.
msg133666 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-04-13 13:52
So i'll changed the existing message_from_(binary_)?file() tests
to also test the file closing.
msg144206 - (view) Author: Steffen Daode Nurpmeso (sdaoden) Date: 2011-09-17 16:26
Right, and that is considered to be a non-issue due to
that close() is allowed multiple times without causing harm.
History
Date User Action Args
2011-09-17 16:26:55sdaodensetstatus: open -> closed

messages: + msg144206
2011-04-16 19:59:28r.david.murraysetassignee: r.david.murray
2011-04-14 00:08:01brett.cannonsetnosy: - brett.cannon
2011-04-13 13:52:35sdaodensetfiles: + 11701.2.diff

messages: + msg133666
2011-04-13 13:34:53sdaodensetmessages: + msg133665
2011-04-13 13:07:26r.david.murraysetmessages: + msg133664
2011-04-13 11:27:30sdaodensetfiles: + 11701.1.diff
title: email.parser.BytesParser() uses TextIOWrapper -> email.parser.BytesParser().parse() closes file argument
nosy: + brett.cannon

messages: + msg133659

keywords: + patch
2011-03-28 12:30:19sdaodencreate