Issue #3714: nntplib module broken by str to unicode conversion

Patch by Victor, Reviewed by Barry

Lib/nntplib.py

@@ -7,7 +7,7 @@ Example:
 >>> resp, count, first, last, name = s.group('comp.lang.python')
 >>> print('Group', name, 'has', count, 'articles, range', first, 'to', last)
 Group comp.lang.python has 51 articles, range 5770 to 5821
->>> resp, subs = s.xhdr('subject', first + '-' + last)
+>>> resp, subs = s.xhdr('subject', '{0}-{1}'.format(first, last))
 >>> resp = s.quit()
 >>>
 
@@ -15,7 +15,7 @@ Here 'resp' is the server response line.
 Error responses are turned into exceptions.
 
 To post an article from a file:
->>> f = open(filename, 'r') # file containing article, including header
+>>> f = open(filename, 'rb') # file containing article, including header
 >>> resp = s.post(f)
 >>>
 
@@ -81,11 +81,11 @@ NNTP_PORT = 119
 
 
 # Response numbers that are followed by additional text (e.g. article)
-LONGRESP = ['100', '215', '220', '221', '222', '224', '230', '231', '282']
+LONGRESP = [b'100', b'215', b'220', b'221', b'222', b'224', b'230', b'231', b'282']
 
 
 # Line terminators (we always output CRLF, but accept any of CRLF, CR, LF)
-CRLF = '\r\n'
+CRLF = b'\r\n'
 
 
 
@@ -128,7 +128,7 @@ class NNTP:
                 # error 500, probably 'not implemented'
                 pass
             except NNTPTemporaryError as e:
-                if user and e.response[:3] == '480':
+                if user and e.response.startswith(b'480'):
                     # Need authorization before 'mode reader'
                     readermode_afterauth = 1
                 else:
@@ -148,13 +148,13 @@ class NNTP:
         # Perform NNRP authentication if needed.
         if user:
             resp = self.shortcmd('authinfo user '+user)
-            if resp[:3] == '381':
+            if resp.startswith(b'381'):
                 if not password:
                     raise NNTPReplyError(resp)
                 else:
                     resp = self.shortcmd(
                             'authinfo pass '+password)
-                    if resp[:3] != '281':
+                    if not resp.startswith(b'281'):
                         raise NNTPPermanentError(resp)
             if readermode_afterauth:
                 try:
@@ -196,6 +196,7 @@ class NNTP:
     def putcmd(self, line):
         """Internal: send one command to the server (through putline())."""
         if self.debugging: print('*cmd*', repr(line))
+        line = bytes(line, "ASCII")
         self.putline(line)
 
     def getline(self):
@@ -205,8 +206,10 @@ class NNTP:
         if self.debugging > 1:
             print('*get*', repr(line))
         if not line: raise EOFError
-        if line[-2:] == CRLF: line = line[:-2]
+        if line[-2:] == CRLF:
-        elif line[-1:] in CRLF: line = line[:-1]
+            line = line[:-2]
+        elif line[-1:] in CRLF:
+            line = line[:-1]
         return line
 
     def getresp(self):
@@ -215,11 +218,11 @@ class NNTP:
         resp = self.getline()
         if self.debugging: print('*resp*', repr(resp))
         c = resp[:1]
-        if c == '4':
+        if c == b'4':
             raise NNTPTemporaryError(resp)
-        if c == '5':
+        if c == b'5':
             raise NNTPPermanentError(resp)
-        if c not in '123':
+        if c not in b'123':
             raise NNTPProtocolError(resp)
         return resp
 
@@ -239,12 +242,12 @@ class NNTP:
             list = []
             while 1:
                 line = self.getline()
-                if line == '.':
+                if line == b'.':
                     break
-                if line[:2] == '..':
+                if line.startswith(b'..'):
                     line = line[1:]
                 if file:
-                    file.write(line + "\n")
+                    file.write(line + b'\n')
                 else:
                     list.append(line)
         finally:
