classification
Title: bug in nntplib.body() method with possible fix
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: hynek, mdmullins, pitrou, python-dev, r.david.murray
Priority: normal Keywords: easy, patch

Created on 2010-01-06 10:29 by mdmullins, last changed 2012-02-15 17:57 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
nntp-file-test.diff hynek, 2012-02-15 15:23 Tests for NTTP.head() and NNTP.body() w/ file argument review
Messages (9)
msg97301 - (view) Author: Michael Mullins (mdmullins) Date: 2010-01-06 10:29
When using NNTP.body(id,file) I get the following repeatable error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "nntplib.py", line 436, in body
    return self.artcmd('BODY {0}'.format(id), file)
  File "nntplib.py", line 410, in artcmd
    resp, list = self.longcmd(line, file)
  File "nntplib.py", line 267, in longcmd
    return self.getlongresp(file)
  File "nntplib.py", line 249, in getlongresp
    file.write(line + b'\n')
  File "/usr/lib/python3.0/io.py", line 1478, in write
    s.__class__.__name__)
TypeError: can't write bytes to text stream


But by simply changing the line

    openedFile = file = open(file, "w")

...to...

    openedFile = file = open(file, "wb")

...in the following code:

    def getlongresp(self, file=None):
        """Internal: get a response plus following text from the server.
        Raise various errors if the response indicates an error."""

        openedFile = None
        try:
            # If a string was passed then open a file with that name
            if isinstance(file, str):
                openedFile = file = open(file, "wb")

...it seems to fix the problem. I discovered this in 3.0, but downloading and inspecting the source for 3.1 shows the same problem.

MDMullins
msg97306 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-01-06 13:21
('crash' is for the interpreter crashing)

Unfortunately there currently are no unit tests for nntplib.  Unless someone feels like creating an nntp test framework we may have to commit this fix without a test.
msg97343 - (view) Author: Michael Mullins (mdmullins) Date: 2010-01-07 04:30
"('crash' is for the interpreter crashing)"

Sorry.

     "Unless someone feels like creating an nntp test framework..."

This sounds like it is beyond me. But given the evidence, specifically the previous line:

     "file.write(line + b'\n')"

...the module is obviously writing as binary. I know that opening the files created by this method in 3.0 requires the 'rb' flag so it seems a safe bet. And I am actually using this module (as revised above) to get work done.

...

I apologise if this isn't the place for this (should I open another ticket?) but as a larger issue with 3.x in general, it was very confusing about when to use binary data and when to use strings when using nntplib. It took a lot of trial and error. If there was someway to make this more clear, or perhaps the methods themselves could be made flexible, excepting both types but always outputing in binary. (Liberal with input but conservative in output.) This would allow anyone working with nntplib to interact with the module in a completely binary way, understanding that all output will be explicitly binary, and that if strings are necessary, it's for the user to decode().

Just a thought.

MDMullins
msg97351 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-01-07 12:37
Yes, it should be another issue.  That said, there are similar (worse, actually) problems with the py3 email package that we are moving toward addressing.  There we are planning to have dual APIs.  If you want to work on fixing nntplib similarly, that would be great.  I'm expecting there to be synergy between the two, so you might be interested in joining the effort to update the email module as well.  See http://wiki.python.org/moin/Email%20SIG for more details.  (Trying to use nntplib in py3 is how I myself wound up involved in the email effort).
msg153253 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-02-13 08:15
Looking into the source code of nntplib, I'd claim this bug isn't valid anymore?

At least the file is opened in binary mode now – see Lib/nntplib.py:462

In any case, we have a test suite now.
msg153293 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-13 20:05
test_nntplib doesn't seem to exercise the second argument to body() (the file object). Perhaps you want to add a test for that?
msg153402 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2012-02-15 15:23
I have also added a test for NTTP.head(), enjoy.
msg153423 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-02-15 17:57
New changeset f1401d20bc6d by Antoine Pitrou in branch '3.2':
Issue #7644: Add tests for the file argument of NNTP.head() and NNTP.body().
http://hg.python.org/cpython/rev/f1401d20bc6d

New changeset 096b31e0f8ea by Antoine Pitrou in branch 'default':
Issue #7644: Add tests for the file argument of NNTP.head() and NNTP.body().
http://hg.python.org/cpython/rev/096b31e0f8ea
msg153424 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-02-15 17:57
Thank you! Closing now.
History
Date User Action Args
2012-02-15 17:57:37pitrousetstatus: open -> closed
resolution: fixed
messages: + msg153424

stage: test needed -> resolved
2012-02-15 17:57:03python-devsetnosy: + python-dev
messages: + msg153423
2012-02-15 15:23:58hyneksetfiles: + nntp-file-test.diff
keywords: + patch
messages: + msg153402
2012-02-13 20:05:53pitrousetnosy: + pitrou
messages: + msg153293
2012-02-13 08:15:33hyneksetnosy: + hynek

messages: + msg153253
versions: + Python 3.3, - Python 3.1
2010-01-07 12:37:16r.david.murraysetmessages: + msg97351
2010-01-07 04:30:26mdmullinssetmessages: + msg97343
2010-01-06 13:21:54r.david.murraysetpriority: normal

type: crash -> behavior
versions: + Python 3.2
keywords: + easy
nosy: + r.david.murray

messages: + msg97306
stage: test needed
2010-01-06 10:35:39mdmullinssettype: crash
components: + Library (Lib)
2010-01-06 10:29:13mdmullinscreate