gh-138479: Ensure that `__typing_subst__` returns a tuple by ZeroIntensity · Pull Request #138482 · python/cpython

ZeroIntensity · 2025-09-03T21:05:01Z

Raise an exception if __typing_subst__ returns a non-tuple object.

The test case is a little messy because it's based on fuzzer-generated code, so I can come up with my own repro if people aren't happy with using it.

Issue: crash in PyTuple_GET_SIZE: Assertion `PyTuple_Check(op)' failed #138479

Oops.

sobolevn · 2025-09-03T22:04:09Z

Objects/genericaliasobject.c

            return NULL;
        }
        if (unpack) {
+            if (!PyTuple_Check(arg)) {


Do we care about tuple subclasses here? If so, do we want to test this? Or use PyTuple_CheckExact?

I think it would be fine to do either. It would technically be a breaking change to disallow tuple subclasses here, but I doubt anyone is actually doing that in practice.

It's an internal undocumented API so I wouldn't feel terrible about disallowing tuple subclasses, but we could go either way.

Let's just use PyTuple_Check for the time being. There's no drawback to keeping subclass support here, so we might as well keep it.

JelleZijlstra · 2025-09-05T20:36:00Z

Objects/genericaliasobject.c

+                             "expected a tuple, not %T", arg);
+                Py_DECREF(arg);
+                return NULL;
+            }


Can't comment there but are we leaking newargs on line 543 below?

Looks like it. Do you want me to fix that in this PR?

Sure, might as well.

serhiy-storchaka · 2025-09-06T07:53:53Z

Objects/genericaliasobject.c

            if (jarg < 0) {
                Py_DECREF(item);
                Py_XDECREF(tuple_args);
+                Py_DECREF(newargs);


It is NULL here.

Ah, tuple_extend steals the reference on failure. I added a comment here instead. Thanks for clearing this up.

Lib/test/test_typing.py

serhiy-storchaka · 2025-09-06T08:55:24Z

Objects/genericaliasobject.c

+                Py_DECREF(item);
+                Py_XDECREF(tuple_args);
+                PyErr_Format(PyExc_TypeError,
+                             "expected a tuple, not %T", arg);


This error message is not helpful. Please add what method of what type has been called (this was method __typing_subst__ of original argument PyTuple_GET_ITEM(args, iarg)).

Done, it's now expected __typing_subst__ of %T objects to return a tuple, not %T. Is that good?

Co-authored-by: Serhiy Storchaka <[email protected]>

Also fix incorrect DECREF.

serhiy-storchaka · 2025-09-07T12:44:27Z

Lib/test/test_typing.py

+
+        evil = EvilTypeVar()
+        type type_alias[*_] = 0
+        with self.assertRaises(TypeError):


Could you please test an error message? No need to test the full match, just some unique pattern to ensure that this is an expected TypeError.

vstinner

LGTM. The change does fix the crash.

Objects/genericaliasobject.c

Lib/test/test_typing.py

ZeroIntensity · 2025-09-11T01:45:47Z

@serhiy-storchaka Would you like to finish up your review? If not, I'll merge this.

serhiy-storchaka

LGTM. 👍

miss-islington-app · 2025-09-11T10:39:12Z

Thanks @ZeroIntensity for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

…onGH-138482) Raise an exception if __typing_subst__ returns a non-tuple object. --------- (cherry picked from commit 1da989b) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

miss-islington-app · 2025-09-11T10:39:20Z

Sorry, @ZeroIntensity, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 1da989be74eaa94590ec28e750e5352de1ead227 3.13

bedevere-app · 2025-09-11T10:39:25Z

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

bedevere-app · 2025-09-11T10:46:49Z

GH-138786 is a backport of this pull request to the 3.13 branch.

…onGH-138482) Raise an exception if __typing_subst__ returns a non-tuple object. --------- Co-authored-by: Serhiy Storchaka <[email protected]> (cherry picked from commit 1da989b)

pythonGH-138482) Raise an exception if __typing_subst__ returns a non-tuple object. --------- (cherry picked from commit 1da989b) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

…138482) (GH-138786) * gh-138479: Ensure that `__typing_subst__` returns a tuple (GH-138482) Raise an exception if __typing_subst__ returns a non-tuple object. (cherry picked from commit 1da989b) Co-authored-by: Serhiy Storchaka <[email protected]>

…138482) (#138784) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

ZeroIntensity added 4 commits September 3, 2025 16:48

Validate that the result is a tuple.

9458120

Add a simple test case.

c70e82e

Add blurb.

cb4351b

Fix a reference leak.

c50846b

Oops.

ZeroIntensity requested a review from serhiy-storchaka September 3, 2025 21:05

ZeroIntensity requested review from AlexWaygood and JelleZijlstra as code owners September 3, 2025 21:05

ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Sep 3, 2025

bedevere-app bot added the awaiting core review label Sep 3, 2025

bedevere-app bot mentioned this pull request Sep 3, 2025

crash in PyTuple_GET_SIZE: Assertion `PyTuple_Check(op)' failed #138479

