After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669594 - bug in gspawn sequence-of-strings checking
bug in gspawn sequence-of-strings checking
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-07 18:26 UTC by Allison Karlitskaya (desrt)
Modified: 2012-02-07 20:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
pygspawn: improve error checking (1.85 KB, patch)
2012-02-07 18:45 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2012-02-07 18:26:19 UTC
this will cause a segfault:

from gi.repository import GLib
import os

GLib.spawn_async (argv=['echo', 'hello'], envp=os.environ)


when this is okay:

GLib.spawn_async (argv=['echo', 'hello'], envp=[])

and this is okay (and is properly detected as an error):

GLib.spawn_async (argv=['echo', 'hello'], envp={})


It appears that os.environ has some weird type that's not quite a dict that's tripping up the detection code here.  That's a problem for people who try to pass os.environ to envp thinking that it's a reasonable thing to do: we need to give them a proper error message in that case, not a segfault.
Comment 1 Allison Karlitskaya (desrt) 2012-02-07 18:45:44 UTC
Created attachment 207014 [details] [review]
pygspawn: improve error checking

gspawn 'argv' and 'envp' parameters expect sequences of strings.  This
is enforced by checking that the passed argument is a sequence and that
each item returned from it is a string.

We do now, however, verify that each item can be successfully taken from
the sequence.  'os.environ' is an example of an object that passes
PySequence_Check() but fails to return objects from PySequence_ITEM().

Add a simple NULL check to avoid the crash.
Comment 2 Allison Karlitskaya (desrt) 2012-02-07 18:46:59 UTC
I did a quick check for PySequence_ITEM and PySequence_GetItem and no other code in _glib/ is calling either of those, so it seems that this particular class of problem is limited to the gspawn code.
Comment 3 Paolo Borelli 2012-02-07 19:56:10 UTC
Review of attachment 207014 [details] [review]:

Patch is obviously correct: py docs say that _ITEM can return null so we must be robust, besides we even use xdecref already.

I am not sure why this happens with os.environ and not with a dict though... Oh well :-)
Comment 4 Allison Karlitskaya (desrt) 2012-02-07 19:59:13 UTC
Attachment 207014 [details] pushed as 945fd18 - pygspawn: improve error checking
Comment 5 Dieter Verfaillie 2012-02-07 20:24:01 UTC
(In reply to comment #3)
> I am not sure why this happens with os.environ and not with a dict though... Oh
> well :-)

os.environ is a os._Environ instance which is a UserDict.UserDict
subclass. UserDict looks like but is not a dict and doesn't really
offer much value on modern Python 2 versions, it is kept for
backwards compatibility with pre Python 2.2 (iirc) code where builtin
types couldn't be subclassed. At long last, UserDict was removed in
Python 3.