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

Difference in behaviour with grp.getgrgid and pwd.getpwuid #70317

Closed
SimonFr mannequin opened this issue Jan 15, 2016 · 7 comments
Closed

Difference in behaviour with grp.getgrgid and pwd.getpwuid #70317

SimonFr mannequin opened this issue Jan 15, 2016 · 7 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@SimonFr
Copy link
Mannequin

SimonFr mannequin commented Jan 15, 2016

BPO 26129
Nosy @brettcannon, @larryhastings, @serhiy-storchaka
Files
  • grp_getgrgid_non_integer_gid.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 = 'https://github.com/serhiy-storchaka'
    closed_at = <Date 2016-01-18.16:53:45.421>
    created_at = <Date 2016-01-15.19:48:25.318>
    labels = ['type-bug', 'library']
    title = 'Difference in behaviour with grp.getgrgid and pwd.getpwuid'
    updated_at = <Date 2016-01-18.16:53:45.420>
    user = 'https://bugs.python.org/SimonFr'

    bugs.python.org fields:

    activity = <Date 2016-01-18.16:53:45.420>
    actor = 'serhiy.storchaka'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2016-01-18.16:53:45.421>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2016-01-15.19:48:25.318>
    creator = 'SimonFr'
    dependencies = []
    files = ['41640']
    hgrepos = []
    issue_num = 26129
    keywords = ['patch']
    message_count = 7.0
    messages = ['258325', '258467', '258468', '258472', '258476', '258525', '258526']
    nosy_count = 6.0
    nosy_names = ['brett.cannon', 'larry', 'SilentGhost', 'python-dev', 'serhiy.storchaka', 'SimonFr']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26129'
    versions = ['Python 3.6']

    @SimonFr
    Copy link
    Mannequin Author

    SimonFr mannequin commented Jan 15, 2016

    grp.getgrgid is capable of accepting a string:

    from grp import getgrgid
    print(getgrgid('0'))

    However, pwd.getpwuid can't do the same:

    from pwd import getpwuid
    print(getpwuid('0'))
    Traceback (most recent call last):
      File "getpwuid_test.py", line 2, in <module>
        print(getpwuid('0'))
    TypeError: an integer is required

    This seems to be because inside Modules/pwdmodule.c, getpwuid uses PyNumber_ParseTuple with a converter that uses PyNumber_Index to get a Python integer, and that raises an exception on failure.

    However, in Modules/grpmodule.c, grp_getgrgid uses PyNumber_Long (Or PyNumber_Int for an old enough Python) as a conversion first, and as the documentation says at https://docs.python.org/3/c-api/number.html, this is the equivalent of running int(o), which can convert a string to an integer. Only then is it given to PyNumber_Index, by way of a helper function _Py_Gid_Converter

    Should these have different behaviours? Is there a reason for the difference?

    The behaviour of getgrgid seems more helpful, and it's odd that it doesn't apply to both functions. Is this undesirable behaviour in getgrgid or getpwuid?

    @SimonFr SimonFr mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 15, 2016
    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Jan 17, 2016

    I'm adding Larry since it was his implementation of _Py_Uid_Converter in bpo-15301 that seems to lead to this behaviour.

    @larryhastings
    Copy link
    Contributor

    larryhastings commented Jan 17, 2016

    Nope. Argument Clinic was merged in 3.4, and in 3.3 pwd.getpwuid wouldn't accept strings. So this isn't a bug introduced in the Clinic conversion in 3.4, this is historical behavior, and we can't change it now.

    If anything, I'd prefer that grp.getgrid *didn't* accept strings, but it's too late to change that too. "Helpfully" automatically calling int() on an argument is un-pythonic.

    (To answer your specific questions: they probably shouldn't have different behaviors, and I assume there's no particular reason for the difference. It's probably that they had different implementors, and one did a sloppier job than the other.)

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Jan 17, 2016

    This looks as unintentional consequences of ab0221811771.

    I think the current behavior of grp.getgrgid() is not correct, because it accepts str, float and other types. Python is strong-typed language and shouldn't make unwanted implicit type conversions. I guess the purpose was to support long arguments in Python 2.

    There is similar problem with grp.getgrnam() in 2.7. It accepts arguments of any types and convert them to str by calling str(). I guess the purpose was to support unicode arguments. In 3.x only str is accepted.

    Proposed patch deprecates accepting non-integer arguments in grp.getgrgid(). May be we can just remove this without starting deprecating process. I don't know.

    @brettcannon
    Copy link
    Member

    brettcannon commented Jan 17, 2016

    A single deprecation cycle should be enough since it obviously shouldn't work with floats and strings might make sense but updating code to not use them is easy enough.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 18, 2016

    New changeset 65e0e06b70b6 by Serhiy Storchaka in branch 'default':
    Issue bpo-26129: Deprecated accepting non-integers in grp.getgrgid().
    https://hg.python.org/cpython/rev/65e0e06b70b6

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Jan 18, 2016

    Thank you for your review Brett.

    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-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants