gh-153970: Validate the returncode type in CalledProcessError by fedonman · Pull Request #153971 · python/cpython

fedonman · 2026-07-18T13:46:56Z

CalledProcessError.returncode holds a process exit status, which is always an integer. Calling str() on one whose returncode was None raised TypeError, because __str__ formats the value with %d. A returncode of another type, such as a string, failed even earlier at the self.returncode < 0 comparison.

The constructor now raises a clear TypeError when returncode is neither an integer nor None, so __str__ can never be handed a value it cannot format. Its else branch also uses an f-string, so a None returncode renders as "Command ... returned non-zero exit status None." instead of crashing. A test that built the exception with an empty-string returncode is updated to pass an integer, which is what a real failed command produces.

The constructor check is a behavior change, so it is documented with a .. versionchanged:: 3.16 note. returncode is a public attribute, so assigning a bad value after construction and then calling str() can still fail, but guarding every __str__ call would be the kind of defensive code the stdlib avoids.

Found as item 9 in devdanzin's standard library audit: https://gist.github.com/devdanzin/3198710e3c0128fda5e0a7b4e0768e5f

Fixes #153970.

Issue: subprocess: CalledProcessError.__str__ crashes when returncode is None #153970

… is None CalledProcessError.__str__ fell through to a branch that formats the return code with %d, which raises TypeError when returncode is None. Handle the None case explicitly and return a message reporting an unknown exit status.

picnixz · 2026-07-18T13:58:43Z

See my reservations on the issue; I don't think the constructor is a public API and thus it's a case of GIGO (garbage-in, garbage-out)

fedonman · 2026-07-18T14:18:46Z

See my reservations on the issue; I don't think the constructor is a public API and thus it's a case of GIGO (garbage-in, garbage-out)

Thank you for reviewing this. Answered on the Issue thread.

vstinner · 2026-07-18T18:47:55Z

I don't think that there is a reason to format the returncode using %d. The issue can be addressed by using a f-string, without checking self.returncode:

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 6fe2ec98fb4..df23fb18a51 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -151,8 +151,8 @@ def __str__(self):
                 return "Command %r died with unknown signal %d." % (
                         self.cmd, -self.returncode)
         else:
-            return "Command %r returned non-zero exit status %d." % (
-                    self.cmd, self.returncode)
+            return (f"Command {self.cmd!r} returned non-zero "
+                    f"exit status {self.returncode}.")
 
     @property
     def stdout(self):

picnixz · 2026-07-18T18:50:21Z

I guess this patch would be acceptable indeed.

The implementation now formats a None returncode with an f-string ("returned non-zero exit status None.") rather than a dedicated branch, so update the test assertion and the NEWS wording to match.

…com:fedonman/cpython into fix-pythongh-153970-calledprocesserror-none

fedonman · 2026-07-19T08:21:28Z

Thank you. I implemented the requested changes.

picnixz

Actually, after sleeping a bit, we would still have an issue if someone is using a returncode as a string for instance (just because self.returncode < 0 would raise here). Since we are considering only theoretical usages, I really wonderf whether we need the change.

This change would make None supported, but would still fail on truthy returncodes not comparable to 0.

So... I'd say:

either check returncode type in the constructor
do nothing but reword the docs a bit.

…teger The earlier fix changed CalledProcessError.__str__ to tolerate a None returncode, but that only handled one non-integer value and would still fail on others, such as a returncode set to a string. The returncode attribute is meant to hold a process exit status, which subprocess always sets to an integer, so there is no real bug in __str__. This drops the code change and the extra test, and instead notes in the docs that returncode is always an integer.

read-the-docs-community · 2026-07-22T08:17:23Z

Documentation build overview

📚 cpython-previews | 🛠️ Build #33706459 | 📁 Comparing 46d6a68 against main (0eac28e)

🔍 Preview build

5 files changed · ± 5 modified

± Modified

Calling str() on a CalledProcessError whose returncode was None raised TypeError, because the __str__ else branch formats the value with %d. A returncode of another type, such as a string, failed even earlier at the self.returncode < 0 comparison. The constructor now raises a clear TypeError when returncode is neither an integer nor None, so __str__ can never be handed a value it cannot format. The else branch also uses an f-string, so a None returncode renders as a message instead of crashing. One test that built the exception with an empty-string returncode is updated to pass an integer, which is what a real failed command produces.

vstinner · 2026-07-23T12:46:28Z

      cmd, returncode, stdout, stderr, output
    """
    def __init__(self, returncode, cmd, output=None, stderr=None):
+        if returncode is not None and not isinstance(returncode, int):


I dislike raising a new exception, since CalledProcessError is created when we are already handling another exception. CalledProcessError should accept anything for returncode, and then does it best to convert returncode to a string in __str__().

vstinner · 2026-07-23T12:46:46Z


    def __str__(self):
+        # A None returncode is falsy, so it skips the '< 0' comparison
+        # below and is formatted by the final branch instead.


returncode should not be None, so I would prefer to omit this comment.

vstinner · 2026-07-23T12:47:29Z

-            return "Command %r returned non-zero exit status %d." % (
-                    self.cmd, self.returncode)
+            return (f"Command {self.cmd!r} returned non-zero "
+                    f"exit status {self.returncode}.")


This two lines changes is the only needed change IMO.

But then we are still having the possibility of the TypeError. In this case I would rather just document that we need an error code that is an integer and leave it as is.

But then we are still having the possibility of the TypeError

Would you mind to elaborate which operation would raise TypeError? The issue is about str() and the change above does fix str().

With the change above, CalledProcessError can be called with any returncode, it just works.

The issue with <0 branch. If you pass [1] you get a type error there

Oh I'm sorry, I completely missed the error on self.returncode < 0.

Well, in that case, I suggest replacing:

if self.returncode and self.returncode < 0:

with:

if isinstance(self.returncode, int) and self.returncode < 0:

IMO we should accept any returncode value in the CalledProcessError constructor and str() should not fail.

It's slightly awkward for me to accept arbitrary return codes. I would assume them to be integers, and not None or other things, and would rather change the docs. In the first place, the class signature is not exposed so I don't consider it as part of the public API, whether subclassing or not.

But, if you really want to support arbitrary types, I'm ok with this change (+ the __repr__ one), but I really think it's a bit too niche.

fedonman requested a review from gpshead as a code owner July 18, 2026 13:46

bedevere-app Bot added the awaiting review label Jul 18, 2026

bedevere-app Bot mentioned this pull request Jul 18, 2026

subprocess: CalledProcessError.__str__ crashes when returncode is None #153970

Open

fedonman added 4 commits July 19, 2026 02:49

change implementation per comment by vstinner

7162066

Merge branch 'main' into fix-pythongh-153970-calledprocesserror-none

a6d21b9

pythongh-153970: Align test and NEWS with the f-string message

fa1a5d6

The implementation now formats a None returncode with an f-string ("returned non-zero exit status None.") rather than a dedicated branch, so update the test assertion and the NEWS wording to match.

Merge branch 'fix-pythongh-153970-calledprocesserror-none' of github.…

da53d9b

…com:fedonman/cpython into fix-pythongh-153970-calledprocesserror-none

picnixz reviewed Jul 19, 2026

View reviewed changes

fedonman and others added 2 commits July 21, 2026 15:33

Merge branch 'main' into fix-pythongh-153970-calledprocesserror-none

8dbfbe3

fedonman changed the title ~~gh-153970: Fix CalledProcessError.__str__ crash when returncode is None~~ gh-153970: Document that CalledProcessError.returncode is an integer Jul 22, 2026

Merge branch 'main' into fix-pythongh-153970-calledprocesserror-none

5dffa29

fedonman requested a review from picnixz July 22, 2026 08:16

fedonman changed the title ~~gh-153970: Document that CalledProcessError.returncode is an integer~~ gh-153970: Validate the returncode type in CalledProcessError Jul 22, 2026

Merge branch 'main' into fix-pythongh-153970-calledprocesserror-none

46d6a68

vstinner reviewed Jul 23, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

fedonman commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz commented Jul 18, 2026

Uh oh!

fedonman commented Jul 18, 2026

Uh oh!

vstinner commented Jul 18, 2026

Uh oh!

picnixz commented Jul 18, 2026

Uh oh!

fedonman commented Jul 19, 2026

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

read-the-docs-community Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fedonman commented Jul 18, 2026 •

edited

Loading

read-the-docs-community Bot commented Jul 22, 2026 •

edited

Loading