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 the C extension module to harden is_namedtuple. #284

Merged
merged 5 commits into from Aug 24, 2021

Conversation

gpshead
Copy link
Contributor

@gpshead gpshead commented Jul 17, 2021

Protects against looks-a-likes such as Mocks. Also prevent dict encoding
from causing an unraised SystemError when encountering a non-Dict.
Noticed by running application tests against a CPython interpreter with C
asserts enabled (COPTS += -UNDEBUG) which crashed.

I didn't actually dive into figure out what data structure the code managed to
try to serialize that managed to get a MagicMock or StubMock instance
into encoder_listencode_obj where it wound up passing _is_namedtuple
leading to the assert-crash triggering problems this PR fixes.

Contributed-By: Gregory P. Smith [Google]

gpshead and others added 2 commits July 16, 2021 23:09
Protects against looks-a-likes such as Mocks. Also prevent dict encoding
from causing an unraised SystemError when encountering a non-Dict.
Noticed by running user tests against a CPython interpreter with C
asserts enabled (COPTS += -UNDEBUG).
Copy link
Member

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Thank you for finding this issue! The ideal solution would be done a bit differently (per review comments) and have a test to avoid future regressions

simplejson/_speedups.c Outdated Show resolved Hide resolved
simplejson/_speedups.c Outdated Show resolved Hide resolved
simplejson/_speedups.c Outdated Show resolved Hide resolved
@etrepum etrepum enabled auto-merge August 24, 2021 04:09
@etrepum etrepum merged commit 8dce5b0 into simplejson:master Aug 24, 2021
@etrepum etrepum mentioned this pull request Aug 24, 2021
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.

None yet

2 participants