Skip to content

gh-99184: Bypass instance attribute access in repr of weakref.ref #99244

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
Apr 24, 2023
Merged

gh-99184: Bypass instance attribute access in repr of weakref.ref #99244

merged 2 commits into from
Apr 24, 2023

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 8, 2022 •
edited by bedevere-bot
Loading

Consider this as a fix proposal :)

Thanks a lot to @MojoVampire for the idea.

@bedevere-bot bedevere-bot mentioned this pull request Nov 8, 2022
Closed
sobolevn
sobolevn commented Nov 8, 2022
View reviewed changes
Objects/weakrefobject.c
@@ -170,10 +170,7 @@ weakref_repr(PyWeakReference *self)
}

Py_INCREF(obj);
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we still need it? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, without it:

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00000001039365c0 (most recent call first):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_weakref.py", line 116 in test_basic_ref
  File "/Users/sobolev/Desktop/cpython/Lib/unittest/case.py", line 579 in _callTestMethod

@yanboliang yanboliang mentioned this pull request Nov 11, 2022
Closed
@sobolevn
Copy link
Member Author

You were suggested reviewers :)

@sobolevn
Copy link
Member Author

sobolevn commented Mar 27, 2023 •
edited by AlexWaygood
Loading

Note for reviewers about dynamic __weakref__. One might think that this PR can break existing dynamic __weakref__ assignment. For example:

>>> import weakref
>>> class A: ...
... 
>>> a = A()
>>> w = weakref.ref(a)
>>> a1 = A()
>>> a1.__weakref__
>>> a1.__weakref__ = w
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: attribute '__weakref__' of 'A' objects is not writable

But, as you can see it is not writable. So, it should not affect anything really.
I am going to update the branch and trigget all buildbots, just to be sure.

@sobolevn sobolevn added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Mar 27, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit d0aad14 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit d0aad14 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Mar 27, 2023
@sobolevn sobolevn added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 27, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 5ca1917 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Mar 27, 2023
carljm
carljm approved these changes Mar 29, 2023
View reviewed changes
Copy link
Member

@carljm carljm left a comment

Choose a reason for hiding this comment

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

LGTM

@carljm carljm merged commit 58b6be3 into python:main Apr 24, 2023
@carljm carljm added the needs backport to 3.11 only security fixes label Apr 24, 2023
@miss-islington
Copy link
Contributor

Thanks @sobolevn for the PR, and @carljm for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 24, 2023
@sobolevn @miss-islington
pythongh-99184: Bypass instance attribute access in repr of `weakre…
29adef0 
…f.ref` (pythonGH-99244)

(cherry picked from commit 58b6be3)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-bot
Copy link

GH-103789 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 Apr 24, 2023
carljm pushed a commit that referenced this pull request Apr 24, 2023
@miss-islington @sobolevn
[3.11] gh-99184: Bypass instance attribute access in repr of `weakr…
c8de883 
…ef.ref` (GH-99244) (#103789)

gh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (GH-99244)
(cherry picked from commit 58b6be3)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
carljm added a commit to carljm/cpython that referenced this pull request Apr 24, 2023
@carljm
Merge branch 'main' into superopt
19b8025 
* main: (53 commits)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  Add tests for empty range equality (python#103751)
  pythongh-103712: Increase the length of the type name in AttributeError messages (python#103713)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Apr 24, 2023
@carljm
Merge branch 'superopt' into superopt_spec
1972ca4 
* superopt: (82 commits)
  pythongh-101517: fix line number propagation in code generated for except* (python#103550)
  pythongh-103780: Use patch instead of mock in asyncio unix events test (python#103782)
  pythongh-102498 Clean up unused variables and imports in the email module  (python#102482)
  pythongh-99184: Bypass instance attribute access in `repr` of `weakref.ref` (python#99244)
  pythongh-99032: datetime docs: Encoding is no longer relevant (python#93365)
  pythongh-94300: Update datetime.strptime documentation (python#95318)
  pythongh-103776: Remove explicit uses of $(SHELL) from Makefile (pythonGH-103778)
  pythongh-87092: fix a few cases of incorrect error handling in compiler (python#103456)
  pythonGH-103727: Avoid advancing tokenizer too far in f-string mode (pythonGH-103775)
  Revert "Add tests for empty range equality (python#103751)" (python#103770)
  pythongh-94518: Port 23-argument `_posixsubprocess.fork_exec` to Argument Clinic (python#94519)
  pythonGH-65022: Fix description of copyreg.pickle function (python#102656)
  pythongh-103323: Get the "Current" Thread State from a Thread-Local Variable (pythongh-103324)
  pythongh-91687: modernize dataclass example typing (python#103773)
  pythongh-103746: Test `types.UnionType` and `Literal` types together (python#103747)
  pythongh-103765: Fix 'Warning: py:class reference target not found: ModuleSpec' (pythonGH-103769)
  pythongh-87452: Improve the Popen.returncode docs
  Removed unnecessary escaping of asterisks (python#103714)
  pythonGH-102973: Slim down Fedora packages in the dev container (python#103283)
  pythongh-103091: Add PyUnstable_Type_AssignVersionTag (python#103095)
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@carljm carljm carljm approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently

@iritkatriel iritkatriel Awaiting requested review from iritkatriel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sobolevn @bedevere-bot @miss-islington @carljm