Skip to content

Python: Decode percent-encoded base64 data URI payloads - #9664

Open
Zhang Bowen (MuonChaser) wants to merge 1 commit into
microsoft:mainfrom
MuonChaser:codex/9653-percent-encoded-data-uri
Open

Zhang Bowen (MuonChaser) wants to merge 1 commit into
microsoft:mainfrom
MuonChaser:codex/9653-percent-encoded-data-uri

Conversation

@MuonChaser

@MuonChaser Zhang Bowen (MuonChaser) commented Oct 24, 2026 •

Copy link
Copy Markdown

Motivation & Context

Equivalent literal and percent-encoded base64 data URI payloads currently produce different results: the literal PNG is detected and extracted, while its escaped form fails. Percent-decoding the payload before base64 processing restores their equivalence, as required by the Fetch data URL processor.

Description & Review Guide

  • What are the major changes? Decode payload URL escapes once in the existing shared parser with unquote, which preserves literal +. The extraction helper continues to return a base64 string. Add offline regressions through public media detection and Content construction, checking decoded bytes, escaped /, +, =, lowercase escapes, raw inputs, nested escapes, metadata and errors.
  • What is the impact of these changes? Equivalent data URI payloads now produce the same detected type and extracted bytes. Stored URIs, metadata validation, raw string/byte inputs, public signatures, dependencies, and base64 decoder settings are unchanged. Core wheel/sdist build, changed-file Ruff checks, strict source Pyright, and all five test-file type checkers passed. The related type/serialization/observability selection passed 872 tests with 14 skipped and 3 warnings; existing observability exporter shutdown errors were also reproduced against unmodified source. The standard aggregate unit suite (uv run poe test -A -m "not integration" -n 4) also passed on Linux / Python 3.12.3: 19,158 passed, 551 skipped, 2 xfailed, with 790 warnings and no failures or errors. This follows the repository default selection, excluding integration, DevUI and Lab. Live provider calls and the other Python/OS CI matrix lanes were not run locally.
  • What do you want reviewers to focus on? The single decode boundary and the regression that a literal + survives while %252F remains %2F after one decode.

Related Issue

Fixes #9653.

Trivial-change exception: this is an obvious correction to the existing base64 data URI processing order, restoring equality for two representations of the same payload. It introduces no API, URI mode, decoder permissiveness, or provider-specific transformation. No maintainer agreement is claimed. Pre-publication all-state searches found no competing PR; implementation is coordinated in #9653.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Reproduction, implementation, regression tests, local validation and PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI balanced review requested due to automatic review settings October 24, 2026 04:38

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 insufficient remaining quota.

@MuonChaser

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

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

Independent offline verification of 532e37bc2932764c303f0ac335d1ec1744f99151 adds serialized-message coverage beyond the PNG cases: Linux / CPython 3.11 job.

I exercised public Content.from_uri() → Message.to_dict() → ordinary JSON encode/decode → Message.from_dict(), then public media detection and the existing binary extraction helpers. Across PNG, JPEG, GIF and PDF, both fully percent-escaped and mixed literal/escaped payloads preserve the original URI, media type and decoded bytes. Literal + remains literal in the mixed cases; metadata containing %2F stays unchanged in the stored URI.

  • Restoring only _types.py to the PR's original merge base 9c8a3514a15a702f81bfda456d80c7ccaa4d2712 produces 8 genuine invalid-base64 failures and 4 passing literal-URI controls. The same 12 cases all pass on this head.
  • The complete packages/core/tests/core/test_types.py passes: 508 tests, zero skips.
  • The normal changed-file prek checks pass, with 4 nonapplicable hook skips and no tracked source or configuration changes afterward.

This is a focused factual COMMENT, not whole-PR approval. No live provider/model calls, other OS/Python lanes, complete workspace tests or distribution build were run by this review. The contributor's source/branch and PR description remain unchanged, and no competing fix was opened. AI assistance was used for the independent probes and this report.

@MuonChaser

Copy link
Copy Markdown
Author

BlueRaddish Thanks for the additional serialized-message coverage and baseline comparison. I checked the linked ms-review logs: the 12 probes, 508 type tests, and scoped hooks pass on 532e37bc2932. No code changes are needed in response to this factual review. This remains focused independent verification; maintainer approval and upstream CI are still outstanding.

This branch was successfully deployed

1 active deployment
github-app-auth — 532e37bc Deployed Oct 24, 2026 by MuonChaser via add_label #26078
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: Percent-encoded base64 data URIs fail media type detection

3 participants