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

Use IntEnum and IntFlags in ssl module #72212

Closed
tiran opened this issue Sep 8, 2016 · 10 comments
Closed

Use IntEnum and IntFlags in ssl module #72212

tiran opened this issue Sep 8, 2016 · 10 comments
Labels
type-feature A feature request or enhancement

Comments

@tiran
Copy link
Member

tiran commented Sep 8, 2016

BPO 28025
Nosy @giampaolo, @tiran, @alex, @ethanfurman, @dstufft
Files
  • ssl_enum.patch
  • convert-ssl-constants-to-enums.patch
  • 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 2017-03-07.18:49:32.946>
    created_at = <Date 2016-09-08.17:35:49.514>
    labels = ['type-feature']
    title = 'Use IntEnum and IntFlags in ssl module'
    updated_at = <Date 2017-03-07.18:49:32.945>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2017-03-07.18:49:32.945>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-03-07.18:49:32.946>
    closer = 'serhiy.storchaka'
    components = []
    creation = <Date 2016-09-08.17:35:49.514>
    creator = 'christian.heimes'
    dependencies = []
    files = ['44468', '44473']
    hgrepos = []
    issue_num = 28025
    keywords = ['patch']
    message_count = 10.0
    messages = ['275076', '275096', '275113', '275119', '275131', '275137', '275138', '275146', '275155', '275472']
    nosy_count = 7.0
    nosy_names = ['janssen', 'giampaolo.rodola', 'christian.heimes', 'alex', 'ethan.furman', 'python-dev', 'dstufft']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue28025'
    versions = ['Python 3.6']

    @tiran
    Copy link
    Member Author

    tiran commented Sep 8, 2016

    The patch removes ssl._import_symbols and adds more enums to the ssl module. Please review the patch. I'll update documentation in the next step.

    With IntFlags it is much easier to understand flags like SSLContext.options

    >>> import ssl
    >>> ctx = ssl.create_default_context()
    >>> ssl.Options(ctx.options)
    <Options.OP_ALL|OP_NO_SSLv3|OP_NO_SSLv2|OP_NO_COMPRESSION: 2197947391>

    @tiran tiran added the type-feature A feature request or enhancement label Sep 8, 2016
    @tiran
    Copy link
    Member Author

    tiran commented Sep 8, 2016

    Thanks Ethan! I have updated the patch with documentation and your fix.

    Do you have an easy recipe to wrap properties like SSLContext.options to return an enum? It's not easy because super() does not support attribute proxy assignment. super().verify_mode = value raises an AttributeError.

    @ethanfurman
    Copy link
    Member

    ethanfurman commented Sep 8, 2016

    Can you give me a code sample?

    Also, wouldn't

    <Options.ALL|NO_SSLv3|NO_SSLv2|NO_COMPRESSION: 2197947391>

    be more readable? And keep the "OP_" names as aliases.

    @tiran
    Copy link
    Member Author

    tiran commented Sep 8, 2016

    I tried:

    class SSLContext(_SSLContext):
        ...
    
        @property
        def options(self):
            return Options(super().options)
    
        @options.setter
        def options(self, value)
            super().options = value
        #          ^^^^^
        #          This fails with an AttributeError
    
        # it would be cool to have something like this:
        options = MagicEnumWrapper(Options)

    About the shorter names, I'm worried that it might confuse people. The long names have been module constants for a long time.

    @ethanfurman
    Copy link
    Member

    ethanfurman commented Sep 8, 2016

    Huh. Well, for property this works:

        @property
        def options(self):
            return Options(_SSLContext.options.__get__(self))
    
        @options.setter
        def options(self, value):
            _SSLContext.options.__set__(self, Options.OP_ALL)

    Sure is ugly, though.

    I think there's a PEP about making super() work with descriptors... Ah, PEP-447 (not sure it's the same issue, though.)

    @ethanfurman
    Copy link
    Member

    ethanfurman commented Sep 8, 2016

    A little more research shows this is a problem with inheriting descriptors.

    @ethanfurman
    Copy link
    Member

    ethanfurman commented Sep 8, 2016

    See bpo-14965.

    @ethanfurman
    Copy link
    Member

    ethanfurman commented Sep 8, 2016

    Evidently the correct form is:

    super(SSLContext, SSLContext).options.__set__(self, value)
    

    Not sure that's any better. :(

    @tiran
    Copy link
    Member Author

    tiran commented Sep 8, 2016

    Too bad, but the ugly variant will suffice.

    >>> import ssl
    >>> ctx = ssl.create_default_context()
    >>> ctx.options
    <Options.OP_ALL|OP_NO_SSLv3|OP_NO_SSLv2|OP_NO_COMPRESSION: 2197947391>
    >>> ctx.verify_flags
    <VerifyFlags.VERIFY_X509_TRUSTED_FIRST: 32768>
    >>> ctx.verify_mode
    <VerifyMode.CERT_REQUIRED: 2>

    Latest patch: https://github.com/tiran/cpython/commits/feature/ssl_enum

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 9, 2016

    New changeset c32e9f9b00f7 by Christian Heimes in branch 'default':
    Issue bpo-28025: Convert all ssl module constants to IntEnum and IntFlags.
    https://hg.python.org/cpython/rev/c32e9f9b00f7

    @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
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants