Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[venv / PC/launcher] issue with a space in the installed python path #90844

Closed
hokiedsp mannequin opened this issue Feb 8, 2022 · 7 comments
Closed

[venv / PC/launcher] issue with a space in the installed python path #90844

hokiedsp mannequin opened this issue Feb 8, 2022 · 7 comments
Labels
3.8 3.9 3.10 3.11 OS-windows type-bug

Comments

@hokiedsp
Copy link
Mannequin

@hokiedsp hokiedsp mannequin commented Feb 8, 2022

BPO 46686
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @hokiedsp

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2022-02-08.22:48:39.910>
labels = ['type-bug', '3.8', '3.9', '3.10', '3.11', 'OS-windows']
title = '[venv / PC/launcher] issue with a space in the installed python path'
updated_at = <Date 2022-02-09.16:01:34.021>
user = 'https://github.com/hokiedsp'

bugs.python.org fields:

activity = <Date 2022-02-09.16:01:34.021>
actor = 'hokiedsp'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Windows']
creation = <Date 2022-02-08.22:48:39.910>
creator = 'hokiedsp'
dependencies = []
files = []
hgrepos = []
issue_num = 46686
keywords = []
message_count = 6.0
messages = ['412874', '412883', '412885', '412889', '412894', '412918']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'hokiedsp']
pr_nums = []
priority = 'critical'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue46686'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11']

@hokiedsp
Copy link
Mannequin Author

@hokiedsp hokiedsp mannequin commented Feb 8, 2022

After months of proper operation, my per-user Python install started to error out when I attempt python -m venv .venv with

"Error: Command '['C:\\Users\\kesh\\test\\.venv\\Scripts\\python.exe', '-Im', 'ensurepip', '--upgrade', '--default-pip']' returned non-zero exit status 101."

Following the StackOverflow solution, I reinstalled Python for all users and it was working OK. I recently looked into it deeper and found the root issue in the function PC/launcher.c/run_child(). The path to the "...\Python\Python310\python.exe" contains a space, and the CreateProcessW() call on Line 811 is passing the path without quoting the path, causing the process creation to fail.

I fixed my issue by using the Windows short path convention on my path env. variable, but there must be a more permanent fix possible.

Here is the link to my question and self-answering to the problem:

https://stackoverflow.com/questions/71039131/troubleshooting-the-windows-venv-error-101

@hokiedsp hokiedsp mannequin added 3.10 3.8 3.9 OS-windows type-bug labels Feb 8, 2022
@eryksun
Copy link
Contributor

@eryksun eryksun commented Feb 9, 2022

run_child() expects cmdline to be correctly quoted, and normally it is.

I can't reproduce this problem with Python 3.10.2. I created a user account with a space in the account name, logged on, and installed 3.10.2 for the current user, with the option enabled to add Python to PATH. Next I opened a command prompt in the user profile directory and created a virtual environment via python.exe -m venv .venv. Running ".venv\Scripts\python.exe -X utf8" worked. The command line used by the venv "python.exe" launcher was properly quoted and thus properly parsed in the original list of command-line arguments, sys.orig_argv.

@hokiedsp
Copy link
Mannequin Author

@hokiedsp hokiedsp mannequin commented Feb 9, 2022

@eryksun - I knew the reproducibility is the issue with this bug. On the same PC I've been having a problem with, I created another dummy account with a space in its username, and it worked flawlessly... So, it's something I've done to my account which triggered this behavior.

Is there anything that I can try and report on my machine? I'd be happy to do so

@eryksun
Copy link
Contributor

@eryksun eryksun commented Feb 9, 2022

I checked the source code in PC/launcher.c process(). It turns out that executable is not getting quoted in the venv launcher case.

CreateProcessW() tries to get around this. If the command isn't quoted, it has a loop that consumes up to a space (or tab) and checks for an existing file (not a directory). If it finds a file, it rewrites the command line to quote the path of the file.

My test happened to work. But it's simple enough to create an example that fails. For example, as an elevated admin, create a file named "C:\Program". Now the venv launcher won't be able to execute a base interpreter that's installed in "C:\Program Files":

C:\Temp>echo >C:\Program

C:\Temp>"C:\Program Files\Python310\python.exe" -m venv env
Error: Command '['C:\\Temp\\env\\Scripts\\python.exe', '-Im', 
'ensurepip', '--upgrade', '--default-pip']' returned non-zero 
exit status 101.

@eryksun
Copy link
Contributor

@eryksun eryksun commented Feb 9, 2022

The venv launcher can quote the executable path either always or only when it contains spaces. The following is a suggestion for implementing the latter.

Before allocating executable, use memchr() (include <memory.h>) to search the UTF-8 source for a space. If found, increment the character count by two. After allocating executable, add the initial quote character, and increment the pointer.

        BOOL add_quotes = FALSE;
        if (memchr(start, ' ', (size_t)len) != NULL) {
            add_quotes = TRUE;
            cch += 2;
        }

        executable = (wchar_t *)malloc(cch * sizeof(wchar_t));
        if (executable == NULL) {
            error(RC_NO_MEMORY, L"A memory allocation failed");
        }
        
        if (add_quotes) {
            *executable++ = L'\"';
        }

Later, after checking the existence via GetFileAttributesW(), add the trailing quote and null, and decrement the pointer:

        if (GetFileAttributesW(executable) == INVALID_FILE_ATTRIBUTES) {
            error(RC_NO_PYTHON, L"No Python at '%ls'", executable);
        }

        if (add_quotes) {
            size_t n = wcslen(executable);
            executable[n] = L'\"';
            executable[n + 1] = L'\0';
            executable--;
        }

@hokiedsp
Copy link
Mannequin Author

@hokiedsp hokiedsp mannequin commented Feb 9, 2022

a file named "C:\Program". Now the venv launcher won't be able to execute

This is exactly what happened on my PC, and the behavior was triggered by Microsoft Visual C++ 2015-2022 Redistributable installer. The installer left a log file "C:\Users\Kesh" with my account "C:\Users\Kesh Ikuma".

Thank you for quickly addressing the issue

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
zooba added a commit to zooba/cpython that referenced this issue Jul 16, 2022
@zooba
Copy link
Member

@zooba zooba commented Jul 16, 2022

FWIW, I went with always quoting. There's a possibility that this will impact sys.orig_argv[0] for apps, but they need to be protected against that anyway, and Windows has a few code paths that will forcibly quote paths even when it's not necessary.

zooba added a commit that referenced this issue Jul 16, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 16, 2022
…hey have spaces in the path (pythonGH-94903)

(cherry picked from commit 4b4439d)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 16, 2022
…hey have spaces in the path (pythonGH-94903)

(cherry picked from commit 4b4439d)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington added a commit that referenced this issue Jul 16, 2022
…ve spaces in the path (GH-94903)

(cherry picked from commit 4b4439d)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington added a commit that referenced this issue Jul 16, 2022
…ve spaces in the path (GH-94903)

(cherry picked from commit 4b4439d)

Co-authored-by: Steve Dower <steve.dower@python.org>
@zooba zooba closed this as completed Jul 17, 2022
iritkatriel pushed a commit to iritkatriel/cpython that referenced this issue Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 3.9 3.10 3.11 OS-windows type-bug
Projects
None yet
Development

No branches or pull requests

2 participants