classification
Title: Difference in behaviour with grp.getgrgid and pwd.getpwuid
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: SilentGhost, SimonFr, brett.cannon, larry, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-01-15 19:48 by SimonFr, last changed 2016-01-18 16:53 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
grp_getgrgid_non_integer_gid.patch serhiy.storchaka, 2016-01-17 15:00 review
Messages (7)
msg258325 - (view) Author: Simon Fraser (SimonFr) Date: 2016-01-15 19:48
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?
msg258467 - (view) Author: SilentGhost (SilentGhost) * Date: 2016-01-17 13:54
I'm adding Larry since it was his implementation of _Py_Uid_Converter in issue15301 that seems to lead to this behaviour.
msg258468 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2016-01-17 14:15
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.)
msg258472 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-17 15:00
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.
msg258476 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-01-17 17:56
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.
msg258525 - (view) Author: Roundup Robot (python-dev) Date: 2016-01-18 16:51
New changeset 65e0e06b70b6 by Serhiy Storchaka in branch 'default':
Issue #26129: Deprecated accepting non-integers in grp.getgrgid().
https://hg.python.org/cpython/rev/65e0e06b70b6
msg258526 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-18 16:53
Thank you for your review Brett.
History
Date User Action Args
2016-01-18 16:53:45serhiy.storchakasetstatus: open -> closed
messages: + msg258526

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2016-01-18 16:51:21python-devsetnosy: + python-dev
messages: + msg258525
2016-01-17 17:56:22brett.cannonsetmessages: + msg258476
2016-01-17 15:00:26serhiy.storchakasetstatus: closed -> open
files: + grp_getgrgid_non_integer_gid.patch


keywords: + patch
nosy: + brett.cannon, serhiy.storchaka
messages: + msg258472
resolution: wont fix -> (no value)
stage: resolved -> patch review
2016-01-17 14:15:36larrysetstatus: open -> closed
resolution: wont fix
messages: + msg258468

stage: resolved
2016-01-17 13:54:28SilentGhostsetnosy: + SilentGhost, larry

messages: + msg258467
versions: + Python 3.6, - Python 2.7, Python 3.5
2016-01-15 19:48:25SimonFrcreate