@@ -312,16 +315,16 @@ class NNTP:
 
         resp, lines = self.descriptions(group)
         if len(lines) == 0:
-            return ""
+            return b''
         else:
             return lines[0][1]
 
     def descriptions(self, group_pattern):
         """Get descriptions for a range of groups."""
-        line_pat = re.compile("^(?P<group>[^ \t]+)[ \t]+(.*)$")
+        line_pat = re.compile(b'^(?P<group>[^ \t]+)[ \t]+(.*)$')
         # Try the more std (acc. to RFC2980) LIST NEWSGROUPS first
         resp, raw_lines = self.longcmd('LIST NEWSGROUPS ' + group_pattern)
-        if resp[:3] != "215":
+        if not resp.startswith(b'215'):
             # Now the deprecated XGTITLE.  This either raises an error
             # or succeeds with the same output structure as LIST
             # NEWSGROUPS.
@@ -344,7 +347,7 @@ class NNTP:
         - name: the group name"""
 
         resp = self.shortcmd('GROUP ' + name)
-        if resp[:3] != '211':
+        if not resp.startswith(b'211'):
             raise NNTPReplyError(resp)
         words = resp.split()
         count = first = last = 0
@@ -368,11 +371,11 @@ class NNTP:
 
     def statparse(self, resp):
         """Internal: parse the response of a STAT, NEXT or LAST command."""
-        if resp[:2] != '22':
+        if not resp.startswith(b'22'):
             raise NNTPReplyError(resp)
         words = resp.split()
         nr = 0
-        id = ''
+        id = b''
         n = len(words)
         if n > 1:
             nr = words[1]
@@ -393,7 +396,7 @@ class NNTP:
         - nr:   the article number
         - id:   the message id"""
 
-        return self.statcmd('STAT ' + id)
+        return self.statcmd('STAT {0}'.format(id))
 
     def next(self):
         """Process a NEXT command.  No arguments.  Return as for STAT."""
@@ -418,7 +421,7 @@ class NNTP:
         - id: message id
         - list: the lines of the article's header"""
 
-        return self.artcmd('HEAD ' + id)
+        return self.artcmd('HEAD {0}'.format(id))
 
     def body(self, id, file=None):
         """Process a BODY command.  Argument:
@@ -431,7 +434,7 @@ class NNTP:
         - list: the lines of the article's body or an empty list
                 if file was used"""
 
-        return self.artcmd('BODY ' + id, file)
+        return self.artcmd('BODY {0}'.format(id), file)
 
     def article(self, id):
         """Process an ARTICLE command.  Argument:
@@ -442,7 +445,7 @@ class NNTP:
         - id: message id
         - list: the lines of the article"""
 
-        return self.artcmd('ARTICLE ' + id)
+        return self.artcmd('ARTICLE {0}'.format(id))
 
     def slave(self):
         """Process a SLAVE command.  Returns:
@@ -458,8 +461,8 @@ class NNTP:
         - resp: server response if successful
         - list: list of (nr, value) strings"""
 
-        pat = re.compile('^([0-9]+) ?(.*)\n?')
+        pat = re.compile(b'^([0-9]+) ?(.*)\n?')
-        resp, lines = self.longcmd('XHDR ' + hdr + ' ' + str, file)
+        resp, lines = self.longcmd('XHDR {0} {1}'.format(hdr, str), file)
         for i in range(len(lines)):
             line = lines[i]
             m = pat.match(line)
@@ -476,10 +479,10 @@ class NNTP:
         - list: list of (art-nr, subject, poster, date,
                          id, references, size, lines)"""
 
-        resp, lines = self.longcmd('XOVER ' + start + '-' + end, file)
+        resp, lines = self.longcmd('XOVER {0}-{1}'.format(start, end), file)
         xover_lines = []
         for line in lines:
-            elem = line.split("\t")
+            elem = line.split(b'\t')
             try:
                 xover_lines.append((elem[0],
                                     elem[1],
@@ -500,7 +503,7 @@ class NNTP:
         - resp: server response if successful
         - list: list of (name,title) strings"""
 
