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

gh-99442: Fix handling in py.exe launcher when argv[0] does not include a file extension #99542

Merged
merged 2 commits into from
Nov 18, 2022

Conversation

zooba
Copy link
Member

@zooba zooba commented Nov 16, 2022

Comment on lines +498 to +500
// the argument have no effect. A quoted argv0 must start and end
// with a double quote character; otherwise, it ends at the first
// ' ' or '\t'.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically speaking, any part of argv0 may be quoted. E.g. C:\"path to"\"py.exe" is parsed as C:\path to\py.exe by the crt. Being quoted just prevents tab or space from terminating the argument.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the associated bug (which I just fixed, because I put the wrong ID in the title originally).

We're following CreateProcess rules here, because it's going to be passed to CreateProcess.

Copy link
Contributor

@eryksun eryksun Nov 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisDenton, The C runtime's more general parsing of argv[0] (i.e. multiple quoted parts terminated by space or tab) is only relevant if the executable path is passed explicitly to CreateProcessW() in lpApplicationName. If CreateProcessW() is called with lpApplicationName as NULL, as the launcher does, then the API parses the command to run from lpCommandLine. If the command line begins with a double quote, the API consumes all characters up to the next double quote or the end of the command line. If the command line doesn't begin with a double quote, the API tokenizes on space and tab characters, repeatedly searching until it finds a non-directory file.

For example, if the command line is r'C:\Program Files\Python311\python.exe', the API will execute "C:\Program.exe" if it exists. If the command line is r'"C:\Program Files\Python311\python.exe"spam', the API will execute "C:\Program Files\Python311\python.exe" if it exists. Note that there's an inconsistency with the C runtime's quoting rules. The CRT always splits arguments on an unquoted space or tab. For example:

>>> script = 'import sys; print(sys.orig_argv)'
>>> subprocess.call(fr'"{sys.executable}"spam -c "{script}"')
['C:\\Program Files\\Python311\\python.exespam', '-c', 'import sys; print(sys.orig_argv)']
0

The API executed "python.exe", not "python.exespam".

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that inconsistency is what worried me. As far as I understand it, no normalization is being performed in the python code so the inconsistency remains? So the CRT and CreateProcessW will be understanding the arguments differently?

@zooba zooba changed the title gh-99370: Fix handling in py.exe launcher when argv[0] does not include a file extension gh-99442: Fix handling in py.exe launcher when argv[0] does not include a file extension Nov 17, 2022
@zooba zooba merged commit a220c6d into python:main Nov 18, 2022
@miss-islington
Copy link
Contributor

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@zooba zooba deleted the gh-99442 branch November 18, 2022 14:15
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 18, 2022
… include a file extension (pythonGH-99542)

(cherry picked from commit a220c6d)

Co-authored-by: Steve Dower <[email protected]>
@bedevere-bot
Copy link

GH-99579 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Nov 18, 2022
zooba pushed a commit that referenced this pull request Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants