Skip to content

gh-150641: Fix evaluating forward references in STRING format can 'leak' internal names in typing#150648

Merged
JelleZijlstra merged 4 commits into
python:mainfrom
ByteFlowing1337:fix-150641
Jul 3, 2026
Merged

gh-150641: Fix evaluating forward references in STRING format can 'leak' internal names in typing#150648
JelleZijlstra merged 4 commits into
python:mainfrom
ByteFlowing1337:fix-150641

Conversation

@ByteFlowing1337

@ByteFlowing1337 ByteFlowing1337 commented May 31, 2026

Copy link
Copy Markdown
Contributor

@ByteFlowing1337 ByteFlowing1337 changed the title Fix evaluating forward references in STRING format can 'leak' internal names in typing gh-150641: Fix evaluating forward references in STRING format can 'leak' internal names in typing May 31, 2026
@AlexWaygood AlexWaygood removed their request for review May 31, 2026 09:22
Comment thread Lib/typing.py Outdated
@JelleZijlstra JelleZijlstra self-requested a review May 31, 2026 19:40
@cossor

cossor commented Jun 1, 2026

Copy link
Copy Markdown

Kudos for addressing this issue so quickly!

When ForwardRef.__extra_names__ is None, __resolved_str__ is identical to __forward_arg__, which should be a string containing the code that was evaluated to produce the ForwardRef. In short, adding __resolved_str__ did not eliminate the __forward_arg__ leak. That interface is still broken.

Renaming the new property might help, but I would not be surprised if additional effort is needed.

(I can imagine a world where evaluate_forward_ref is deprecated and ForwardRef.evaluate recursively evaluates nested forward references as needed.)

@ByteFlowing1337

Copy link
Copy Markdown
Contributor Author

In short, adding __resolved_str__ did not eliminate the __forward_arg__ leak. That interface is still broken.

I think the key is not eliminating the __forward_arg__ leak, but preventing the leak of internal names like __annotationlib_name_1__.

When ForwardRef.__extra_names__ is None, I believe directly returning __forward_arg__ would not leak the internal names.

Besides, this raises a question for me: should we add __resolved_arg__ documentation, since __forward_arg__ is documented?

@cossor

cossor commented Jun 1, 2026

Copy link
Copy Markdown

I think the key is not eliminating the __forward_arg__ leak, but preventing the leak of internal names like __annotationlib_name_1__.

I think we agree. When I wrote "the __forward_arg__ leak" I should have written "leaking internal names via __forward_arg__."

The description of __forward_arg__ in the documentation is a de facto contract. Its value should be appropriate when evaluating a forward reference with format=Format.STRING, and probably is appropriate when forward references are strings. (In my code, forward references are never strings.)

Note that __forward_arg__ is supported by annotationlib.pyi whereas __resolved_str__ is not. At this point, the new property is just an implementation detail. I mentioned renaming it, because I think it tries to be what __forward_arg__ should be.

@DavidCEllis

Copy link
Copy Markdown
Contributor

I think the documentation isn't entirely accurate for .__forward_arg__ at this point, it's more like:

A string containing the code that will be evaluated when attempting to resolve the ForwardRef. This may not be exactly equivalent to the original source code.

I did view .__resolved_str__ as more of an internal detail, expecting that (outside of the class itself) it would be accessed through .evaluate(format=Format.STRING).

@cossor

cossor commented Jun 12, 2026

Copy link
Copy Markdown

My primary goal is preventing internal name exposure via Sphinx. PEP-749 suggests that __forward_arg__ is less important than evaluate_forward_ref, so I belatedly regret calling attention to this attribute and hope this pull request is approved/merged as-is.

@DavidCEllis

Copy link
Copy Markdown
Contributor

Yes, I think any change in documentation can be a separate issue and PR.

typing probably already uses enough of ForwardRef internals that it doesn't matter that it uses .__resolved_str__ as a shortcut, but in general the public way to get this should be through .evaluate(format=Format.STRING) unless we do decide to guarantee and document .__resolved_str__.

Otherwise this looks fine to me.

@ByteFlowing1337

Copy link
Copy Markdown
Contributor Author

@JelleZijlstra Do you mind reviewing this PR again when you have a chance?

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, a few small things.

Comment thread Lib/test/test_typing.py Outdated
Comment thread Lib/test/test_typing.py Outdated
Comment thread Misc/NEWS.d/next/Library/2026-05-31-14-39-25.gh-issue-150641.LLIhd1.rst Outdated
@JelleZijlstra JelleZijlstra added needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jun 30, 2026
ByteFlowing1337 and others added 2 commits June 30, 2026 18:31
…LIhd1.rst

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra JelleZijlstra merged commit f75028f into python:main Jul 3, 2026
55 checks passed
@miss-islington-app

Copy link
Copy Markdown

Thanks @ByteFlowing1337 for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

@miss-islington-app

Copy link
Copy Markdown

Sorry, @ByteFlowing1337 and @JelleZijlstra, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker f75028f7ceebee4cbeb46bf040834e2005d57436 3.14

@bedevere-app

bedevere-app Bot commented Jul 3, 2026

Copy link
Copy Markdown

GH-152935 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 3, 2026
@ByteFlowing1337 ByteFlowing1337 deleted the fix-150641 branch July 3, 2026 08:52
JelleZijlstra added a commit that referenced this pull request Jul 3, 2026
…can 'leak' internal names in `typing` (GH-150648) (#152935)

gh-150641: Fix evaluating forward references in STRING format can 'leak' internal names in `typing` (GH-150648)
(cherry picked from commit f75028f)

Co-authored-by: Ivy Xu <fakeshadow1337@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs backport to 3.14 bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants