classification
Title: '_is_sunder' function in 'enum' module fails on empty string
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: Maxpxt, barry, bdbaraban, cheryl.sabella, corona10, eli.bendersky, ethan.furman, matrixise, miss-islington, remi.lapeyre
Priority: normal Keywords: easy, patch

Created on 2019-02-05 14:27 by Maxpxt, last changed 2019-03-08 21:44 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11840 closed matrixise, 2019-02-13 12:22
PR 11891 merged bdbaraban, 2019-02-16 07:06
PR 12150 merged miss-islington, 2019-03-03 22:09
PR 12151 closed miss-islington, 2019-03-03 22:09
Messages (17)
msg334866 - (view) Author: Maxwell (Maxpxt) Date: 2019-02-05 14:27
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'.
msg334867 - (view) Author: Maxwell (Maxpxt) Date: 2019-02-05 14:32
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)
msg334869 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2019-02-05 14:39
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?
msg334971 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-02-06 18:58
I agree with Rémi Lapeyre.

For reference, the len() check and current tests were added under issue 19156.
msg335028 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-02-07 17:20
Yes, the first solution will be fine.  Maxwell, can you create a github pull request with that?
msg335176 - (view) Author: Brennan D Baraban (bdbaraban) * Date: 2019-02-10 22:46
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.
msg335178 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-02-10 23:15
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.
msg335180 - (view) Author: Brennan D Baraban (bdbaraban) * Date: 2019-02-10 23:20
Got it, makes sense. Thank you. New contributor here :)
msg335427 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) Date: 2019-02-13 12:23
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.
msg335500 - (view) Author: Brennan D Baraban (bdbaraban) * Date: 2019-02-14 06:37
Thank you, Stéphane. I submitted a change request to your PR just now.
msg335595 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) Date: 2019-02-15 09:57
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
msg335644 - (view) Author: Brennan D Baraban (bdbaraban) * Date: 2019-02-15 20:47
Yes, I will submit a new PR today.
msg335799 - (view) Author: Stéphane Wirtel (matrixise) * (Python committer) Date: 2019-02-18 08:04
Thank you for your contribution.

Have a nice day,
msg336261 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-02-21 21:14
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.
msg337048 - (view) Author: miss-islington (miss-islington) Date: 2019-03-03 22:09
New changeset 8b914d2767acba3a9e78f1dacdc2d61dbfd7e304 by Miss Islington (bot) (Brennan D Baraban) in branch 'master':
bpo-35899: Fix Enum handling of empty and weird strings (GH-11891)
https://github.com/python/cpython/commit/8b914d2767acba3a9e78f1dacdc2d61dbfd7e304
msg337059 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-03-04 01:36
Thank you, everyone!
msg337534 - (view) Author: miss-islington (miss-islington) Date: 2019-03-08 21:44
New changeset 8755f0aeb67125a154e5665a24276fe85d269d85 by Miss Islington (bot) in branch '3.7':
bpo-35899: Fix Enum handling of empty and weird strings (GH-11891)
https://github.com/python/cpython/commit/8755f0aeb67125a154e5665a24276fe85d269d85
History
Date User Action Args
2019-03-08 21:44:27miss-islingtonsetmessages: + msg337534
2019-03-04 01:36:30ethan.furmansetstatus: open -> closed
versions: + Python 3.6, Python 3.8
messages: + msg337059

resolution: fixed
stage: patch review -> resolved
2019-03-03 22:09:43miss-islingtonsetpull_requests: + pull_request12151
2019-03-03 22:09:33miss-islingtonsetpull_requests: + pull_request12150
2019-03-03 22:09:16miss-islingtonsetnosy: + miss-islington
messages: + msg337048
2019-02-21 21:14:11ethan.furmansetmessages: + msg336261
2019-02-18 08:04:18matrixisesetmessages: + msg335799
2019-02-16 07:06:10bdbarabansetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request11919
2019-02-15 20:47:37bdbarabansetmessages: + msg335644
2019-02-15 09:57:37matrixisesetmessages: + msg335595
2019-02-14 06:37:55bdbarabansetmessages: + msg335500
2019-02-14 01:01:36cheryl.sabellasetmessages: - msg335483
2019-02-14 00:07:56cheryl.sabellasetmessages: + msg335483
2019-02-13 12:23:59matrixisesetnosy: + matrixise
messages: + msg335427

keywords: - patch
stage: patch review -> needs patch
2019-02-13 12:22:24matrixisesetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request11870
2019-02-10 23:20:55bdbarabansetmessages: + msg335180
2019-02-10 23:15:22ethan.furmansetmessages: + msg335178
2019-02-10 22:46:29bdbarabansetnosy: + bdbaraban
messages: + msg335176
2019-02-08 01:34:16corona10setnosy: + corona10
2019-02-07 17:20:36ethan.furmansetkeywords: + easy
type: crash -> behavior
messages: + msg335028

stage: needs patch
2019-02-06 18:58:19cheryl.sabellasetnosy: + cheryl.sabella
messages: + msg334971
2019-02-05 16:12:16rhettingersetassignee: ethan.furman
2019-02-05 15:22:55xtreaksetnosy: + barry, eli.bendersky, ethan.furman
2019-02-05 14:39:37remi.lapeyresetnosy: + remi.lapeyre
messages: + msg334869
2019-02-05 14:32:26Maxpxtsetmessages: + msg334867
2019-02-05 14:27:39Maxpxtcreate