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

'_is_sunder' function in 'enum' module fails on empty string #80080

Closed
Maxpxt mannequin opened this issue Feb 5, 2019 · 17 comments
Closed

'_is_sunder' function in 'enum' module fails on empty string #80080

Maxpxt mannequin opened this issue Feb 5, 2019 · 17 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Maxpxt
Copy link
Mannequin

Maxpxt mannequin commented Feb 5, 2019

BPO 35899
Nosy @warsaw, @ethanfurman, @matrixise, @csabella, @corona10, @miss-islington, @remilapeyre, @bdbaraban, @Maxpxt
PRs
  • bpo-35899: Support of empty and weird strings by Enum #11840
  • bpo-35899: Fix Enum handling of empty and weird strings #11891
  • [3.7] bpo-35899: Fix Enum handling of empty and weird strings (GH-11891) #12150
  • [3.6] bpo-35899: Fix Enum handling of empty and weird strings (GH-11891) #12151
  • 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 = 'https://github.com/ethanfurman'
    closed_at = <Date 2019-03-04.01:36:30.400>
    created_at = <Date 2019-02-05.14:27:38.984>
    labels = ['3.7', '3.8', 'type-bug', 'library', 'easy']
    title = "'_is_sunder' function in 'enum' module fails on empty string"
    updated_at = <Date 2019-03-08.21:44:27.325>
    user = 'https://github.com/Maxpxt'

    bugs.python.org fields:

    activity = <Date 2019-03-08.21:44:27.325>
    actor = 'miss-islington'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2019-03-04.01:36:30.400>
    closer = 'ethan.furman'
    components = ['Library (Lib)']
    creation = <Date 2019-02-05.14:27:38.984>
    creator = 'Maxpxt'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35899
    keywords = ['patch', 'easy']
    message_count = 17.0
    messages = ['334866', '334867', '334869', '334971', '335028', '335176', '335178', '335180', '335427', '335500', '335595', '335644', '335799', '336261', '337048', '337059', '337534']
    nosy_count = 10.0
    nosy_names = ['barry', 'eli.bendersky', 'ethan.furman', 'matrixise', 'cheryl.sabella', 'corona10', 'miss-islington', 'remi.lapeyre', 'bdbaraban', 'Maxpxt']
    pr_nums = ['11840', '11891', '12150', '12151']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue35899'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @Maxpxt
    Copy link
    Mannequin Author

    Maxpxt mannequin commented Feb 5, 2019

    This is a really minor bug.

    In enum.py the function _is_sunder(name) fails on empty string with an IndexError.

    As a result, attempting to create an Enum with an empty string fails.

    >>> from enum import Enum
    >>> Yay = Enum('Yay', ('', 'B', 'C'))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "C:\Program Files\Python37\lib\enum.py", line 311, in __call__
        return cls._create_(value, names, module=module, qualname=qualname, type=type, start=start)
      File "C:\Program Files\Python37\lib\enum.py", line 422, in _create_
        classdict[member_name] = member_value
      File "C:\Program Files\Python37\lib\enum.py", line 78, in __setitem__
        if _is_sunder(key):
      File "C:\Program Files\Python37\lib\enum.py", line 36, in _is_sunder
        return (name[0] == name[-1] == '_' and
    IndexError: string index out of range
    >>>

    Expected behavior is for it to not fail, as Enum accepts wierd strings. Example:

    >>> from enum import Enum
    >>> Yay = Enum('Yay', ('!', 'B', 'C'))
    >>> getattr(Yay, '!')
    <Yay.!: 1>
    >>>

    Transcript of lines 26 to 39 of enum.py:

    def _is_dunder(name):
        """Returns True if a __dunder__ name, False otherwise."""
        return (name[:2] == name[-2:] == '__' and
                name[2:3] != '_' and
                name[-3:-2] != '_' and
                len(name) > 4)
    
    
    def _is_sunder(name):
        """Returns True if a _sunder_ name, False otherwise."""
        return (name[0] == name[-1] == '_' and
                name[1:2] != '_' and
                name[-2:-1] != '_' and
                len(name) > 2)

    Solution 1: Replace with:

    def _is_dunder(name):
        """Returns True if a __dunder__ name, False otherwise."""
        return (len(name) > 4 and
                name[:2] == name[-2:] == '__' and
                name[2] != '_' and
                name[-3] != '_')
    
    
    def _is_sunder(name):
        """Returns True if a _sunder_ name, False otherwise."""
        return (len(name) > 2 and
                name[0] == name[-1] == '_' and
                name[1:2] != '_' and
                name[-2:-1] != '_')

    In this solution, function '_is_dunder' was also altered for consistency.
    Altering '_is_dunder' is not necessary, though.

    Solution 2: Replace with:

    def _is_dunder(name):
        """Returns True if a __dunder__ name, False otherwise."""
        return (name[:2] == name[-2:] == '__' and
                name[2:3] != '_' and
                name[-3:-2] != '_' and
                len(name) > 4)
    
    
    def _is_sunder(name):
        """Returns True if a _sunder_ name, False otherwise."""
        return (name[:0] == name[-1:] == '_' and
                name[1:2] != '_' and
                name[-2:-1] != '_' and
                len(name) > 2)

    In this solution, function '_is_sunder' was altered to follow
    the style used in function '_is_dunder'.

    @Maxpxt Maxpxt mannequin added type-crash A hard crash of the interpreter, possibly with a core dump 3.7 (EOL) end of life stdlib Python modules in the Lib dir labels Feb 5, 2019
    @Maxpxt
    Copy link
    Mannequin Author

    Maxpxt mannequin commented Feb 5, 2019

    Typo fix on solution 2:

    def _is_sunder(name):
        """Returns True if a _sunder_ name, False otherwise."""
        return (name[:1] == name[-1:] == '_' and
                name[1:2] != '_' and
                name[-2:-1] != '_' and
                len(name) > 2)

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Feb 5, 2019

    Hi @Maxpxt, I think your first solution is appropriate, do you want to open a new pull request with your solution and an appropriate test case?

    @csabella
    Copy link
    Contributor

    csabella commented Feb 6, 2019

    I agree with Rémi Lapeyre.

    For reference, the len() check and current tests were added under bpo-19156.

    @ethanfurman
    Copy link
    Member

    Yes, the first solution will be fine. Maxwell, can you create a github pull request with that?

    @ethanfurman ethanfurman added easy type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Feb 7, 2019
    @bdbaraban
    Copy link
    Mannequin

    bdbaraban mannequin commented Feb 10, 2019

    I'm not sure if Maxwell is still working on this issue, but may I pick it up? I can submit a PR within the day.

    @ethanfurman
    Copy link
    Member

    Let's give Maxwell until the 14th (so a week from when I asked him to turn his code into a patch) and if nothing from him by then you are welcome to take it over.

    @bdbaraban
    Copy link
    Mannequin

    bdbaraban mannequin commented Feb 10, 2019

    Got it, makes sense. Thank you. New contributor here :)

    @matrixise
    Copy link
    Member

    Hi,

    I have created the PR for Maxwell.

    After tomorrow, if we have no news from him, I propose to you to update/comment the PR. Of course, I will add a co-authored-by field in the commit.

    @bdbaraban
    Copy link
    Mannequin

    bdbaraban mannequin commented Feb 14, 2019

    Thank you, Stéphane. I submitted a change request to your PR just now.

    @matrixise
    Copy link
    Member

    Hi Brennan,

    Normally, you wanted to work on this issue and you have waited for one week after the last message of Maxwell.

    Do you want to work on this issue and submit your PR?

    Have a nice day,

    Stéphane

    @bdbaraban
    Copy link
    Mannequin

    bdbaraban mannequin commented Feb 15, 2019

    Yes, I will submit a new PR today.

    @matrixise
    Copy link
    Member

    Thank you for your contribution.

    Have a nice day,

    @ethanfurman
    Copy link
    Member

    The changes to _is_sunder and _is_dunder look good, but there is a problem with the underlying assumptions of what Enum should be doing:

    • nameless members are not to be allowed
    • non-alphanumeric characters are not supported

    In other words, while _is_sunder should not fail, neither should an empty string be allowed as a member name. This can be checked at line 154 (just add '' to the set) -- then double check that the error raised is a ValueError and not an IndexError.

    For the strange character portion, use some non-latin numbers and letters to make sure they work, but don't check for symbols such as exclamation points -- while they might work, we are not supporting such things, and having a test that checks to make sure they work suggests that we do support it.

    @miss-islington
    Copy link
    Contributor

    New changeset 8b914d2 by Miss Islington (bot) (Brennan D Baraban) in branch 'master':
    bpo-35899: Fix Enum handling of empty and weird strings (GH-11891)
    8b914d2

    @ethanfurman
    Copy link
    Member

    Thank you, everyone!

    @ethanfurman ethanfurman added the 3.8 only security fixes label Mar 4, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset 8755f0a by Miss Islington (bot) in branch '3.7':
    bpo-35899: Fix Enum handling of empty and weird strings (GH-11891)
    8755f0a

    @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
    3.7 (EOL) end of life 3.8 only security fixes easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants