RFC 4643:

   The server MAY list the AUTHINFO capability with no arguments, which
   indicates that it complies with this specification and does not
   permit any authentication commands in its current state.  In this
   case, the client MUST NOT attempt to utilize any AUTHINFO commands,
   even if it contains logic that might otherwise cause it to do so
   (e.g., for backward compatibility with servers that are not compliant
   with this specification).

Yet, nntplib attempts to authenticate.


self.capabilities() should be sent at startup.

If "READER" is advertised, no need to send a "MODE READER" command at all...

If "MODE-READER" is advertised, then "MODE READER" (if wanted) can be sent.
Then, self.capabilities() should be sent again.  Capabilities changed!

Then authentication if "AUTHINFO USER" is advertised with NNTP version >=2.  If "AUTHINFO" without "USER", no authentication at all.

And after authentication, self.capabilities() should be sent again.



Please note that the readermode_afterauth variable I see in the source code should normally not be used by a client...  RFC 4643 mentions:

   o  the MODE READER command MUST NOT be used in the
      same session following successful authentication.

By always sending capabilities at connection, some servers immediately throw an error. I've modified the class initialization to include an optional parameter to indicate if this should be disabled.

A drawback is that CAPABILITIES will still being unsent after an upgrade of the news server to a more up-to-date version supporting the command.

If CAPABILITIES is not understood by the news server, it is not an issue.  A 500 response code is usually answered by the news server, which is RFC-compliant.  nntplib should then behave properly.  (getcapabilities() returns an empty dictionary when CAPABILITIES is not implemented.)

> By always sending capabilities at connection, some servers immediately
> throw an error. I've modified the class initialization to include an
> optional parameter to indicate if this should be disabled.

Which kind of error? As Julien says, we could simply catch it and
proceed.
By the way, when sending a patch, please send a unified diff if possible
("diff -u"; if you are using Mercurial, this should be the default).

When using Easynews, it sends a 480 error (e.g. "nntplib.NNTPTemporaryError: 480 You must log in.")

I think I've fixed it to do as described. Alas, I have no Easynews account to test it (I mailed their support about that so maybe that'll change).

Would someone with an account mind to test, if it works? Nathan?

Hynek, the patch doesn't apply properly here. Are you soon it's been generated against an up-to-date working copy?

Also, I think the logic is wrong: the capabilities should *always* be queried after auth, even if they are already known. That's because they can change after auth is successful.

See RFC 4643: “Other capabilities returned in response to a CAPABILITIES
   command received after authentication MAY be different from those
   returned before authentication.  For example, an NNTP server may not
   want to advertise support for a specific extension unless a client
   has been authenticated.”

(oh, and better if it can come with a test)

Sure, I wanted to add tests as soon as I know that the flow is correct (which it isn't :)).

So essentially we want always

CAPABILITIES
LOGIN
CAPABILITIES

?

That's rather simple to achieve. The tests are going to be the harder part. ;)

Re the patch: I tried to generate it using SourceTree but probably did something wrong – will use hg next time again.

I've added general tests of AUTH USER/PASS as there weren't any present yet.

As I haven't alas heard back from Easynews, I can only presume they allow CAPABILITIES after a login – I've added a test case that models this behavior.

Otherwise I did as it was asked: getcapabilities() is tolerant to temporary errors and we get them again after a login now.

(I'm also using the remote repo tool now to avoid further patch fuckups – sorry for the extra mails.)

I've made a couple of small comments (see "review" link next to the patch).

Thanks! I think I've addressed everything in the latest patch.

New changeset d789f453387d by Antoine Pitrou in branch '3.2':
Issue #10287: nntplib now queries the server's CAPABILITIES again after authenticating (since the result may change, according to RFC 4643).
http://hg.python.org/cpython/rev/d789f453387d

New changeset 98ad67529241 by Antoine Pitrou in branch 'default':
Issue #10287: nntplib now queries the server's CAPABILITIES again after authenticating (since the result may change, according to RFC 4643).
http://hg.python.org/cpython/rev/98ad67529241

Thank you :) Committed now.

Perhaps I closed too quickly... Julien's original message had two other requests:

“If "READER" is advertised, no need to send a "MODE READER" command at all...

If "MODE-READER" is advertised, then "MODE READER" (if wanted) can be sent.
Then, self.capabilities() should be sent again.  Capabilities changed!”

There's one more issue Julien has raised: nntplib attempts to authenticate when "AUTHINFO" is sent w/o USER. (haven't tested it but I presume it's still valid)

It's trivial to catch that but I'd say that it's fine to let the server handle it if the user has specifically told us to authenticate by giving credentials – no? Maybe if dealing with not-entirely-rfc-compliant servers?

I will look into the READER/MODE-READER part.

So my take on the whole issue and Antoine "tends to agree". ;)

1. If the user tells us (s)he _wants_ us to authenticate or send MODE READER, we do it even if capabilities don't advertise it. There's a lot of rfc-non-conformant servers out there. Permanent errors (i.e. not supported) will be gracefully swallowed.

That's already status quo.

2. If the server tells us it already _is_ in READER mode, we don't send it because in that case we can assume the server knows what it's doing.

The next patch checks whether we're in READER mode before trying to switch the mode.

I've also added a test for the basic case where a connection is made to a MODE-READER advertising server.

I also added a handler to the nntp2 tests that raises an exception if "mode reader" is called although the server advertises itself as "reader".

If there's a consensus of not sending "MODE READER" if not advertised, it's a matter of two lines to fix that. Personally, I like this middle ground best.

I think the patch is fine. Will apply soon (thanks!)
.

New changeset a7f1ffd741d6 by Antoine Pitrou in branch '3.2':
Issue #10287: nntplib now queries the server's CAPABILITIES first before sending MODE READER, and only sends it if not already in READER mode.
http://hg.python.org/cpython/rev/a7f1ffd741d6

New changeset 03019bb62d83 by Antoine Pitrou in branch 'default':
Issue #10287: nntplib now queries the server's CAPABILITIES first before sending MODE READER, and only sends it if not already in READER mode.
http://hg.python.org/cpython/rev/03019bb62d83

This issue can now be closed for good.