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

fix(pymodule): improve safety of PyModule::from_code #4777

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sxlijin
Copy link

@sxlijin sxlijin commented Dec 6, 2024

If PyImport_ExecCodeModuleEx is called with an empty filename or module name, references to any Python variables defined in this context may break assumptions in standard library code.

Notably, if inspect.stack() is called while any stack frame holds a reference to a variable declared in this Python snippet, and file_name is empty, then inspect.stack() will throw while trying to resolve the file in which said variable was defined.

The exec builtin handles this by defaulting file_name to <string> and module_name to <module> - these are not the most obvious defaults, but in the spirit of consistency and providing pyo3 users with a safe API, it makes sense for PyModule::from_code to do the same.

Fixes #4769


Thank you for contributing to PyO3!

By submitting these contributions you agree for them to be dual-licensed under PyO3's MIT OR Apache-2.0 license.

Please consider adding the following to your pull request:

  • an entry for this PR in newsfragments - see [https://pyo3.rs/main/contributing.html#documenting-changes]
    • or start the PR title with docs: if this is a docs-only change to skip the check
  • docs to all new functions and / or detail in the guide
  • tests for all new or changed functions

PyO3's CI pipeline will check your pull request, thus make sure you have checked the Contributing.md guidelines. To run most of its tests
locally, you can run nox. See nox --list-sessions
for a list of supported actions.

If `PyImport_ExecCodeModuleEx` is called with an empty filename or
module name, references to any Python variables defined in this
context may break assumptions in standard library code.

Notably, if `inspect.stack()` is called while any stack frame holds
a reference to a variable declared in this Python snippet, and
`file_name` is empty, then `inspect.stack()` will throw while trying
to resolve the file in which said variable was defined.

The `exec` builtin handles this by defaulting `file_name` to `<string>`
and `module_name` to `<module>` - these are not the most obvious
defaults, but in the spirit of consistency and providing pyo3 users with
a safe API, it makes sense for `PyModule::from_code` to do the same.
@sxlijin
Copy link
Author

sxlijin commented Dec 6, 2024

Still need to add tests, but I can't build pyo3 on my machine right now (something about ld not being able to resolve py3.10, even though I have it installed on my machine... - will figure this out later)

Copy link
Member

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

Thanks!

I now realize that we have quite a few code examples that do something like

PyModule::from_code(py, CODE, c_str!(""), c_str!(""))?;

Should we change those to take non-"" input? We should probably set a good example.

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.

Can PyModule::from_code enforce a non-empty filename?
2 participants