Skip to content

Escape and guard traced dictionary keys and channel names - #805

Closed
simpleqt wants to merge 2 commits into
pytest-dev:mainfrom
simpleqt:fix/trace-key-escape
Closed

simpleqt wants to merge 2 commits into
pytest-dev:mainfrom
simpleqt:fix/trace-key-escape

Conversation

@simpleqt

Copy link
Copy Markdown

Escape traced dictionary keys and channel names

TagTracer already renders the message labels and the values of a traced dictionary
through helpers that escape lone surrogates and survive a str()/repr() that
raises — that is what #424, #681 and #728 fixed, so a successful hook call can no
longer be turned into an error by the tracing machinery. The keys of that
dictionary and the channel names were still interpolated straight into the
message, so they escaped that protection:

pm.hook.h(arg=1, **{"\ud800": 2})   # with --debug, tracing to a UTF-8 file
# UnicodeEncodeError: 'utf-8' codec can't encode character '\ud800' ... surrogates not allowed

The UnicodeEncodeError is raised while the traced call is being written, so it
propagates out of the hook call itself, and a key whose __str__ raises
propagates its own exception the same way. Both are now rendered like every other
part of the message, so the trace text is escaped and a broken key is shown as a
rendered failure instead of breaking the call.

The three regression tests cover a surrogate key, a surrogate channel name and a
key whose str() raises; each fails on main (the surrogate cases with
UnicodeEncodeError, the broken key with its RuntimeError) and passes with the
change. Full suite: 225 passed before, 228 passed after; ruff and mypy are
clean at the versions pinned in .pre-commit-config.yaml.

I found this while testing the public API — there is no issue to close.

AI assistance: this change was produced with AI assistance (an autonomous coding
agent), and the reproduction, red/green evidence and test run were executed
locally; a human reviewed the result before submission.

`TagTracer._format_message` rendered labels and values through
`_safe_str()`/`_safe()`/`_render_value()`, which escape lone surrogates and
turn a failing `str()`/`repr()` into a rendered failure. The keys of a traced
dict and the channel names were interpolated directly instead, so a key
containing a lone surrogate -- an argument name, for instance, which any
caller may pass -- made the whole message unencodable for any utf-8 writer,
pytest's `--debug` file included, and the `UnicodeEncodeError` propagated out
of the hook call being traced. A key whose `str()` raises likewise propagated
out of the hook call.

Render both through `_safe_str()` so that every part of the message is
writable and no traced object can fail the call it is tracing; pytest-dev#424 and pytest-dev#681
fixed exactly this for the labels and the values.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

Copy link
Copy Markdown
Member

🤖 Written by Claude Opus 5.5 via Claude Code for the pluggy maintainers; I prompted it, it did the work, I read it.

Closing. We don't accept PRs from unattended AI agents. This account opened 30+ PRs across unrelated projects today.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants