Fix PEP 768 code injection SyntaxError when temp path contains backslashes#2039
Fix PEP 768 code injection SyntaxError when temp path contains backslashes#2039tonyroberts wants to merge 1 commit into
Conversation
…ashes On Windows, sys.remote_exec-based (PEP 768) attach-to-PID fails entirely because the temp file path (e.g. C:\Users\...\Temp\tmpXXX) is embedded raw into the injected Python code, where backslashes are misinterpreted as escape sequences. This fix correctly writes the path with the necessary escape characters. Fixes microsoft#2037
| tmp_file.write(python_code.encode()) | ||
| tmp_file.write( | ||
| """import os;os.remove("{tmp_file_path}");""".format( | ||
| """import os;os.remove({tmp_file_path!r});""".format( |
There was a problem hiding this comment.
Can you add a test that verifies remote_exec works on windows?
There was a problem hiding this comment.
I see you have a copilot PR going as well, so I'm happy for you to close this one and let you continue working in the other PR.
There was a problem hiding this comment.
Sure that sounds good. Copilot had pretty much the same change, but I'm not sure it works. I still can't get attach to work on windows even with the change so wanted to have a test.
There was a problem hiding this comment.
FWIW, I'm using the code from this PR and it 'works for me' ;)
There was a problem hiding this comment.
I get this error output with your change:
OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect: "'C:\Users\rchiodo\AppData\Local\Temp\tmp3iy0muhg'"
It put extra quotes around the path.
There was a problem hiding this comment.
Did you just copy the change? If so, you might have missed that I removed the quotes that were there originally. The code as checked in was working, but no problem if you prefer to go with another approach.
There was a problem hiding this comment.
Oh, yes i missed the quotes that were already there. It works now with your fix. I'll have the copilot one do that too.
…ashes (#2038) * Initial plan * Fix PEP 768 code injection failing with backslash in temp path When attaching using PEP 768 style code injection, the os.remove() call written to the temp file embedded the raw temp file path. On Windows, backslashes in the path (e.g. C:\Users\...\Temp\tmpXXX) were interpreted as Python escape sequences, causing a SyntaxError. Fix by replacing backslashes with forward slashes in the temp file path before embedding it in the Python code string. Windows accepts forward slashes as path separators, so this is safe on all platforms. Also add a regression test that confirms the unfixed code raises SyntaxError and the fixed code compiles correctly. Closes #2037 * Address code review: rename test function and variable for clarity * Add test verifying sys.remote_exec is called with valid injected code on Windows paths Add test_pep_768_remote_exec_called_with_backslash_path which mocks sys.remote_exec, tempfile.NamedTemporaryFile (using a Windows-style backslash path), and calls attach_to_pid() directly. It asserts that: - sys.remote_exec is called with the correct path - The bytes written to the temp file compile as valid Python - The os.remove() call uses forward slashes (no raw backslash escapes) * Remove tautological test; use exact assertion in remote_exec test * Fix POSIX regression: use repr() instead of replace for path embedding * Use {tmp_file_path!r} format per PR #2039 style --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
On Windows, sys.remote_exec-based (PEP 768) attach-to-PID fails entirely because the temp file path (e.g. C:\Users...\Temp\tmpXXX) is embedded raw into the injected Python code, where backslashes are misinterpreted as escape sequences.
This fix correctly writes the path with the necessary escape characters.
Fixes #2037