python / cpython

The Python programming language
https://www.python.org
Other
63.55k stars 30.45k forks source link

NNTPS support in nntplib #46220

Closed 899d1431-664d-46ec-8e90-b67a81c17cdf closed 14 years ago

899d1431-664d-46ec-8e90-b67a81c17cdf commented 16 years ago
BPO 1926
Nosy @pitrou, @giampaolo, @tiran
Dependencies
  • bpo-9360: nntplib cleanup
  • Files
  • python-nntps-patch-1.txt: Patch for NNTPS support in nntplib
  • python-nntps-patch-2.txt
  • python_nntp_ssl_patch1.diff: New nntps/starttls patch, for 3.2.
  • nntps_only_patch.diff
  • python_nntp_ssl_patch2.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = created_at = labels = ['type-feature', 'library'] title = 'NNTPS support in nntplib' updated_at = user = 'https://bugs.python.org/chasonr' ``` bugs.python.org fields: ```python activity = actor = 'ajvant' assignee = 'none' closed = True closed_date = closer = 'pitrou' components = ['Library (Lib)'] creation = creator = 'chasonr' dependencies = ['9360'] files = ['9280', '9286', '19463', '19485', '19519'] hgrepos = [] issue_num = 1926 keywords = ['patch'] message_count = 35.0 messages = ['61648', '61654', '61668', '61684', '61685', '61689', '61733', '61764', '61772', '98511', '115774', '115793', '116382', '120166', '120209', '120210', '120239', '120242', '120285', '120289', '120295', '120334', '120368', '120428', '120463', '120525', '120526', '120606', '120608', '120610', '120612', '120641', '120645', '120889', '120896'] nosy_count = 8.0 nosy_names = ['janssen', 'pitrou', 'giampaolo.rodola', 'christian.heimes', 'chasonr', 'ajvant', 'jelie', 'StevenJ'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue1926' versions = ['Python 3.2'] ```

    899d1431-664d-46ec-8e90-b67a81c17cdf commented 16 years ago

    This patch adds SSL support to nntplib. It is a followup to issue bpo-1535659 and addresses the objections raised in the comments to that issue; it also changes the default port to 563 (nntps) rather than 119 if SSL is requested.

    tiran commented 16 years ago

    I assign it to janssens. He is our SSL expert. I also set the version to 2.6. New features get into the next major release.

    2d426258-7f6b-45ab-bd06-a8d8e1c32ffb commented 16 years ago

    Unfortunately, it uses the deprecated socket.ssl calls. Re-worked to use the new SSL module, it would be OK.

    899d1431-664d-46ec-8e90-b67a81c17cdf commented 16 years ago

    OK, I got a copy of the Subversion sources and the new SSL library looks like just what is needed here. Examining the other protocol modules that already support SSL, I find these things:

    httplib has: HTTPConnection(host[, port[, strict[, timeout]]]) HTTPSConnection(host[, port[, key_file[, cert_file[, strict[, timeout]]]]])

    poplib has: POP3(host[, port[, timeout]]) POP3_SSL(host[, port[, keyfile[, certfile]]])

    imaplib has: IMAP4([host[, port]]) IMAP4_SSL([host[, port[, keyfile[, certfile]]]])

    smtplib has: SMTP([host[, port[, local_hostname[, timeout]]]]) SMTP_SSL([host[, port[, local_hostname[, keyfile[, certfile[, timeout]]]]]])

    Now nntplib as it stands has no SSL-using variant. The existing factory method is: NNTP(host[, port[, user[, password[, readermode[, usenetrc]]]]])

    What seems most consistent with the other modules is to define: NNTP_SSL(host[, port[, keyfile[, certfile[, user[, password[, readermode[, usenetrc]]]]]]])

    2d426258-7f6b-45ab-bd06-a8d8e1c32ffb commented 16 years ago

    Sounds good. If you want to develop this with 2.5.1, you can get an API-compliant version of the SSL module for 2.5.1 from http://pypi.python.org/pypi/ssl/.

    899d1431-664d-46ec-8e90-b67a81c17cdf commented 16 years ago

    Here's take 2.

    The pre-patch NNTP class has a long and complicated constructor. Rather than duplicate this constructor in NNTP_SSL, the patch converts most of the NNTP class to a new class, NNTPBase, which takes an already-connected socket as a parameter. NNTP and NNTP_SSL both inherit NNTPBase and create that socket in their own respective ways.

    2d426258-7f6b-45ab-bd06-a8d8e1c32ffb commented 16 years ago

    Great, Ray.

    I don't see any test cases for the nntp library in the Lib/test/ directory. How can we make sure it works on the buildbots?

    Bill

    On Jan 25, 2008 12:49 PM, Ray Chason \report@bugs.python.org\ wrote:

    Ray Chason added the comment:

    Here's take 2.

    The pre-patch NNTP class has a long and complicated constructor. Rather than duplicate this constructor in NNTP_SSL, the patch converts most of the NNTP class to a new class, NNTPBase, which takes an already-connected socket as a parameter. NNTP and NNTP_SSL both inherit NNTPBase and create that socket in their own respective ways.

    Added file: http://bugs.python.org/file9286/python-nntps-patch-2.txt


    Tracker \report@bugs.python.org\ \http://bugs.python.org/issue1926\


    899d1431-664d-46ec-8e90-b67a81c17cdf commented 16 years ago

    It seems that I, or whoever writes any future test_nntplib.py, would have to understand how existing tests such as test_smtplib.py work. It looks like that one is setting up some kind of miniature mail server and accepting a connection on localhost -- neat trick, binding a server socket to port 0 so it still works if you have a real mail server running.

    Anyway, getting good coverage isn't something I can bang out in an afternoon. Class NNTPBase and its derivatives (I'm talking post-patch of course) have lots of methods and I want to cover them as well as possible.

    2d426258-7f6b-45ab-bd06-a8d8e1c32ffb commented 16 years ago

    Yeah, it took me a couple of months to do a reasonable test case for the SSL code.

    Perhaps we could test against an existing NNTP server? Like Google's?

    Bill

    On Jan 27, 2008 6:59 PM, Ray Chason \report@bugs.python.org\ wrote:

    Ray Chason added the comment:

    It seems that I, or whoever writes any future test_nntplib.py, would have to understand how existing tests such as test_smtplib.py work. It looks like that one is setting up some kind of miniature mail server and accepting a connection on localhost -- neat trick, binding a server socket to port 0 so it still works if you have a real mail server running.

    Anyway, getting good coverage isn't something I can bang out in an afternoon. Class NNTPBase and its derivatives (I'm talking post-patch of course) have lots of methods and I want to cover them as well as possible.


    Tracker \report@bugs.python.org\ \http://bugs.python.org/issue1926\


    pitrou commented 14 years ago

    Yes, a test could use Google or gmane (according to the FAQ, nntps is supported: http://gmane.org/faq.php ). The test should skip gracefully (using the skipTest() API) if the connection fails, so that network errors or service unavailability don't make buildbots go red.

    pitrou commented 14 years ago

    The patch might need a little reworking to make it work under 3.x, although probably not much. It should also, IMHO, take an instance of the new SSLContext class (*) as a parameter, rather than the keyfile and certfile args.

    (*) http://docs.python.org/dev/library/ssl.html#ssl-contexts

    giampaolo commented 14 years ago

    Unfortunately nntplib lacks a test suite. I created bpo-9791 to address this issue.

    pitrou commented 14 years ago

    It should be noted that there are two possibilities for encrypted NNTP:

    For the record, gmane provides the former (on snews.gmane.org:563) but not the latter.

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    Regarding these two possibilities, please note that the first one is discouraged (per RFC 4642). STARTTLS is the preferrable way to start a TLS session.

    In some existing implementations, TCP port 563 has been dedicated to NNTP over TLS. These implementations begin the TLS negotiation immediately upon connection and then continue with the initial steps of an NNTP session. This use of TLS on a separate port is discouraged for the reasons documented in Section 7 of "Using TLS with IMAP, POP3 and ACAP" [TLS-IMAPPOP].

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    At Antoine's suggestion I've written a new patch for this for 3.2, adding support for both SSL on 563 and STARTTLS on a normal connection. A NNTP_SSL class supports the former and a NNTP.starttls() method supports the latter. As a side effect of getting starttls working I had to add a NNTP.login() method as well. (see below)

    The patch updates nntplib.py, test_nntplib.py, nntplib.rst, and Misc/NEWS.

    Some implementation notes:

    By default, NNTP objects attempt to log in during initialization,and there didn't seem to be any support for logging in later. This was a problem as STARTTLS has to be issued before logging in. In order to get around this and handle a STARTTLS nuance involving CAPABILITIES, I moved the authentication part of _NNTPBase.__init__ into a method of its own, NNTP.login(), and another piece of it into NNTP.getcapabilities(). This doesn't affect the observable behavior of NNTP() or the existing methods.

    As a side note, IMHO NNTP shouldn't attempt to log in at initialization at all. The user, password, and usenetrc args should not be part of the constructors and the program using the library should be able to choose when or if to log in after connecting. I couldn't think of any way to do this without breaking existing programs, though. At the very least I think usenetrc should default to False, but that would break programs that rely on the default behavior too, I think.

    I updated the test cases, with a caveat. They use gmane to perform the tests. Gmane doesn't support starttls. Therefor the tests currently only check for correct failure rather than correct success. There's test code for the starttls() method but it never runs. I checked the functionality against eternal-september to be sure it does, in fact, correctly succeed as well, but I'm not about to put my login details in the automatic test script. ;-)

    The NNTPS-on-563 test, OTOH, works exactly like the existing NNTP test and seems to work fine. It was a hell of a lot easier to implement that part too.

    Hope I didn't miss anything. First time doing this for a public project. Apparently the personal-itch theory is correct.

    \<click>

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    Also, Julien: Thanks for mentioning the 563-discouraged thing. I added a note about it to the new documentation before submitting.

    pitrou commented 14 years ago

    Andrew, thank you for posting a patch. I have a couple of comments:

    All in all, it looks good though.

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    Thanks a lot for having implemented STARTTLS, Andrew! That's great news!

    + Since the order in which certain operations need to be done varies + between normal, SSL, and STARTTLS connections varies, some + initialization must be done in the subclasses.

    One "varies" is enough.

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    On 2 Nov 2010 at 17:05, Antoine Pitrou wrote:

    • the command-line option for the SSLContext won't work, since a context is a custom object, not a string; I would rework this part anyway, since I don't think separate options for the "SSL host" are useful (I'd rather add a SSL-enabling flag; also, STARTTLS can be queried from the capabilities)

    I'm not sure I understand which part you're talking about here....the tests don't take a context for anything that I can see, and the mechanism I use in nntplib (a context as a named argument) is the same one used in smtplib and poplib for \<prot>_SSL.

    (if you can suggest a method-of-use that breaks it, that may enlighten me better)

    • on a stylistic note, there are a couple of places where you use tabs for indentation; also, comments should have a space after the '#' ('# xxx' and not '#xxx')

    Meh. My editor wasn't set up properly when I started. I thought I'd eliminated all the tabs. Guess not. There's probably a way to grep for lines that have tabs in them, I'll fix it.

    • not moving methods around (such as getwelcome) would make it easier to review the real changes

    I'll see what I can do. Some amount of that was necessary to fit STARTTLS in. (because init assumed you wanted to log in right away and never again)

    • in test_starttls, I would clearly report that the server doesn't support STARTTLS, e.g.:

    Can do. I didn't know about self.skip; I don't really understand the testing framework, I was just working by comparison to the other tests.

    • starttls() should probably test the tls_on attribute first and raise a ValueError if True (as you point out, a client mustn't attempt to start a new TLS session if one is already active).

    ...I actually meant to do exactly that, but forgot to put the check in. Thank you.

    --

    Andrew.

    pitrou commented 14 years ago

    > * the command-line option for the SSLContext won't work, since a > context is a custom object, not a string; I would rework this part > anyway, since I don't think separate options for the "SSL host" are > useful (I'd rather add a SSL-enabling flag; also, STARTTLS can be > queried from the capabilities)

    I'm not sure I understand which part you're talking about here....the tests don't take a context for anything that I can see, and the mechanism I use in nntplib (a context as a named argument) is the same one used in smtplib and poplib for \<prot>_SSL.

    I'm talking about the code under "if __name == '__main'". Specifically the "-c" option for specifying an SSL context.

    > * in test_starttls, I would clearly report that the server doesn't > support STARTTLS, e.g.:

    Can do. I didn't know about self.skip; I don't really understand the testing framework, I was just working by comparison to the other tests.

    Well, actually it's "self.skipTest(...)". My bad.

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    On 2 Nov 2010 at 23:22, Antoine Pitrou wrote:

    I'm talking about the code under "if __name == '__main'". Specifically the "-c" option for specifying an SSL context.

    Now I feel silly. Partly because of the mistake, partly because I did test_nntplib.py last and by the time I was done I'd forgotten there was another section of test code involved.

    Thanks for the advice; I'll fix it as best I can in the next day or two and submit a new version.

    --

    Andrew

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago
    • starttls() should probably test the tls_on attribute first and raise a ValueError if True (as you point out, a client mustn't attempt to start a new TLS session if one is already active).

    ...I actually meant to do exactly that, but forgot to put the check in. Thank you.

    And it should also make sure the user is not already authenticated. And that STARTTLS is advertised (if nntp_version >=2).

    407c39f8-c061-4f4b-ba0e-f1f9a6dcf0d2 commented 14 years ago

    I too was just looking at NNTPS support because I have a need for it. The latest patch looks like great work and I think the things it implements are needed for this library. But it seems to me that the latest patch combines 3 things which might otherwise be able to be separately considered. NNTPS, START_TLS (RFC 4642) extension and AUTHINFO extension (RFC 4643). It may be that START_TLS and AUTHINFO are indivisible, I need to read those more, but shouldn't that be a new topic of discussion as this feature request is for NNTPS support?

    I also don't understand the difficulty with plain NNTPS as it doesn't need a new interface a very simple patch (attached) achieves NNTPS?? (Most of the patch is test case and variant defaults. The actual NNTPS code is just: + # Make sure we can actually use ssl if its attempted. + if ssl_context: + self.sslcontext = ssl_context + self.sock = self.sslcontext.wrap_socket(self.sock)

    I also don't understand why START_TLS and AUTHINFO need to change how the module is interfaced to (separating log in/authentication, etc), my reading of START_TLS and AUTHINFO seem to me that it should all be under the covers. It even explains this in Section 7 of "Using TLS with IMAP, POP3 and ACAP" [TLS-IMAPPOP]. That the idea is "It just works". So surely if someone uses this module and they do not specify NNTPS and it supports START_TLS and AUTHINFO and so does the server then it just works. Otherwise it seems a bunch of NNTP Extension requirements and processing spills over to the users of this module when they can probably be contained locally??

    Perhaps there needs to be a separate feature request "START_TLS and AUTHINFO extension support for nntplib" so the issues and any necessary interface changes can be considered in isolation from simple NNTP over SSL?

    I think it would be nice to have NNTPS in for 3.2.

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    Hi Steven,

    I also don't understand why START_TLS and AUTHINFO need to change how the module is interfaced to (separating log in/authentication, etc)

    Because once you have used AUTHINFO, STARTTLS is no longer a valid command in a session. The authentication part is currently delt with in the init function (in nntplib) so it needs to be separated because one could want to first use STARTTLS, and then AUTHINFO. Currently, AUTHINFO is sent just after the initial log in; it is therefore better to have AUTHINFO handled in another function.

    407c39f8-c061-4f4b-ba0e-f1f9a6dcf0d2 commented 14 years ago

    Hi Julien,

    > I also don't understand why START_TLS and AUTHINFO need to change > how the module is interfaced to (separating log in/authentication, etc)

    Because once you have used AUTHINFO, STARTTLS is no longer a valid command in a session.

    I understand this, but doesn't this mean the init function needs to change to something like:

    init: if capability STARTTLS is advertised STARTLS stuff reget capabilities because STARTTLS changed them probably

    Now handle AUTHINFO Stuff

    Is there a case where a server advertises STARTTLS and one would not use it? If so then couldn't that just be handled with an option to the class inhibiting it?

    This is one of the reasons why I proposed dividing the job across two features.

    1. Simple NNTPS which seems to be the most common secured NNTP in use at the moment and is easy to implement, and verify (either as an improvement to the NNTP class or as a new NNTP_SSL class).
    2. SASL AUTHINFO/STARTTLS Extension handling implementation and improvements.

    There are also other bugs with properly handling capabilities at start related to this, are there not? http://bugs.python.org/issue10287

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    Hi Steven,

    I agree with what you suggest for the implementation.

    Is there a case where a server advertises STARTTLS and one would not use it?

    Yes, the overhead added by the encryption. It is what people usually mention for the reason not to use it. However, the TLS protocol allows to negotiate compression; in this case, it is very powerful! LIST ACTIVE answers are for instance a lot faster. The same for OVER answers.

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    (Note that smtplib can give ideas for an implementation of AUTHINFO SASL with PLAIN, LOGIN and CRAM-MD5 mechanisms.)

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    Here's a second version of the previous patch taking into account the errors Antoine noticed and some odds and ends from the other comments. Specifically:

    Comments fixed and tabs (I think...I hope...) all removed.

    Added explicit skip to test_nntplib when attempting to test starttls using a server that doesn't support it.

    Exceptions raised when issuing STARTTLS for: Authorization already done, TLS already active, server not advertising STARTTLS.

    Method moving: I'm not sure why the first patch seemed to think I was e.g. moving getwelcome (I wasn't, as far as I knew), but this time I put the new functions at the end of the class and it's no longer doing it. (still a bit of a mess for the code moved from __init__ to login, starttls, and readermode though)

    Selftest cli option: I threw out what I did before (as pointed out, it didn't work anyway) and added a -l flag to use SSL. It uses port 563 for the test only if the user hasn't asked for a non-default port.

    I'd like to address some of the objections other users have to the way I implemented it the first time.

    NNTPS and STARTTLS separation:

    One objection noted that nntps and STARTTLS support are really separate things and should be handled as such. Actually they're probably right. I did both at once because I view them as simply two ways of performing the same function, as someone mentioned earlier in the thread.

    Unnecessary Method Separation:

    One objection noted that separating out login and starttls was unnecessary and the calling program shouldn't have to worry about these steps. I disagree; I think the caller should be able to choose when and if to log in or engage encryption. (in fact I think usenetrc should be false by default for this reason, but I figure that would break backwards compatibility for programs that rely on it being true by default, and I'm not sure what the rules are regarding this)

    More significant than my own potentially newbie-ish opinion is that the RFC suggests as a valid use case the idea of a client starting up TLS or authentication in reaction to a 483 command response, rather than right off the bat. I'm pretty sure this is impossible under the current setup, where login/encryption happens only at initialization and there's no method exposed to do it later.

    If I'm going to get overruled on this, I guess the other option is to add a starttls argument to the constructors. If so, I'd suggest it have settings to forbid starttls, use it if it's advertised, or insist on it (by raising an exception if it can't be used). Perhaps false/none/true respectively.

    Existing issue for AUTHINFO and MODE READER:

    One objection noted there's already an issue open for them (bpo-10287). Which is true, but the AUTHINFO and MODE READER changes were necessary to make STARTTLS work as intended. (in my opinion, I suppose) So I did them as I went along. I'm not sure if I violated custom there, but my apologies if so.

    At least having them functioned out will make most of what's mentioned in 10287 easier to fix, I suppose.

    Randomness:

    BTW, I've been maintaining the readermode_afterauth thing because of the comment next to it, but it sounds from that other issue like it's not supposed to be there at all...which kind of restores my faith in the universe since it smelled bad from the start to me.

    pitrou commented 14 years ago

    Thanks for the patch, Andrew. It looks mostly good.

    I would rename setreadermode to _setreadermode (there's no reason to make it public IMO). Also, I would not explicitly check "STARTTLS" in the capabilities. It the server doesn't support it, it will issue an error anyway. Some servers might support it without advertising it, who knows?

    I can take care of all that when committing, you don't need to submit a new patch (you can of course, if you want).

    (in fact I think usenetrc should be false by default for this reason, but I figure that would break backwards compatibility for programs that rely on it being true by default, and I'm not sure what the rules are regarding this)

    In 3.2, the reworked nntplib already breaks compatibility. It is reasonable to switch the default for usenetrc to False; but I prefer to do it in a separate commit for clarity.

    9249f25b-75db-43f4-aea7-00c24f0ddc5a commented 14 years ago

    More significant than my own potentially newbie-ish opinion is that the RFC suggests as a valid use case the idea of a client starting up TLS or authentication in reaction to a 483 command response, rather than right off the bat.

    Yes. (Currently, it would only be TLS with nntplib, because SASL mechanisms which negotiate an encryption layer are not implemented yet in nntplib. But in case someone wishes to test, I have the following capabilities on my news server, news.trigofacile.com : AUTHINFO USER SASL SASL GSSAPI OTP PLAIN NTLM LOGIN DIGEST-MD5 CRAM-MD5 STARTTLS )

    I'm pretty sure this is impossible under the current setup, where login/encryption happens only at initialization and there's no method exposed to do it later.

    Absolutely.

    I've been maintaining the readermode_afterauth thing [...] it smelled bad from the start to me.

    Yep. According to RFC 4643:

    Additionally, the client MUST NOT issue a MODE READER command after authentication, and a server MUST NOT advertise the MODE-READER capability.

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    On 6 Nov 2010 at 11:48, Antoine Pitrou wrote:

    I would rename setreadermode to _setreadermode (there's no reason to make it public IMO). Also, I would not explicitly check "STARTTLS" in the capabilities. It the server doesn't support it, it will issue an error anyway. Some servers might support it without advertising it, who knows?

    I can take care of all that when committing, you don't need to submit a new patch (you can of course, if you want).

    I'd appreciate it if you took care of it, if you feel the patch is otherwise commit-worthy. It takes a pretty excessive amount of time to convince myself that it's really doing what I want it to (and only what I want it to).

    I forgot to include a note for Misc/NEWS in the new patch version, by the way.

    > (in fact I think usenetrc should be false by default for this > reason, but I figure that would break backwards compatibility for > programs that rely on it being true by default, and I'm not sure > what the rules are regarding this)

    In 3.2, the reworked nntplib already breaks compatibility. It is reasonable to switch the default for usenetrc to False; but I prefer to do it in a separate commit for clarity.

    Okay. For the time being I left a note in the documentation pointing out that it had to be set False for starttls to be useful.

    --

    Andrew

    407c39f8-c061-4f4b-ba0e-f1f9a6dcf0d2 commented 14 years ago

    The only comment I have is, if the caller needs to organise when to auth and instigate tls then for completeness getcapabilities() should have an option to force a reget of the current capabilities, in line with rfc3977 5.2.2:

    An NNTP client MAY cache the results of this command, but MUST NOT rely on the correctness of any cached results, whether from earlier in this session or from a previous session, MUST cope gracefully with the cached status being out of date, and SHOULD (if caching results) provide a way to force the cached information to be refreshed.

    As it stands, the nntplib can cause the cached capabilities to be refreshed at certain points automatically (as it should), but I think it should be possible for the caller of the method to also specify that fresh capabilities are required and not cached ones.

    something like this perhaps? :

    mynntp.getcapabilites(refresh=True)
    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    On 6 Nov 2010 at 17:23, StevenJ wrote:

    As it stands, the nntplib can cause the cached capabilities to be refreshed at certain points automatically (as it should), but I think it should be possible for the caller of the method to also specify that fresh capabilities are required and not cached ones.

    I agree. I actually added a way to do this when I functioned out __init__, so that starttls could refresh it when required.

    It was ugly and not exposed to the caller though.

    something like this perhaps? :

    mynntp.getcapabilites(refresh=True)

    My method sucked, this one doesn't. Might belong in a separate issue though. (I feel like a bit of a hypocrite saying that now)

    --

    Andrew

    pitrou commented 14 years ago

    I have committed the patch in r86365, and I've made usenetrc False by default in r86366. Thanks for contributing!

    1ce17cca-77c1-4b53-8d66-ee24d61965f5 commented 14 years ago

    On 9 Nov 2010 at 18:59, Antoine Pitrou wrote:

    I have committed the patch in r86365, and I've made usenetrc False by default in r86366. Thanks for contributing!

    Woot. I thank you.

    Regarding usenetrc, the NNTP.login and NNTP.starttls documentation assumed it was true by default (correct when I wrote it) and mentioned it had to be forced false for starttls to work in some cases.

    I'm glad that's not the case anymore but the documentation as I wrote it is wrong now. :-)

    --

    Andrew