Closed

sobolevn reviewed Sep 3, 2025

View reviewed changes

Fix use-after-decref.

04d509c

JelleZijlstra reviewed Sep 5, 2025

View reviewed changes

Fix existing refleak.

761812d

AlexWaygood removed their request for review September 6, 2025 07:41

serhiy-storchaka reviewed Sep 6, 2025

View reviewed changes

ZeroIntensity and others added 2 commits September 6, 2025 10:18

Update Lib/test/test_typing.py

46b6fa3

Co-authored-by: Serhiy Storchaka <[email protected]>

Improve error message.

4445b5a

Also fix incorrect DECREF.

serhiy-storchaka reviewed Sep 7, 2025

View reviewed changes

Use assertRaisesRegex.

d80223a

vstinner approved these changes Sep 9, 2025

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Sep 9, 2025

picnixz reviewed Sep 9, 2025

View reviewed changes

Objects/genericaliasobject.c Outdated Show resolved Hide resolved

Add an assertion instead of a comment for newargs.

b8ced40

warsaw reviewed Sep 9, 2025

View reviewed changes

Lib/test/test_typing.py Show resolved Hide resolved

Add a comment.

ff5078e

serhiy-storchaka approved these changes Sep 11, 2025

View reviewed changes

ZeroIntensity merged commit 1da989b into python:main Sep 11, 2025
45 checks passed

ZeroIntensity deleted the gh-138479-unpack-crash branch September 11, 2025 10:39

bedevere-app bot removed the awaiting merge label Sep 11, 2025

miss-islington-app bot assigned ZeroIntensity Sep 11, 2025

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Sep 11, 2025

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 11, 2025

hugovk pushed a commit that referenced this pull request Sep 12, 2025

[3.14] gh-138479: Ensure that __typing_subst__ returns a tuple (GH-…

fbfbfdf

…138482) (#138784) Co-authored-by: Peter Bierma <[email protected]> Co-authored-by: Serhiy Storchaka <[email protected]>

JanEricNitschke mentioned this pull request Sep 13, 2025

Missing issues with CPython tests JanEricNitschke/typing_extensions#1

Open

Uh oh!

Comments

Conversation

ZeroIntensity commented Sep 3, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZeroIntensity Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity commented Sep 11, 2025

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miss-islington-app bot commented Sep 11, 2025

Uh oh!

miss-islington-app bot commented Sep 11, 2025

Uh oh!

bedevere-app bot commented Sep 11, 2025

Uh oh!

bedevere-app bot commented Sep 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ZeroIntensity commented Sep 3, 2025 •

edited by bedevere-app bot

Loading

ZeroIntensity Sep 6, 2025 •

edited

Loading

vstinner left a comment •

edited

Loading