Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imaplib should support SSL contexts #53054

Closed
pitrou opened this issue May 24, 2010 · 8 comments
Closed

imaplib should support SSL contexts #53054

pitrou opened this issue May 24, 2010 · 8 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@pitrou
Copy link
Member

pitrou commented May 24, 2010

BPO 8808
Nosy @jcea, @pitrou, @giampaolo
Files
  • 8808.patch: Patch for issue 8808
  • 8808.patch: Updated patch for 8808 with fixes from previous review
  • 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:

    assignee = None
    closed_at = <Date 2011-05-06.16:52:17.658>
    created_at = <Date 2010-05-24.16:13:55.904>
    labels = ['type-feature', 'library']
    title = 'imaplib should support SSL contexts'
    updated_at = <Date 2011-05-06.16:52:17.656>
    user = 'https://github.com/pitrou'

    bugs.python.org fields:

    activity = <Date 2011-05-06.16:52:17.656>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2011-05-06.16:52:17.658>
    closer = 'pitrou'
    components = ['Library (Lib)']
    creation = <Date 2010-05-24.16:13:55.904>
    creator = 'pitrou'
    dependencies = []
    files = ['21810', '21863']
    hgrepos = []
    issue_num = 8808
    keywords = ['patch']
    message_count = 8.0
    messages = ['106368', '134419', '134420', '134614', '134944', '135009', '135322', '135324']
    nosy_count = 6.0
    nosy_names = ['jcea', 'janssen', 'pitrou', 'giampaolo.rodola', 'sijinjoseph', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8808'
    versions = ['Python 3.3']

    @pitrou
    Copy link
    Member Author

    pitrou commented May 24, 2010

    3.2 introduces SSL contexts, which allow bundling SSL configuration options, certificates and private keys into a single (potentially long-lived) structure.
    http://docs.python.org/dev/py3k/library/ssl.html#ssl.SSLContext

    The IMAP4_SSL constructor should allow passing an SSL context object instead of a key/cert pair.

    @pitrou pitrou added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 24, 2010
    @sijinjoseph
    Copy link
    Mannequin

    sijinjoseph mannequin commented Apr 25, 2011

    Is anyone working on this?

    @pitrou
    Copy link
    Member Author

    pitrou commented Apr 25, 2011

    Is anyone working on this?

    I don't think so, you could try if you are interested.

    @sijinjoseph
    Copy link
    Mannequin

    sijinjoseph mannequin commented Apr 27, 2011

    I am attaching a patch for the default branch that adds a ssl_context parameter to IMAP4_SSL. Also added a couple of tests to test_imaplib to test the existing ctor with certfile and file and also the new one that accepts an SSLContext.

    Currently if the ssl_context param is provided then the keyfile and certfile are ignored, I wasn't sure if the ssl_context should be loaded with the certfile if that is provided along with the ssl_context.

    If this looks ok, I can add something similar for smtplib as well.

    @pitrou
    Copy link
    Member Author

    pitrou commented May 1, 2011

    Thanks for the patch. Comments:

    • the keyfile / certfile pair and the context parameter should be mutually exclusive (see e.g. the POP3_SSL constructor in Lib/poplib.py)
    • I don't think the remote test server used in test_imaplib supports client certificates, it probably just ignores them; that said, it's better than nothing
    • you have a misindented line in test_logincapa
    • since we're using a remote, third-party test server, it may be better not to do any spurious connects (in the current patch, a first connection is established in the setUp() and then ignored since another one is established in the test body)
    • you need to update the documentation in Doc/library/imaplib.rst

    @sijinjoseph
    Copy link
    Mannequin

    sijinjoseph mannequin commented May 2, 2011

    Thanks Antoine. I've attached an updated patch.

    >

    • the keyfile / certfile pair and the context parameter should be mutually exclusive (see e.g. the POP3_SSL constructor in Lib/poplib.py)
      [Sijin] - Yes, Thanks, I don't know why I didn't check POP3 impl before.

    • I don't think the remote test server used in test_imaplib supports client certificates, it probably just ignores them; that said, it's better than nothing
      [Sijin] - Agreed, I don't think the server supports client certificates, but at least we are able to test that the connect still works. Maybe we should open a new task to have a remote server that supports client certificate connections? We could use that in test cases for other SSL related modules.

    • you have a misindented line in test_logincapa
      [Sijin] - Fixed.

    • since we're using a remote, third-party test server, it may be better not to do any spurious connects (in the current patch, a first connection is established in the setUp() and then ignored since another one is established in the test body)
      [Sijin] - Fixed.

    • you need to update the documentation in Doc/library/imaplib.rst
      [Sijin] - Done. Also added some corresponding info in the poplib and docs for poplib.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 6, 2011

    New changeset aba7d1f2d2a9 by Antoine Pitrou in branch 'default':
    Issue bpo-8808: The IMAP4_SSL constructor now allows passing an SSLContext
    http://hg.python.org/cpython/rev/aba7d1f2d2a9

    @pitrou
    Copy link
    Member Author

    pitrou commented May 6, 2011

    Thank you! I've tweaked the patch slightly (mostly cosmetics (*)) and committed it to 3.3. I've left out the poplib doc changes, they could be committed separately.

    (*) 80-line character limit, calling logout() on the test server

    @pitrou pitrou closed this as completed May 6, 2011
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant