Skip to content

gh-151485: Fix command quoting in subprocess.CalledProcessError.__str__#151486

Merged
vstinner merged 5 commits into
python:mainfrom
BenjyWiener:fix-CalledProcessError-quoting
Jun 24, 2026
Merged

gh-151485: Fix command quoting in subprocess.CalledProcessError.__str__#151486
vstinner merged 5 commits into
python:mainfrom
BenjyWiener:fix-CalledProcessError-quoting

Conversation

@BenjyWiener

@BenjyWiener BenjyWiener commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

subprocess.CalledProcessError.__str__ previously formatted cmd as "... '%s' ...". This lead to unbalanced quoting when cmd contains single-quotes or, more commonly, when cmd is a list. This change updates the relevant format strings to use %r instead.

In terms of backwards compatibility, the only impact I anticipate is on doctests, and I suspect doctests for subprocess.CalledProcessError to be quite rare.

Before this PR:

>>> import subprocess
>>> subprocess.check_call(['false'])
Traceback (most recent call last):
  ...
subprocess.CalledProcessError: Command '['false']' returned non-zero exit status 1.

After:

>>> import subprocess
>>> subprocess.check_call(['false'])
Traceback (most recent call last):
  ...
subprocess.CalledProcessError: Command ['false'] returned non-zero exit status 1.

CalledProcessError previously formatted cmd as `"... '%s' ..."`. This lead to
unbalanced quoting when cmd contains single-quotes or, more commonly, when cmd
is a list. This change updates the relevant format strings to use %r instead.
@bedevere-app

bedevere-app Bot commented Jun 15, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@vstinner vstinner 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.

Can you please add a test to test_subprocess?

Comment thread Lib/test/test_subprocess.py Outdated
@BenjyWiener BenjyWiener requested a review from vstinner June 16, 2026 13:39

@vstinner vstinner 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.

LGTM

@vstinner

Copy link
Copy Markdown
Member

Tests / Sanitizers / TSan (free-threading) (pull_request): Cancelled after 60m

Ah, the job was cancelled, test_abc seems to be stuck:

Run ./python -m test --tsan-parallel --parallel-threads=4 -j4 -W
Using random seed: 2157163694
0:00:00 load avg: 2.93 mem: 274.0 MiB Run 2 tests in parallel using 2 worker processes
0:00:12 load avg: 3.10 mem: 591.6 MiB [1/2] test_hashlib passed
0:00:42 load avg: 1.88 mem: 591.6 MiB running (1): test_abc (42.3 sec)
0:01:13 load avg: 1.14 mem: 591.7 MiB running (1): test_abc (1 min 12 sec)
...
0:37:00 load avg: 0.01 mem: 533.8 MiB running (1): test_abc (36 min 59 sec)
Error: The operation was canceled.

@vstinner

Copy link
Copy Markdown
Member

Ah, the job was cancelled, test_abc seems to be stuck

I created #151593 to investigate this test_abc issue.

@python-cla-bot

python-cla-bot Bot commented Jun 21, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@vstinner vstinner merged commit b35c379 into python:main Jun 24, 2026
51 checks passed
@vstinner

Copy link
Copy Markdown
Member

Merged, thanks for the fix. I don't think that it should be backported since it can impact tests relying on the exact str(exc).

@BenjyWiener

Copy link
Copy Markdown
Contributor Author

Thanks! Out of curiosity, what's the policy for fixes like these when it comes to beta releases?

@vstinner

Copy link
Copy Markdown
Member

Thanks! Out of curiosity, what's the policy for fixes like these when it comes to beta releases?

After the feature freeze (beta1), new features and backward incompatible changes are no longer accepted (in the 3.15 branch).

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.

2 participants