-        line_pat = re.compile("^([^ \t]+)[ \t]+(.*)$")
+        line_pat = re.compile(b'^([^ \t]+)[ \t]+(.*)$')
         resp, raw_lines = self.longcmd('XGTITLE ' + group, file)
         lines = []
         for raw_line in raw_lines:
@@ -516,8 +519,8 @@ class NNTP:
         resp: server response if successful
         path: directory path to article"""
 
-        resp = self.shortcmd("XPATH " + id)
+        resp = self.shortcmd('XPATH {0}'.format(id))
-        if resp[:3] != '223':
+        if not resp.startswith(b'223'):
             raise NNTPReplyError(resp)
         try:
             [resp_num, path] = resp.split()
@@ -535,7 +538,7 @@ class NNTP:
         time: Time suitable for newnews/newgroups commands etc."""
 
         resp = self.shortcmd("DATE")
-        if resp[:3] != '111':
+        if not resp.startswith(b'111'):
             raise NNTPReplyError(resp)
         elem = resp.split()
         if len(elem) != 2:
@@ -546,28 +549,29 @@ class NNTP:
             raise NNTPDataError(resp)
         return resp, date, time
 
+    def _post(self, command, f):
+        resp = self.shortcmd(command)
+        # Raises error_??? if posting is not allowed
+        if not resp.startswith(b'3'):
+            raise NNTPReplyError(resp)
+        while 1:
+            line = f.readline()
+            if not line:
+                break
+            if line.endswith(b'\n'):
+                line = line[:-1]
+            if line.startswith(b'.'):
+                line = b'.' + line
+            self.putline(line)
+        self.putline(b'.')
+        return self.getresp()
 
     def post(self, f):
         """Process a POST command.  Arguments:
         - f: file containing the article
         Returns:
         - resp: server response if successful"""
-
+        return self._post('POST', f)
-        resp = self.shortcmd('POST')
-        # Raises error_??? if posting is not allowed
-        if resp[0] != '3':
-            raise NNTPReplyError(resp)
-        while 1:
-            line = f.readline()
-            if not line:
-                break
-            if line[-1] == '\n':
-                line = line[:-1]
-            if line[:1] == '.':
-                line = '.' + line
-            self.putline(line)
-        self.putline('.')
-        return self.getresp()
 
     def ihave(self, id, f):
         """Process an IHAVE command.  Arguments:
@@ -576,22 +580,7 @@ class NNTP:
         Returns:
         - resp: server response if successful
         Note that if the server refuses the article an exception is raised."""
-
+        return self._post('IHAVE {0}'.format(id), f)
-        resp = self.shortcmd('IHAVE ' + id)
-        # Raises error_??? if the server already has it
-        if resp[0] != '3':
-            raise NNTPReplyError(resp)
-        while 1:
-            line = f.readline()
-            if not line:
-                break
-            if line[-1] == '\n':
-                line = line[:-1]
-            if line[:1] == '.':
-                line = '.' + line
-            self.putline(line)
-        self.putline('.')
-        return self.getresp()
 
     def quit(self):
         """Process a QUIT command and close the socket.  Returns:
@@ -620,7 +609,7 @@ if __name__ == '__main__':
     resp, count, first, last, name = s.group('comp.lang.python')
     print(resp)
     print('Group', name, 'has', count, 'articles, range', first, 'to', last)
-    resp, subs = s.xhdr('subject', first + '-' + last)
+    resp, subs = s.xhdr('subject', '{0}-{1}'.format(first, last))
     print(resp)
     for item in subs:
         print("%7s %s" % item)

Misc/NEWS

@@ -15,6 +15,8 @@ What's New in Python 3.0 beta 5
 Core and Builtins
 -----------------
 
+- Issue #3714: Fixed nntplib by using bytes where appropriate.
+
 - Issue #1210: Fixed imaplib and its documentation.
 
 - Issue #4233: Changed semantic of ``_fileio.FileIO``'s ``close()``