[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Stefan Krah stefan-use...@bytereef.org added the comment: The trustworthy buildbots look good, so I'm closing this. -- keywords: -needs review, patch resolution: accepted - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Changes by R. David Murray rdmur...@bitdance.com: Removed file: http://bugs.python.org/file18016/issue9265.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
R. David Murray rdmur...@bitdance.com added the comment: Stefan: patch looks good to me in principle, but it looks like you made it against 2.7, and we commit to py3k first now. The test does not work correctly on py3k because of a bytes/string issue, but I'm not clear on why the problem happens (I haven't looked in to it, I just observe that the test fails). And yes I think this should go into 2.6 and 3.1 as well. Narnie: if only we had enough manpower to handle all bugs as rapidly. Please keep learning and help us where you can :) As for your test...I'm not clear if you are saying Stefan's patch fixes or problem or not, but please observe this: os.execv('/bin/bash', ['/bin/sh', 'echos', 'foo']) /bin/sh: echos: No such file or directory That is, it is /bin/bash that is running, but it thinks its name is '/bin/sh' because that is what we passed in arg0 in the execv call. Similarly you could do: os.execv('/bin/bash', ['my_funny_walk', 'echos', 'foo']) my_funny_walk: echos: No such file or directory So, popen is *calling* the correct shell, it's just that it is giving it the wrong arg[0] name, which Stefan's patch fixes. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Stefan Krah stefan-use...@bytereef.org added the comment: David, thanks for all the comments! - The string/bytes issue was caused by the fact that p.stdout is only opened in text mode when universal_newlines=True. Committed fix in r82971, r82972, r82973 and r82974. I'll close this issue if all buildbots are ok. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Narnie Harshoe signupli...@gmail.com added the comment: David, working as fast at learning Python as I can and loving it! Stefan, thanks for the patches. Thanks guys, Narnie -- versions: -Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Narnie Harshoe signupli...@gmail.com added the comment: Hello, I am impressed by the activity here. I am a new programmer and cutting my teeth with python. I wish I knew how to do a patch and write unit tests, but I'm just not there yet. I can show a test case in this fashion where it fails. First I prove that /bin/sh is /bin/sh (er, dash) and /bin/bash is /bin/bash. Then it can be seen that when executable is set as /bin/bash and there is a shell error, that it returns a shell error from /bin/sh, not from /bin/bash. I see there are patches here that should fix it, but for those wanting to see the buggy code in action, here it is using the fact that echo was mistyped as echos to raise a shell error (but shell=True so it doesn't raise an OSError exception). _ in shell terminal: $ file /bin/bash /bin/bash: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.15, stripped $ file /bin/sh /bin/sh: symbolic link to `dash' $ file /bin/dash /bin/dash: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.15, stripped in python interactive interpreter shell = Popen('echos', shell=True, executable=/bin/bash, stdin=PIPE, stderr=PIPE, stdout=PIPE) shell.communicate() ('', '/bin/sh: echos: command not found\n') shell = Popen('echos', shell=True, executable=/bin/bash, stdin=PIPE, stderr=PIPE, stdout=PIPE) shell.communicate() ('', '/bin/bash: echos: command not found\n') Thank you for working on this so quickly. This is truly an amazing community. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Narnie Harshoe signupli...@gmail.com added the comment: I'm sorry, but the last lines in the interpreter reflect my code changes which seemed to not be sufficient, but it shows that just for this case at least, it corrects it and allows it to be run in /bin/bash shells (much of my shell code is bash since I'm linux only and this is usually available and I make use of c-like math or incrementors like $(( x++ )) in the shell code (I know, I know, I should make my code more portable such that it can run on /bin/sh, but I'm lazy). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Stefan Krah stefan-use...@bytereef.org added the comment: Ok, here's a more comprehensive test. Comments: 1) Instead of emulating 'which' one could use find_executable from distutils.spawn. But this feels wrong. 2) Skip the test if the primary issue cannot be tested. 3) Exercise the test for /bin/sh, if it isn't a symlink. Then this part will be executed on some systems, e.g. FreeBSD. 4) The check os.path.isfile('/bin/sh') isn't necessary, but good practice nonetheless. Should this go into 2.6 and 3.1 as well? -- Added file: http://bugs.python.org/file18027/issue9265-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
R. David Murray rdmur...@bitdance.com added the comment: Stephan: that might be a cleaner API, rather than overloading the semantics of 'executable'. But that would be a separate feature request. I see what you are saying about just the name. I misread Éric's message as saying he'd confirmed the bug, but I guess he's saying he confirmed that it works as documented. It does look like subprocess lacks a test for this case. -- title: Can't choose other shell in subprocess - Incorrect name passed as arg[0] when shell=True and executable specified ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Stefan Krah stefan-use...@bytereef.org added the comment: Here's a patch with a test case. Without the fix in subprocess.py, the test prints: == FAIL: test_specific_shell (__main__.POSIXProcessTestCase) -- Traceback (most recent call last): File Lib/test/test_subprocess.py, line 650, in test_specific_shell self.assertEqual(p.stdout.read().strip(), sh) AssertionError: '/bin/sh' != '/bin/bash' I think trying bash and ksh is sufficient; echo $0 works for both (for csh it does not). David, does the patch look good? Éric, I'm by no means a tracker expert, so I wonder why you set the resolution to 'accepted' at such an early stage. Does 'accepted' mean 'This is a valid report.' or does it mean 'I think the patch is correct.'? -- keywords: +needs review stage: unit test needed - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Changes by Stefan Krah stefan-use...@bytereef.org: -- keywords: +patch Added file: http://bugs.python.org/file18016/issue9265.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
R. David Murray rdmur...@bitdance.com added the comment: The test unfortunately is too fragile. There is no guarantee that those shells will exist with those paths on any given system. Maybe you could use 'which' to find the path to the alternate shell for use in the test, and skip it if you can't find said shell.[*] I don't think there's a need to test more than one alternate shell. Does this combo (executable + shell=True) have a meaning on Windows? [*] or make issue 444582 a dependency for this bug :) -- nosy: +brian.curtin ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Éric Araujo mer...@netwok.org added the comment: David: My message was ambiguous. Your second reading was correct :) Stefan: $0 works with dash too (no reason why it should not, but I still tested :). “Accepted” means “valid bug report”; to say that the patch is good people use a message or commit directly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
Stefan Krah stefan-use...@bytereef.org added the comment: Hm, /bin/sh should actually be removed from the list. It might be a symlink to csh, for example. I know this isn't an exhaustive test. If /bin/bash or /bin/ksh don't exist, the test is skipped. I thought this is good enough, since the majority of systems have one of those, so the test is guaranteed to fail on one of the buildbots should the changes to subprocess.py be removed in the future. If a system only has /bin/csh, I wouldn't know how to write the test. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9265] Incorrect name passed as arg[0] when shell=True and executable specified
R. David Murray rdmur...@bitdance.com added the comment: Ah, I misread the test the first time. Well, I'd prefer a test that always did a real test on posix, but I suppose this has to be good enough since there could be systems that Python otherwise supports that *only* have /bin/sh. Actually I suppose one could create a local symlink to /bin/sh and call through that...but I don't think this issue warrants going to any more effort, so I think the patch is fine as is. You could argue that /bin/sh is good to have in the list since it makes sure that arg[0] doesn't get changed inappropriately in the default case. I'd be more comfortable if the test generated a skip message if the only one of the shells that is found is /bin/sh, but I'm not going to insist on it. (This will be true, for example, on FreeBSD (at least on 6, which is the install I can easily check...maybe you could add /usr/local/bin/bash as one of your candidates? Most FreeBSDs will end up having that one installed, though not of course all of them). I vote we don't worry about the csh only case :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9265 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com