classification
Title: platform.py: _syscmd_file() can't handle target path with space or special shell character
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.0, Python 2.6, Python 2.5
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: lemburg Nosy List: jfdp, lemburg, ocean-city
Priority: normal Keywords: patch

Created on 2008-08-29 00:40 by jfdp, last changed 2008-09-02 15:50 by jfdp. This issue is now closed.

Files
File name Uploaded Description Edit
platform.patch ocean-city, 2008-09-02 10:15
Messages (11)
msg72118 - (view) Author: (jfdp) Date: 2008-08-29 00:40
If you install python in a location which has a space
or shell character in the path then platform.platform()
will generate an error and/or fail to get all platform
information.

For example

$ pwd
/disk0/tmp/foobar(2)/python2.4/bin
$ ./python
Python 2.4.4 (#8, Apr 11 2008, 11:42:39) [C] on sunos5
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform
>>> print platform.platform()
sh: syntax error at line 1: `(' unexpected
SunOS-5.10-sun4u-sparc-32bit

Note the error from 'sh' was well as the fact that "ELF"
is missing from the platform string. If you are in a path
with a space then it silently fails to identify "ELF".

The problem is in platform.py: _syscmd_file(target,default='')
in particular this line:

f = os.popen('file %s 2> /dev/null' % target)

This should be:

f = os.popen('file "%s" 2> /dev/null' % target)

Note the double quotes to protect the path from the shell.

I've examined the 2.5, 2.6 and 3.0 source and they all
have this same problem.
msg72225 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-09-01 11:27
Is adding the double-quotes enough to solve the problem ?
msg72321 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2008-09-02 10:15
Can you try this patch?

1. used "file -b" option to eliminate file path. Otherwide, re.split
won't work because file path will contain space like this. (I hope -b
option can be used anywhere "file" command exists)

a b/python.exe: MS-DOS executable PE  for MS Windows (console) Intel
80386 32-bit

2. used subprocess module instead of popen. subprocess is useful when
argument contains space or quote like this issue.
msg72322 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2008-09-02 10:19
[off topic]
I used cygwin to create this patch, but platform.architecture()[1]
becomes empty string because sys.executable in cygwin points to
directory. (at least before installing)

Python 2.6b3+ (trunk:66142M, Sep  2 2008, 17:09:46)
[GCC 3.4.4 (cygming special, gdc 0.12, using dmd 0.125)] on cygwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.executable
'/home/WhiteRabbit/python-dev/trunk/python'

I workarounded this by inserting "target = './python.exe'" in
_syscmd_file, but sorry if this patch won't work.
msg72324 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-09-02 10:31
Using subprocess is only possible conditionally, ie. if available.
Please note that the module is intended to be usable with multiple
Python versions and must at least support Python 2.1.

We could consider an updated patch for 2.7 and 3.1, but not for 2.6/3.0,
since it needs more testing.

I think for 2.6/3.0, adding the extra quotes is enough to get things
working.

Please open a new patch ticket for your patch and update it to work with
popen if subprocess is not available. The patch should also not remove
the lines:

-    if sys.platform in ('dos','win32','win16','os2'):
-        # XXX Others too ?
-        return default

from _syscmd_file().

I'll checkin the extra quotes.
msg72325 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-09-02 10:31
Updated the Python versions.
msg72327 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2008-09-02 10:55
Well, if python was installed into directory named
"foo ELF boo", python will say wrong architecture.

>>> import platform
>>> platform.architecture()
('32bit', 'ELF')

Is it not good to use "file -b" option?
msg72328 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-09-02 11:06
On 2008-09-02 12:55, Hirokazu Yamamoto wrote:
> Hirokazu Yamamoto <ocean-city@m2.ccsnet.ne.jp> added the comment:
> 
> Well, if python was installed into directory named
> "foo ELF boo", python will say wrong architecture.
> 
>>>> import platform
>>>> platform.architecture()
> ('32bit', 'ELF')
> 
> Is it not good to use "file -b" option?

Well, I'd say that install path is rather unlikely to occur in real
life ;-)

I'm not sure how wide-spread support for the -b option is, so we'd
have to check during the next release cycle.
msg72329 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2008-09-02 11:09
Checked in as r66145 and r66146.
msg72330 - (view) Author: Hirokazu Yamamoto (ocean-city) * (Python committer) Date: 2008-09-02 11:13
OK, I understand. And I must apologizes about previous
my patch.

  fileout = _architecture_split(output)[1:]

should not be deleted. It was should be changed to

  fileout = _architecture_split(output)

Thank you.
msg72347 - (view) Author: (jfdp) Date: 2008-09-02 15:50
To respond to a couple questions:

Adding the double quotes fixed the issue I had -- but I did very
limited testing.

The "-b" option is not support by 'file' on Solaris.
History
Date User Action Args
2008-09-02 15:50:21jfdpsetmessages: + msg72347
2008-09-02 11:13:25ocean-citysetmessages: + msg72330
2008-09-02 11:09:29lemburgsetstatus: open -> closed
messages: + msg72329
2008-09-02 11:06:20lemburgsetmessages: + msg72328
title: platform.py: _syscmd_file() can't handle target path with space or special shell character -> platform.py: _syscmd_file() can't handle target path with space or special shell character
2008-09-02 10:55:59ocean-citysetmessages: + msg72327
2008-09-02 10:31:42lemburgsetmessages: + msg72325
versions: + Python 2.6, Python 2.5, Python 3.0, - Python 2.4
2008-09-02 10:31:02lemburgsetmessages: + msg72324
2008-09-02 10:19:02ocean-citysetmessages: + msg72322
2008-09-02 10:15:44ocean-citysetfiles: + platform.patch
keywords: + patch
messages: + msg72321
nosy: + ocean-city
2008-09-01 11:27:06lemburgsetmessages: + msg72225
2008-09-01 02:23:30benjamin.petersonsetassignee: lemburg
nosy: + lemburg
2008-08-29 00:40:12jfdpcreate