classification
Title: Misleading error from getspnam function of spwd module
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: berker.peksag, loewis, pitrou, python-dev, rbcollins, serhiy.storchaka, vajrasky
Priority: low Keywords: patch

Created on 2013-08-20 04:13 by vajrasky, last changed 2016-03-23 20:16 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
fix_error_message_getspnam.patch vajrasky, 2013-08-20 04:13 review
fix_error_message_getspnam_v2.patch vajrasky, 2013-08-20 10:50 review
issue18787.diff berker.peksag, 2015-09-24 19:22 review
Messages (12)
msg195678 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-08-20 04:12
As root:

$ sudo python3
[sudo] password for ethan: 
Python 3.2.3 (default, Apr 10 2013, 05:07:54) 
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import spwd
>>> spwd.getspnam("I_don't_exist")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'getspnam(): name not found'
>>> spwd.getspnam("bin")
spwd.struct_spwd(sp_nam='bin', sp_pwd='*', sp_lstchg=15259, sp_min=0, sp_max=99999, sp_warn=7, sp_inact=-1, sp_expire=-1, sp_flag=-1)

As normal user:

$ python3
Python 3.2.3 (default, Apr 10 2013, 05:07:54) 
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import spwd
>>> spwd.getspnam("I_don't_exist")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'getspnam(): name not found'
>>> spwd.getspnam("bin")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'getspnam(): name not found'

People can be confused. How could the name not found if the name actually exists?

Attached the patch to differentiate the error whether the error is really 'name not found' or 'permission denied'.

I use the error message from the getspnam manual:
http://man7.org/linux/man-pages/man3/getspnam.3.html

There is another error message from getspnam beside EACCES (permission denied), which is ERANGE, but I don't think we need to handle this.
msg195683 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-20 09:32
PyErr_SetFromErrno(PyExc_OSError) looks more appropriate for permission denied error. However this change is backward incompatible so perhaps we have to left a KeyError.
msg195685 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-08-20 10:50
Attached the patch to accomodate Serhiy Storchaka's suggestion by using KeyError exception for backward compatibility.
msg195688 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-20 13:50
It was not a suggestion, it was a doubt.

See also issue10388.
msg195693 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-08-20 14:28
Sorry about that.

Okay, so for now, we wait for other people's suggestion, which way we should use to handle this situation?
msg233288 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-01-01 17:44
I think we should always raise OSError if errno is non-zero (and not KeyError).
msg247375 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015-07-25 19:07
Sounds like we want something analogous to http://bugs.python.org/file33404/fix_error_message_getspall_v2.patch for this bug. Moving to patch needed.

It looks like it would be easily tested too, so I'd like to see a test too please.
msg251542 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-09-24 19:22
Here is a patch with a test.
msg251544 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-24 20:03
As far as this change breaks compatibility. please add a note in the "Porting to Python 3.6" section in What's New.
msg262034 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-19 09:43
New changeset a1d738158390 by Berker Peksag in branch 'default':
Issue #18787: spwd.getspnam() now raises a PermissionError if the user
https://hg.python.org/cpython/rev/a1d738158390
msg262292 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-23 17:47
New changeset 430af393d8f1 by Victor Stinner in branch 'default':
Try to fix test_spwd on OpenIndiana
https://hg.python.org/cpython/rev/430af393d8f1
msg262303 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-23 20:16
New changeset c1677c8f92e1 by Victor Stinner in branch 'default':
Fix test_spwd on OpenIndiana
https://hg.python.org/cpython/rev/c1677c8f92e1
History
Date User Action Args
2016-03-23 20:16:18python-devsetmessages: + msg262303
2016-03-23 17:47:56python-devsetmessages: + msg262292
2016-03-19 09:44:42berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2016-03-19 09:43:04python-devsetnosy: + python-dev
messages: + msg262034
2015-09-24 20:03:10serhiy.storchakasetmessages: + msg251544
2015-09-24 19:22:40berker.peksagsetfiles: + issue18787.diff
versions: + Python 3.6, - Python 3.4
nosy: + berker.peksag

messages: + msg251542

stage: needs patch -> patch review
2015-07-25 19:07:08rbcollinssetnosy: + rbcollins

messages: + msg247375
stage: test needed -> needs patch
2015-01-01 17:44:32pitrousetnosy: + pitrou
messages: + msg233288
2014-12-31 16:23:50akuchlingsetnosy: - akuchling
2014-01-09 20:41:21serhiy.storchakasetpriority: normal -> low
2013-08-20 14:28:43vajraskysetmessages: + msg195693
2013-08-20 13:50:38serhiy.storchakasetnosy: + akuchling
messages: + msg195688
2013-08-20 10:50:11vajraskysetfiles: + fix_error_message_getspnam_v2.patch

messages: + msg195685
2013-08-20 09:32:45serhiy.storchakasetnosy: + loewis, serhiy.storchaka

messages: + msg195683
stage: test needed
2013-08-20 04:13:01vajraskycreate