Skip to content

gh-133886: Fix sys.remote_exec() for non-UTF-8 paths #133887

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

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented May 11, 2025

It now supports non-ASCII paths in non-UTF-8 locales and non-UTF-8 paths in UTF-8 locales.

It now supports non-ASCII paths in non-UTF-8 locales and
non-UTF-8 paths in UTF-8 locales.
@serhiy-storchaka serhiy-storchaka force-pushed the sys-remote_exec-non-utf8 branch from 45e4604 to bd30460 Compare May 12, 2025 08:33
run_remote_debugger_script(path);
PyObject *path_obj = PyUnicode_DecodeFSDefault(path);
if (path_obj == NULL) {
PyErr_FormatUnraisable("Can't decode debugger script");
Copy link
Member

Choose a reason for hiding this comment

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

Can we include the script path in this exception ?

Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

Left a suggestion

'undecodable paths are not supported on macOS')
def test_remote_exec_undecodable(self):
script = 'print("Remote script executed successfully!")'
script_path = os_helper.TESTFN_UNDECODABLE + b'_undecodable_remote.py'
Copy link
Member

Choose a reason for hiding this comment

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

Oh nice! I didn't know about this

}
Py_DECREF(path);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can have a goto label for the error at this stage. I don't mind either way so up to you

@serhiy-storchaka serhiy-storchaka merged commit c09cec5 into python:main May 13, 2025
39 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 13, 2025
…133887)

It now supports non-ASCII paths in non-UTF-8 locales and
non-UTF-8 paths in UTF-8 locales.
(cherry picked from commit c09cec5)

Co-authored-by: Serhiy Storchaka <[email protected]>
@serhiy-storchaka serhiy-storchaka deleted the sys-remote_exec-non-utf8 branch May 13, 2025 08:55
@bedevere-app
Copy link

bedevere-app bot commented May 13, 2025

GH-133963 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 13, 2025
@serhiy-storchaka
Copy link
Member Author

Thank you for your review, @pablogsal.

@serhiy-storchaka
Copy link
Member Author

We could also use natural UTF-16 for paths on Windows, but this is more significant change.

serhiy-storchaka added a commit that referenced this pull request May 13, 2025
… (GH-133963)

It now supports non-ASCII paths in non-UTF-8 locales and
non-UTF-8 paths in UTF-8 locales.
(cherry picked from commit c09cec5)

Co-authored-by: Serhiy Storchaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants