From e7fefbf68dc3384b835d38bd8897657d7289f826 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 3 Apr 2002 01:47:00 +0000 Subject: Fix bugs: 457466: popenx() argument mangling hangs python 226766: popen('python -c"...."') tends to hang Fixes argument quoting in w9xpopen.exe for Windows 9x. w9xpopen.exe also never attempts to display a MessageBox when not executed interactively. Added test_popen() test. This test currently just executes "python -c ..." as a child process, and checks that the expected arguments were all recieved correctly by the child process. This test succeeds for me on Win9x, win2k and Linux, and I hope it does for other popen supported platforms too :) --- Lib/test/output/test_popen | 3 +++ Lib/test/test_popen.py | 36 +++++++++++++++++++++++++++++ Modules/posixmodule.c | 8 ++++++- PC/w9xpopen.c | 56 ++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 Lib/test/output/test_popen create mode 100644 Lib/test/test_popen.py diff --git a/Lib/test/output/test_popen b/Lib/test/output/test_popen new file mode 100644 index 0000000..db2ac06 --- /dev/null +++ b/Lib/test/output/test_popen @@ -0,0 +1,3 @@ +test_popen +Test popen: +popen seemed to process the command-line correctly diff --git a/Lib/test/test_popen.py b/Lib/test/test_popen.py new file mode 100644 index 0000000..2681eb0 --- /dev/null +++ b/Lib/test/test_popen.py @@ -0,0 +1,36 @@ +#! /usr/bin/env python +"""Basic tests for os.popen() + + Particularly useful for platforms that fake popen. +""" + +import os +import sys +from test_support import TestSkipped +from os import popen + +# Test that command-lines get down as we expect. +# To do this we execute: +# python -c "import sys;print sys.argv" {rest_of_commandline} +# This results in Python being spawned and printing the sys.argv list. +# We can then eval() the result of this, and see what each argv was. +def _do_test_commandline(cmdline, expected): + cmd = 'python -c "import sys;print sys.argv" %s' % (cmdline,) + data = popen(cmd).read() + got = eval(data)[1:] # strip off argv[0] + if got != expected: + print "Error in popen commandline handling." + print " executed '%s', expected '%r', but got '%r'" \ + % (cmdline, expected, got) + +def _test_commandline(): + _do_test_commandline("foo bar", ["foo", "bar"]) + _do_test_commandline('foo "spam and eggs" "silly walk"', ["foo", "spam and eggs", "silly walk"]) + _do_test_commandline('foo "a \\"quoted\\" arg" bar', ["foo", 'a "quoted" arg', "bar"]) + print "popen seemed to process the command-line correctly" + +def main(): + print "Test popen:" + _test_commandline() + +main() diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 46e145f..5b80479 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3285,9 +3285,15 @@ _PyPopenCreateProcess(char *cmdstring, s2 = (char *)_alloca(x); ZeroMemory(s2, x); + /* To maintain correct argument passing semantics, + we pass the command-line as it stands, and allow + quoting to be applied. w9xpopen.exe will then + use its argv vector, and re-quote the necessary + args for the ultimate child process. + */ PyOS_snprintf( s2, x, - "%s \"%s%s%s\"", + "\"%s\" %s%s%s", modulepath, s1, s3, diff --git a/PC/w9xpopen.c b/PC/w9xpopen.c index d96d0f5..31448fd 100644 --- a/PC/w9xpopen.c +++ b/PC/w9xpopen.c @@ -16,9 +16,10 @@ #define WINDOWS_LEAN_AND_MEAN #include +#include const char *usage = -"This program is used by Python's os.pipe function to\n" +"This program is used by Python's os.popen function to\n" "to work around a limitation in Windows 95/98. It is\n" "not designed to be used as stand-alone program."; @@ -28,11 +29,56 @@ int main(int argc, char *argv[]) STARTUPINFO si; PROCESS_INFORMATION pi; DWORD exit_code=0; + int cmdlen = 0; + int i; + char *cmdline, *cmdlinefill; - if (argc != 2) { - MessageBox(NULL, usage, argv[0], MB_OK); + if (argc < 2) { + if (GetFileType(GetStdHandle(STD_INPUT_HANDLE))==FILE_TYPE_CHAR) + /* Attached to a console, and therefore not executed by Python + Display a message box for the inquisitive user + */ + MessageBox(NULL, usage, argv[0], MB_OK); + else { + /* Eeek - executed by Python, but args are screwed! + Write an error message to stdout so there is at + least some clue for the end user when it appears + in their output. + A message box would be hidden and blocks the app. + */ + fprintf(stdout, "Internal popen error - no args specified\n%s\n", usage); + } return 1; } + /* Build up the command-line from the args. + Args with a space are quoted, existing quotes are escaped. + To keep things simple calculating the buffer size, we assume + every character is a quote - ie, we allocate double what we need + in the worst case. As this is only double the command line passed + to us, there is a good chance this is reasonably small, so the total + allocation will almost always be < 512 bytes. + */ + for (i=1;i