The following commands fail badly:

>>> from nntplib import NNTP
>>> s = NNTP("free-text.usenetserver.com")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/py3k/Lib/nntplib.py", line 116, in __init__
    self.welcome = self.getresp()
  File "/py3k/Lib/nntplib.py", line 215, in getresp
    resp = self.getline()
  File "/py3k/Lib/nntplib.py", line 209, in getline
    elif line[-1:] in CRLF: line = line[:-1]
TypeError: 'in <string>' requires string as left operand, not bytes

Actually there are many places in nntplib module which need to be
converted to bytes, or socket input/output values need to be converted
from/to str. I think API need to be updated to pass user defined
encoding at some stages. I can make a patch later if needed.

Yes, the module is unusable in the current state.

I've attached the patch which adds encoding parameter to the NNTP class.

I verified the patch against the trunk. It works fine and solves the issue.
But, I have a minor concern over 'ascii' as the default encoding, which you
have chosen.

For e.g, when I ran python3.0 nntplib.py (It would run tests, as the module
does not have an explicit test), I got the following error due to encoding.
(tests read comp.lang.python)

UnicodeDecodeError: 'ascii' codec can't decode byte 0x93 in position 29:
ordinal not in range(128)

Setting the encoding to 'latin1', it passed.

Would 'latin1' be a better default encoding? Or should we leave it as 'ascii'.

Actually RFC-977 said all characters must be in ASCII, but RFC-3977
changed default character set to UTF-8. So I think UTF-8 must be default
encoding, not Latin-1. Moreover Latin-1 can silently hide a real
encoding, for example:

>>> u'\u0422\u0435\u0441\u0442'.encode("koi8-r").decode("latin1")
u'\xf4\xc5\xd3\xd4'

Additionally in the future it would be a good idea to look in the
article headers for article body encoding.

Setting the default encoding as "ascii" is very conservative until we
know the encoding actually used by the server.

Are you sure that comp.lang.python uses latin-1? RFC3977, which
re-defines the NNTP protocol, prefers utf-8 for the character set.

Is there a way to know the character set used by a server?

When the default encoding 'ascii' failed, I tried, 'utf-8', even that failed to
retrieve the message headers from comp.lang.python. 
'latin1' succeeded. That was reason for my suggestion, considering 'latin1' to
be superset of 'ascii'. 

But, yes checking the encoding at the server and using that would be a good
idea. The for the default, we could follow whatever RFC3977 recommends.

If I understand it correctly there is no "character set used by server"
because every article can be in different encoding. RFC-3977 say:

"""
The character set of article bodies SHOULD be indicated in the
article headers, and this SHOULD be done in accordance with MIME.
"""

But it's not always true, for example fido7.* groups known to use
"KOI-8R" encoding but I didn't find any relevant headers.

RFC-3977 say the following about headers:

- The names of headers (e.g., "From" or "Subject") MUST be in
  US-ASCII.

- Header values SHOULD use US-ASCII or an encoding based on it, such
  as RFC 2047 [RFC2047], until such time as another approach has
  been standardised.  At present, 8-bit encodings (including UTF-8)
  SHOULD NOT be used because they are likely to cause
  interoperability problems.

But in practice for now there is no way to reliable find a header's
encoding.

So, in effect, if we settle for ASCII as the default encoding, then the
attached patch is good enough. Someone should check this in. 
Should it go in py3k, release? Because, without this patch(which is again
simple), nntplib is almost unusable.

Instead of ASCII, I think that it would be better to use ISO-8859-1 
since it's the most common charset.

New patch:
 - use ISO-8859-1 as the default charset
 - remove set_encoding() method: was it really needed
 - use makefile('r', encoding=self.encoding') to get a new 
TextIOWrapper with universal newline and automatic unicode decode => 
getline() is simplified and I removed CRLF

Oh, you need to read the comments first:

- Use of ISO-8859-1 it's a bad idea here. See msg72776 for details.
Moreover RFC-3977 explicitly say about UTF-8, so I think we need to use
UTF-8.
- Maybe set_encoding() isn't needed but you need to have a possibility
to change encoding after object creation. Because different groups can
use different encodings. But with makefile() addition you just remove
this possibility.

Ok for UTF-8 which is a superset of ASCII and raise an error when 
trying to decode Latin1 or KOI-8.

> You need to have a possibility to change encoding after object 
creation

If you share a connection for the different groups, you will have to 
take care of the side effets of set_encoding(). But if you consider 
that set_encoding() is a must-have, ok, forget my patch (because using 
makefile(), it's not possible to change the charset) ;-)

This issue does not seem close to resolution and the nntplib is not
critical enough to block the release, so I'm deferring this to rc3.

As I did for POP3 (poplib) and IMAP4 (imaplib), nntplib should use 
bytes instead of characters because the charset may be different for 
each message and nntplib is unable to retreive the charset from the 
email header.

After testing many different emails servers (POP3, IMAP4 and NNTP), I 
think that the best type is bytes and not str (unicode). Attached 
patch changes nntplib to use bytes:
 - write tests using Gmane NNTP public server
 - use bytes everywhere except for the commands (str), converted to 
bytes using ASCII charset
 - factorizy post() + ihave(): create _post()
 - replace line[-1] == '\n' by line.endswith(b'\n')
 - replace e.response[:3] == '480' by e.response.startswith(b'480')
 - use format() method because str+int is forbidden. Eg. see body() 
method: id can be an integer (or an unicode string).

I like startswith() / endswith() because it's more readable and it 
works with string of 1 byte: line[-1:] == b'\n' vs 
line.endswith(b'\n').

TODO:
 - test authentication
 - test methods marked as "TODO" in the testcase: 
    * Check newgroups(), newnews(), list()
    * Check xhdr(), xover(), xgtitle(), xpath()
    * Check post(), ihave()
 - does .format() work with bytes? body(b'1') may creates the 
command "BODY b'1'" which is wrong <= should I also use bytes for the 
commands?

I don't know NNTP enough to test all methods, and I don't have any 
account to test the authentication :-/ But most common commands should 
work (see the testcase).

I haven't been able to examine the patch in depth, but one thing I see
is that the test case needs to call test.support.requires("network").

Please fix the test as Benjamin suggests ("network").  While I'd prefer
a more fleshed out test suite, I think in time you can add that.  For
now, this is still an improvement and likely the best we're going to get
before 3.0.  Thanks for working on this.

Go ahead and make the suggested change and land the code.

Le Tuesday 04 November 2008 01:08:25 Barry A. Warsaw, vous avez écrit :
> Please fix the test as Benjamin suggests ("network")

Done: see new attached patch.

Applied in revision r67108