From 468c3bf79890ef614764b4e7543608876c792794 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 13 Jan 2023 11:49:01 +0000 Subject: [PATCH] gh-100247: Fix py.exe launcher not using entire shebang command for finding custom commands (GH-100944) --- Doc/using/windows.rst | 39 +++-- Lib/test/test_launcher.py | 57 +++++-- ...-01-11-14-42-11.gh-issue-100247.YfEmSz.rst | 2 + PC/launcher2.c | 150 ++++++++++-------- 4 files changed, 154 insertions(+), 94 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2023-01-11-14-42-11.gh-issue-100247.YfEmSz.rst diff --git a/Doc/using/windows.rst b/Doc/using/windows.rst index fdbe4c15a20..69bca4d7bd3 100644 --- a/Doc/using/windows.rst +++ b/Doc/using/windows.rst @@ -831,7 +831,7 @@ To allow shebang lines in Python scripts to be portable between Unix and Windows, this launcher supports a number of 'virtual' commands to specify which interpreter to use. The supported virtual commands are: -* ``/usr/bin/env python`` +* ``/usr/bin/env`` * ``/usr/bin/python`` * ``/usr/local/bin/python`` * ``python`` @@ -868,14 +868,28 @@ minor version. I.e. ``/usr/bin/python3.7-32`` will request usage of the The ``/usr/bin/env`` form of shebang line has one further special property. Before looking for installed Python interpreters, this form will search the -executable :envvar:`PATH` for a Python executable. This corresponds to the -behaviour of the Unix ``env`` program, which performs a :envvar:`PATH` search. +executable :envvar:`PATH` for a Python executable matching the name provided +as the first argument. This corresponds to the behaviour of the Unix ``env`` +program, which performs a :envvar:`PATH` search. If an executable matching the first argument after the ``env`` command cannot -be found, it will be handled as described below. Additionally, the environment -variable :envvar:`PYLAUNCHER_NO_SEARCH_PATH` may be set (to any value) to skip -this additional search. +be found, but the argument starts with ``python``, it will be handled as +described for the other virtual commands. +The environment variable :envvar:`PYLAUNCHER_NO_SEARCH_PATH` may be set +(to any value) to skip this search of :envvar:`PATH`. -Shebang lines that do not match any of these patterns are treated as **Windows** +Shebang lines that do not match any of these patterns are looked up in the +``[commands]`` section of the launcher's :ref:`.INI file `. +This may be used to handle certain commands in a way that makes sense for your +system. The name of the command must be a single argument (no spaces), +and the value substituted is the full path to the executable (no arguments +may be added). + +.. code-block:: ini + + [commands] + /bin/sh=C:\Program Files\Bash\bash.exe + +Any commands not found in the .INI file are treated as **Windows** executable paths that are absolute or relative to the directory containing the script file. This is a convenience for Windows-only scripts, such as those generated by an installer, since the behavior is not compatible with Unix-style shells. @@ -898,15 +912,16 @@ Then Python will be started with the ``-v`` option Customization ------------- +.. _launcher-ini: + Customization via INI files ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Two .ini files will be searched by the launcher - ``py.ini`` in the current -user's "application data" directory (i.e. the directory returned by calling the -Windows function ``SHGetFolderPath`` with ``CSIDL_LOCAL_APPDATA``) and ``py.ini`` in the -same directory as the launcher. The same .ini files are used for both the -'console' version of the launcher (i.e. py.exe) and for the 'windows' version -(i.e. pyw.exe). +user's application data directory (``%LOCALAPPDATA%`` or ``$env:LocalAppData``) +and ``py.ini`` in the same directory as the launcher. The same .ini files are +used for both the 'console' version of the launcher (i.e. py.exe) and for the +'windows' version (i.e. pyw.exe). Customization specified in the "application directory" will have precedence over the one next to the executable, so a user, who may not have write access to the diff --git a/Lib/test/test_launcher.py b/Lib/test/test_launcher.py index 47152d4a3c0..351a638c1dd 100644 --- a/Lib/test/test_launcher.py +++ b/Lib/test/test_launcher.py @@ -67,12 +67,17 @@ TEST_PY_ENV = dict( ) -TEST_PY_COMMANDS = "\n".join([ +TEST_PY_DEFAULTS = "\n".join([ "[defaults]", - *[f"{k[3:].lower()}={v}" for k, v in TEST_PY_ENV.items()] + *[f"{k[3:].lower()}={v}" for k, v in TEST_PY_ENV.items()], ]) +TEST_PY_COMMANDS = "\n".join([ + "[commands]", + "test-command=TEST_EXE.exe", +]) + def create_registry_data(root, data): def _create_registry_data(root, key, value): if isinstance(value, dict): @@ -429,21 +434,21 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertTrue(data["env.tag"].startswith("2."), data["env.tag"]) def test_py_default(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): data = self.run_py(["-arg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100", data["SearchInfo.tag"]) self.assertEqual("X.Y.exe -arg", data["stdout"].strip()) def test_py2_default(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): data = self.run_py(["-2", "-arg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-32", data["SearchInfo.tag"]) self.assertEqual("X.Y-32.exe -arg", data["stdout"].strip()) def test_py3_default(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): data = self.run_py(["-3", "-arg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) self.assertEqual("3.100-arm64", data["SearchInfo.tag"]) @@ -468,7 +473,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual("X.Y-arm64.exe -X fake_arg_for_test -arg", data["stdout"].strip()) def test_py_default_short_argv0(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): for argv0 in ['"py.exe"', 'py.exe', '"py"', 'py']: with self.subTest(argv0): data = self.run_py(["--version"], argv=f'{argv0} --version') @@ -518,7 +523,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertNotEqual(data2["SearchInfo.lowPriorityTag"], "True") def test_py_shebang(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -526,7 +531,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y.exe -prearg {script} -postarg", data["stdout"].strip()) def test_python_shebang(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! python -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -534,7 +539,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y.exe -prearg {script} -postarg", data["stdout"].strip()) def test_py2_shebang(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python2 -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -542,7 +547,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y-32.exe -prearg {script} -postarg", data["stdout"].strip()) def test_py3_shebang(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python3 -prearg") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -550,7 +555,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y-arm64.exe -X fake_arg_for_test -prearg {script} -postarg", data["stdout"].strip()) def test_py_shebang_nl(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -558,7 +563,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y.exe -prearg {script} -postarg", data["stdout"].strip()) def test_py2_shebang_nl(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python2 -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -566,7 +571,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y-32.exe -prearg {script} -postarg", data["stdout"].strip()) def test_py3_shebang_nl(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python3 -prearg\n") as script: data = self.run_py([script, "-postarg"]) self.assertEqual("PythonTestSuite", data["SearchInfo.company"]) @@ -574,7 +579,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): self.assertEqual(f"X.Y-arm64.exe -X fake_arg_for_test -prearg {script} -postarg", data["stdout"].strip()) def test_py_shebang_short_argv0(self): - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script("#! /usr/bin/python -prearg") as script: # Override argv to only pass "py.exe" as the command data = self.run_py([script, "-postarg"], argv=f'"py.exe" "{script}" -postarg') @@ -591,7 +596,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): def test_search_path(self): stem = Path(sys.executable).stem - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script(f"#! /usr/bin/env {stem} -prearg") as script: data = self.run_py( [script, "-postarg"], @@ -602,7 +607,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): def test_search_path_exe(self): # Leave the .exe on the name to ensure we don't add it a second time name = Path(sys.executable).name - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script(f"#! /usr/bin/env {name} -prearg") as script: data = self.run_py( [script, "-postarg"], @@ -612,7 +617,7 @@ class TestLauncher(unittest.TestCase, RunPyMixin): def test_recursive_search_path(self): stem = self.get_py_exe().stem - with self.py_ini(TEST_PY_COMMANDS): + with self.py_ini(TEST_PY_DEFAULTS): with self.script(f"#! /usr/bin/env {stem}") as script: data = self.run_py( [script], @@ -673,3 +678,21 @@ class TestLauncher(unittest.TestCase, RunPyMixin): f'"{script.parent}\\some\\ random app" -witharg {script}', data["stdout"].strip(), ) + + def test_literal_shebang_command(self): + with self.py_ini(TEST_PY_COMMANDS): + with self.script('#! test-command arg1') as script: + data = self.run_py([script]) + self.assertEqual( + f"TEST_EXE.exe arg1 {script}", + data["stdout"].strip(), + ) + + def test_literal_shebang_invalid_template(self): + with self.script('#! /usr/bin/not-python arg1') as script: + data = self.run_py([script]) + expect = script.parent / "/usr/bin/not-python" + self.assertEqual( + f"{expect} arg1 {script}", + data["stdout"].strip(), + ) diff --git a/Misc/NEWS.d/next/Windows/2023-01-11-14-42-11.gh-issue-100247.YfEmSz.rst b/Misc/NEWS.d/next/Windows/2023-01-11-14-42-11.gh-issue-100247.YfEmSz.rst new file mode 100644 index 00000000000..7bfcbd7ddec --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2023-01-11-14-42-11.gh-issue-100247.YfEmSz.rst @@ -0,0 +1,2 @@ +Restores support for the :file:`py.exe` launcher finding shebang commands in +its configuration file using the full command name. diff --git a/PC/launcher2.c b/PC/launcher2.c index 9b3db04aa48..8371c6014cd 100644 --- a/PC/launcher2.c +++ b/PC/launcher2.c @@ -16,6 +16,7 @@ #include #include #include +#include #define MS_WINDOWS #include "patchlevel.h" @@ -37,6 +38,7 @@ #define RC_INSTALLING 111 #define RC_NO_PYTHON_AT_ALL 112 #define RC_NO_SHEBANG 113 +#define RC_RECURSIVE_SHEBANG 114 static FILE * log_fp = NULL; @@ -702,16 +704,23 @@ _decodeShebang(SearchInfo *search, const char *buffer, int bufferLength, bool on bool -_shebangStartsWith(const wchar_t *buffer, int bufferLength, const wchar_t *prefix, const wchar_t **rest) +_shebangStartsWith(const wchar_t *buffer, int bufferLength, const wchar_t *prefix, const wchar_t **rest, int *firstArgumentLength) { int prefixLength = (int)wcsnlen_s(prefix, MAXLEN); - if (bufferLength < prefixLength) { + if (bufferLength < prefixLength || !_startsWithArgument(buffer, bufferLength, prefix, prefixLength)) { return false; } if (rest) { *rest = &buffer[prefixLength]; } - return _startsWithArgument(buffer, bufferLength, prefix, prefixLength); + if (firstArgumentLength) { + int i = prefixLength; + while (i < bufferLength && !isspace(buffer[i])) { + i += 1; + } + *firstArgumentLength = i - prefixLength; + } + return true; } @@ -723,26 +732,27 @@ searchPath(SearchInfo *search, const wchar_t *shebang, int shebangLength) } wchar_t *command; - if (!_shebangStartsWith(shebang, shebangLength, L"/usr/bin/env ", &command)) { + int commandLength; + if (!_shebangStartsWith(shebang, shebangLength, L"/usr/bin/env ", &command, &commandLength)) { return RC_NO_SHEBANG; } - wchar_t filename[MAXLEN]; - int lastDot = 0; - int commandLength = 0; - while (commandLength < MAXLEN && command[commandLength] && !isspace(command[commandLength])) { - if (command[commandLength] == L'.') { - lastDot = commandLength; - } - filename[commandLength] = command[commandLength]; - commandLength += 1; - } - if (!commandLength || commandLength == MAXLEN) { return RC_BAD_VIRTUAL_PATH; } - filename[commandLength] = L'\0'; + int lastDot = commandLength; + while (lastDot > 0 && command[lastDot] != L'.') { + lastDot -= 1; + } + if (!lastDot) { + lastDot = commandLength; + } + + wchar_t filename[MAXLEN]; + if (wcsncpy_s(filename, MAXLEN, command, lastDot)) { + return RC_BAD_VIRTUAL_PATH; + } const wchar_t *ext = L".exe"; // If the command already has an extension, we do not want to add it again @@ -780,7 +790,7 @@ searchPath(SearchInfo *search, const wchar_t *shebang, int shebangLength) if (GetModuleFileNameW(NULL, filename, MAXLEN) && 0 == _comparePath(filename, -1, buffer, -1)) { debug(L"# ignoring recursive shebang command\n"); - return RC_NO_SHEBANG; + return RC_RECURSIVE_SHEBANG; } wchar_t *buf = allocSearchInfoBuffer(search, n + 1); @@ -994,73 +1004,78 @@ checkShebang(SearchInfo *search) return exitCode; } - // Handle some known, case-sensitive shebang templates + // Handle some known, case-sensitive shebangs const wchar_t *command; int commandLength; + // Each template must end with "python" static const wchar_t *shebangTemplates[] = { - L"/usr/bin/env ", - L"/usr/bin/", - L"/usr/local/bin/", + L"/usr/bin/env python", + L"/usr/bin/python", + L"/usr/local/bin/python", L"python", NULL }; for (const wchar_t **tmpl = shebangTemplates; *tmpl; ++tmpl) { - if (_shebangStartsWith(shebang, shebangLength, *tmpl, &command)) { - commandLength = 0; - // Normally "python" is the start of the command, but we also need it - // as a shebang prefix for back-compat. We move the command marker back - // if we match on that one. - if (0 == wcscmp(*tmpl, L"python")) { - command -= 6; - } - while (command[commandLength] && !isspace(command[commandLength])) { - commandLength += 1; - } - if (!commandLength) { - } else if (_findCommand(search, command, commandLength)) { + // Just to make sure we don't mess this up in the future + assert(0 == wcscmp(L"python", (*tmpl) + wcslen(*tmpl) - 6)); + + if (_shebangStartsWith(shebang, shebangLength, *tmpl, &command, &commandLength)) { + // Search for "python{command}" overrides. All templates end with + // "python", so we prepend it by jumping back 6 characters + if (_findCommand(search, &command[-6], commandLength + 6)) { search->executableArgs = &command[commandLength]; search->executableArgsLength = shebangLength - commandLength; debug(L"# Treating shebang command '%.*s' as %s\n", - commandLength, command, search->executablePath); - } else if (_shebangStartsWith(command, commandLength, L"python", NULL)) { - search->tag = &command[6]; - search->tagLength = commandLength - 6; - // If we had 'python3.12.exe' then we want to strip the suffix - // off of the tag - if (search->tagLength > 4) { - const wchar_t *suffix = &search->tag[search->tagLength - 4]; - if (0 == _comparePath(suffix, 4, L".exe", -1)) { - search->tagLength -= 4; - } + commandLength + 6, &command[-6], search->executablePath); + return 0; + } + + search->tag = command; + search->tagLength = commandLength; + // If we had 'python3.12.exe' then we want to strip the suffix + // off of the tag + if (search->tagLength > 4) { + const wchar_t *suffix = &search->tag[search->tagLength - 4]; + if (0 == _comparePath(suffix, 4, L".exe", -1)) { + search->tagLength -= 4; } - // If we had 'python3_d' then we want to strip the '_d' (any - // '.exe' is already gone) - if (search->tagLength > 2) { - const wchar_t *suffix = &search->tag[search->tagLength - 2]; - if (0 == _comparePath(suffix, 2, L"_d", -1)) { - search->tagLength -= 2; - } - } - search->oldStyleTag = true; - search->executableArgs = &command[commandLength]; - search->executableArgsLength = shebangLength - commandLength; - if (search->tag && search->tagLength) { - debug(L"# Treating shebang command '%.*s' as 'py -%.*s'\n", - commandLength, command, search->tagLength, search->tag); - } else { - debug(L"# Treating shebang command '%.*s' as 'py'\n", - commandLength, command); + } + // If we had 'python3_d' then we want to strip the '_d' (any + // '.exe' is already gone) + if (search->tagLength > 2) { + const wchar_t *suffix = &search->tag[search->tagLength - 2]; + if (0 == _comparePath(suffix, 2, L"_d", -1)) { + search->tagLength -= 2; } + } + search->oldStyleTag = true; + search->executableArgs = &command[commandLength]; + search->executableArgsLength = shebangLength - commandLength; + if (search->tag && search->tagLength) { + debug(L"# Treating shebang command '%.*s' as 'py -%.*s'\n", + commandLength, command, search->tagLength, search->tag); } else { - debug(L"# Found shebang command but could not execute it: %.*s\n", + debug(L"# Treating shebang command '%.*s' as 'py'\n", commandLength, command); } - // search is done by this point return 0; } } + // Unrecognised executables are first tried as command aliases + commandLength = 0; + while (commandLength < shebangLength && !isspace(shebang[commandLength])) { + commandLength += 1; + } + if (_findCommand(search, shebang, commandLength)) { + search->executableArgs = &shebang[commandLength]; + search->executableArgsLength = shebangLength - commandLength; + debug(L"# Treating shebang command '%.*s' as %s\n", + commandLength, shebang, search->executablePath); + return 0; + } + // Unrecognised commands are joined to the script's directory and treated // as the executable path return _useShebangAsExecutable(search, shebang, shebangLength); @@ -2407,7 +2422,12 @@ performSearch(SearchInfo *search, EnvironmentInfo **envs) // Check for a shebang line in our script file // (or return quickly if no script file was specified) exitCode = checkShebang(search); - if (exitCode) { + switch (exitCode) { + case 0: + case RC_NO_SHEBANG: + case RC_RECURSIVE_SHEBANG: + break; + default: return exitCode; }