Issue #10287: nntplib now queries the server's CAPABILITIES first before sending…

Issue #10287: nntplib now queries the server's CAPABILITIES first before sending MODE READER, and only sends it if not already in READER mode.
Patch by Hynek Schlawack.

Lib/nntplib.py

@@ -324,25 +324,30 @@ class _NNTPBase:
         self.debugging = 0
         self.welcome = self._getresp()
 
+        # Inquire about capabilities (RFC 3977).
+        self._caps = None
+        self.getcapabilities()
+
         # 'MODE READER' is sometimes necessary to enable 'reader' mode.
         # However, the order in which 'MODE READER' and 'AUTHINFO' need to
         # arrive differs between some NNTP servers. If _setreadermode() fails
         # with an authorization failed error, it will set this to True;
         # the login() routine will interpret that as a request to try again
         # after performing its normal function.
+        # Enable only if we're not already in READER mode anyway.
         self.readermode_afterauth = False
-        if readermode:
+        if readermode and 'READER' not in self._caps:
             self._setreadermode()
+            if not self.readermode_afterauth:
+                # Capabilities might have changed after MODE READER
+                self._caps = None
+                self.getcapabilities()
 
         # RFC 4642 2.2.2: Both the client and the server MUST know if there is
         # a TLS session active.  A client MUST NOT attempt to start a TLS
         # session if a TLS session is already active.
         self.tls_on = False
 
-        # Inquire about capabilities (RFC 3977).
-        self._caps = None
-        self.getcapabilities()
-
         # Log in and encryption setup order is left to subclasses.
         self.authenticated = False
 
@@ -959,8 +964,12 @@ class _NNTPBase:
         self._caps = None
         self.getcapabilities()
         # Attempt to send mode reader if it was requested after login.
-        if self.readermode_afterauth:
+        # Only do so if we're not in reader mode already.
+        if self.readermode_afterauth and 'READER' not in self._caps:
             self._setreadermode()
+            # Capabilities might have changed after MODE READER
+            self._caps = None
+            self.getcapabilities()
 
     def _setreadermode(self):
         try:

Lib/test/test_nntplib.py

@@ -385,6 +385,12 @@ class MockedNNTPTestsMixin:
         return self.server
 
 
+class MockedNNTPWithReaderModeMixin(MockedNNTPTestsMixin):
+    def setUp(self):
+        super().setUp()
+        self.make_server(readermode=True)
+
+
 class NNTPv1Handler:
     """A handler for RFC 977"""
 
@@ -725,6 +731,9 @@ class NNTPv2Handler(NNTPv1Handler):
         else:
             self.push_lit(fmt.format(''))
 
+    def handle_MODE(self, _):
+        raise Exception('MODE READER sent despite READER has been advertised')
+
     def handle_OVER(self, message_spec=None):
         return self.handle_XOVER(message_spec)
 
@@ -739,6 +748,34 @@ class CapsAfterLoginNNTPv2Handler(NNTPv2Handler):
             super().handle_CAPABILITIES()
 
 
+class ModeSwitchingNNTPv2Handler(NNTPv2Handler):
+    """A server that starts in transit mode"""
+
+    def __init__(self):
+        self._switched = False
+
+    def handle_CAPABILITIES(self):
+        fmt = """\
+            101 Capability list:
+            VERSION 2 3
+            IMPLEMENTATION INN 2.5.1
+            HDR
+            LIST ACTIVE ACTIVE.TIMES DISTRIB.PATS HEADERS NEWSGROUPS OVERVIEW.FMT
+            OVER
+            POST
+            {}READER
+            ."""
+        if self._switched:
+            self.push_lit(fmt.format(''))
+        else:
+            self.push_lit(fmt.format('MODE-'))
+
+    def handle_MODE(self, what):
+        assert not self._switched and what == 'reader'
+        self._switched = True
+        self.push_lit('200 Posting allowed')
+
+
 class NNTPv1v2TestsMixin:
 
     def setUp(self):
@@ -1145,6 +1182,18 @@ class CapsAfterLoginNNTPv2Tests(MockedNNTPTestsMixin, unittest.TestCase):
         self.assertIn('VERSION', self.server._caps)
 
 
+class SendReaderNNTPv2Tests(MockedNNTPWithReaderModeMixin,
+        unittest.TestCase):
+    """Same tests as for v2 but we tell NTTP to send MODE READER to a server
+    that isn't in READER mode by default."""
+
+    nntp_version = 2
+    handler_class = ModeSwitchingNNTPv2Handler
+
+    def test_we_are_in_reader_mode_after_connect(self):
+        self.assertIn('READER', self.server._caps)
+
+
 class MiscTests(unittest.TestCase):
 
     def test_decode_header(self):
@@ -1305,7 +1354,7 @@ class MiscTests(unittest.TestCase):
 
 def test_main():
     tests = [MiscTests, NNTPv1Tests, NNTPv2Tests, CapsAfterLoginNNTPv2Tests,
-            NetworkedNNTPTests]
+            SendReaderNNTPv2Tests, NetworkedNNTPTests]
     if _have_ssl:
         tests.append(NetworkedNNTP_SSLTests)
     support.run_unittest(*tests)

Misc/NEWS

@@ -466,6 +466,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #10287: nntplib now queries the server's CAPABILITIES first before
+  sending MODE READER, and only sends it if not already in READER mode.
+  Patch by Hynek Schlawack.
+
 - Issue #13993: HTMLParser is now able to handle broken end tags when
   strict=False.