[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Note: Other platform functions still use os.popen(), you might open new issues. _syscmd_ver: for cmd in ('ver', 'command /c ver', 'cmd /c ver'): try: pipe = os.popen(cmd) ... def _syscmd_uname(option, default=''): ... f

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Oh right, I didn't read the history of the issue. _syscmd_file() doesn't use popen() anymore, but subprocess, so the issue can now be closed. -- resolution: -> fixed status: open -> closed ___ Python tracker

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Marc-Andre Lemburg added the comment: Shouldn't this ticket be closed ? platform is using subprocess on both the 2.7 and trunk, so the issue should be fixed - and indeed I cannot reproduce it anymore. -- ___ Python tracker

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Christian: would you mind to review my old patch (and rebase it if needed)? -- ___ Python tracker ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Christian Heimes added the comment: I don't see why backwards compatibility with ancient, unsupported Python versions should be more important than security. Let's use the subprocess module here. MAL, if you need platform.py for some old projects, please vendor it and use a copy. --

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Marc-Andre Lemburg added the comment: STINNER Victor wrote: STINNER Victor added the comment: The main reason for keeping the compatibility is that the module is also being used outside the stdlib for Python versions starting from 2.4 and later. I don't want to maintain two separate

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: The main reason for keeping the compatibility is that the module is also being used outside the stdlib for Python versions starting from 2.4 and later. I don't want to maintain two separate versions. Which projects use their own copy of platform.py? Where

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Marc-Andre Lemburg added the comment: Jesús Cea Avión wrote: Antoine, I agree. I beg your pardon. This patch was suppose to be quite trivial, and test_platform passes just fine on my Linux and Solaris computers. And the four buildbots I was monitoring, the testsuite passed. The

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Marc-Andre Lemburg added the comment: Jesús Cea Avión wrote: Jesús Cea Avión added the comment: Thanks for the heads-up, Victor. I have added Marc-Andre Lemburg to the nosy list, so he can know about this issue and can provide feedback (or request a backout for 2.7). Marc-Andre?.

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Marc-Andre Lemburg added the comment: The implementation of platform.architecture shells out to the file command. It tries to escape quotes by replacing with \, but that's not sufficient. $ python3.2 -c 'import platform; platform.architecture(foo\\\; echo Hi there /tmp/Z; echo \\\)'

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: 1.7 -with open(DEV_NULL) as dev_null: 1.8 -proc = subprocess.Popen(['file', '-b', '--', target], 1.9 -stdout=subprocess.PIPE, stderr=dev_null) 1.9 +proc = subprocess.Popen(['file',

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: You can use subprocess in platform for Python 2.7, but only if it's available. Otherwise the module must fall back to the portable popen() that comes with the platform module. Marc-Andre: I still don't understand why you want to run platform.py of Python 2.7

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Changes by STINNER Victor victor.stin...@gmail.com: -- resolution: fixed - status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Antoine Pitrou added the comment: BTW: It's probably better to discuss such patches on the tracker first, before applying them to the code base. It becomes difficult discussing patches that have already been partially applied to the code. Agreed with Marc-André. Jesus, please understand that

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: Antoine, I agree. I beg your pardon. This patch was suppose to be quite trivial, and test_platform passes just fine on my Linux and Solaris computers. And the four buildbots I was monitoring, the testsuite passed. The suggestion of Marc-Andre of simply

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: This is security risk, but I can't think about any platform.architecture() use that would use a untrusted parameter. Nevertheless os.popen() has been deprecated for ages. I take care of this. -- assignee: - jcea nosy: +jcea versions: -Python 2.6,

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Roundup Robot added the comment: New changeset c73b90b6dadd by Jesus Cea in branch '2.7': Closes #16112: platform.architecture does not correctly escape argument to /usr/bin/file http://hg.python.org/cpython/rev/c73b90b6dadd New changeset 6c830b657900 by Jesus Cea in branch '3.2': Closes

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: I have a bootstrap problem with python 2.7. Backout that patch, for now. -- resolution: fixed - status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Roundup Robot added the comment: New changeset d6d908dc11f2 by Jesus Cea in branch '2.7': Closes #16112: platform.architecture does not correctly escape argument to /usr/bin/file. Solve a 2.7 bootstrap issue http://hg.python.org/cpython/rev/d6d908dc11f2 -- resolution: - fixed status:

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: Thanks for bringing this up and for your patch. -- resolution: fixed - status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: I did a similar commit two years ago, but I reverted it because I failed to find a solution for the bootstrap issue. changeset: 60673:7c90ac194e40 branch: legacy-trunk parent: 60664:d7d5c76545fb user:Victor Stinner

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Changes by Jesús Cea Avión j...@jcea.es: -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___ ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: See also issue #9560 and the following email from Marc-Andre Lemburg. http://mail.python.org/pipermail/python-checkins/2010-April/092099.html Viktor, before making such changes to platform.py, please coordinate with me, as I am the maintainer of that module.

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: Thanks for the heads-up, Victor. I have added Marc-Andre Lemburg to the nosy list, so he can know about this issue and can provide feedback (or request a backout for 2.7). Marc-Andre?. -- nosy: +lemburg ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Antoine Pitrou added the comment: This change broke test_platform for me: == ERROR: test_architecture (test.test_platform.PlatformTest) -- Traceback (most

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Reopen: test_platform is failing. Attached patch should fix the issue. It fixes a theoric deadlock issue in _syscmd_file: use proc.communicate() instead of proc.stdout.read(). -- resolution: fixed - Added file:

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

David Benjamin added the comment: Well, the theoretical deadlock's just if stdin is also a pipe, right? I think there isn't be a difference between communicate and stdout.read if only stdout is a pipe. Though it might be worth passing DEVNULL to stdin instead of inheriting, just to be tidy.

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Well, the theoretical deadlock's just if stdin is also a pipe, right? If I remember correctly, the parent process (Python) and the child process (file) can be blocked if the child is blocked in a blocking write into a pipe (ex: stderr), whereas the pipe buffer

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Antoine Pitrou added the comment: Victor: your patch doesn't apply on 3.2, but it works for me on 3.3. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Antoine Pitrou added the comment: Victor is right, communicate() is usually the right way to do it. No need to use unsafe idioms. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

STINNER Victor added the comment: Victor: your patch doesn't apply on 3.2, but it works for me on 3.3. I wrote it for Python 3.4. I'm too lazy for adapt it to other branches. -- ___ Python tracker rep...@bugs.python.org

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

David Benjamin added the comment: Yes, communicate is needed if you have multiple pipes and need to be careful about both ends doing a blocking reads/writes on different ones. There's only one pipe here. Eh, whatever. If you guys really want to use communicate, I don't really care.

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Roundup Robot added the comment: New changeset b94a9ff13199 by Jesus Cea in branch '2.7': #16112: platform.architecture does not correctly escape argument to /usr/bin/file. Use 'communicate()' http://hg.python.org/cpython/rev/b94a9ff13199 -- ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Roundup Robot added the comment: New changeset 04f39958aea9 by Jesus Cea in branch '3.2': #16112: platform.architecture does not correctly escape argument to /usr/bin/file. Use 'communicate()' and decode the bytes http://hg.python.org/cpython/rev/04f39958aea9 New changeset 19d37c8d1882 by

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Roundup Robot added the comment: New changeset 9838ae397a19 by Jesus Cea in branch '3.2': #16112: platform.architecture does not correctly escape argument to /usr/bin/file. Fix original patch http://hg.python.org/cpython/rev/9838ae397a19 New changeset 64a0caf49429 by Jesus Cea in branch '3.3':

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: David, after so many patches, can you confirm that we have solved the original issue? :) -- resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

Jesús Cea Avión added the comment: Why I was not seeing test_platform.py failing in the buildbots? :-? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue16112 ___

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

New submission from David Benjamin: The implementation of platform.architecture shells out to the file command. It tries to escape quotes by replacing with \, but that's not sufficient. $ python3.2 -c 'import platform; platform.architecture(foo\\\; echo Hi there /tmp/Z; echo \\\)' cat