Skip to content

fix(api): preserve binary percent-encoded data URI payloads - #681

Open
linjiaye929 wants to merge 1 commit into
michaelfeil:mainfrom
linjiaye929:codex/binary-data-uri-decoding
Open

linjiaye929 wants to merge 1 commit into
michaelfeil:mainfrom
linjiaye929:codex/binary-data-uri-decoding

Conversation

@linjiaye929

Copy link
Copy Markdown

Summary

Preserve binary bytes when decoding percent-encoded data URI payloads.

The non-base64 parser currently calls unquote, which decodes the payload as UTF-8 text, then re-encodes that text. Non-UTF-8 bytes are replaced or rejected. A valid 77-byte PNG produced by DataURI.make(..., base64=False) becomes 97 bytes and is rejected as ImageCorruption; the same PNG works when base64-encoded.

Use unquote_to_bytes for non-base64 payloads, matching the octet representation in RFC 2397 section 2. When the helper receives text, encode it with the declared charset before percent-quoting. Binary helper input is not transcoded. Base64 payload handling and filename decoding are unchanged.

Compatibility

Default UTF-8 and ASCII text retain their behavior. Text helper calls with a declared charset continue to round-trip, and their emitted payload now matches that charset. For example, Latin-1 café is emitted as caf%E9 rather than incorrectly labeled UTF-8 caf%C3%A9.

Previously persisted non-base64 URIs whose declared charset did not match their percent-encoded bytes may decode differently. Regenerate those payloads using their actual declared encoding. This correction does not add charset or media-type validation.

Validation

Linux, Python 3.12.14. Schema/HTTP checks used NumPy 1.26.4, Pillow 12.3.0, Pydantic 2.14.1, FastAPI 0.115.2, and httpx 0.28.1.

From libs/infinity_emb:

PYTHONPATH=$PWD DO_NOT_TRACK=1 HF_HUB_OFFLINE=1 python -m pytest \
  --confcutdir tests/unit_test/fastapi_schemas \
  tests/unit_test/fastapi_schemas -q -p no:cacheprovider
  • 29 passed after the change, including the two adjacent schema tests.
  • The final DataURI test file gives 10 failed, 17 passed against unchanged 040269b and 27 passed with the fix.
  • Tests cover all 256 byte values, bytes with declared charsets, default/explicit UTF-8 and Latin-1 text, base64 controls, filenames, file helpers, literal plus/percent characters, and generated PNG bytes through actual request validation and image decoding.
  • The focused schema command explicitly bypasses the root test conftest, whose sentence_transformers dependency is absent from this minimal server environment. These tests use no root-conftest fixtures. This is not a full-suite result.
  • A separate local HTTP reproduction used the real FastAPI routes, engine, native batching workers, and Pillow decoder with a deterministic pixel-mean model fixture. Both /embeddings with image modality and /embeddings_image return 200 with identical embeddings for percent-encoded and base64 PNGs after the change; the percent-encoded cases returned 400 before. No pretrained model was downloaded or tested.

In a separate existing CPU test environment with the normal root conftest:

DO_NOT_TRACK=1 HF_HUB_OFFLINE=1 python -m pytest \
  tests/unit_test/test_engine.py::test_async_api_debug \
  tests/unit_test/test_engine.py::test_args_between_array_and_engine_same \
  tests/unit_test/test_sync_engine.py::test_sync_engine \
  tests/unit_test/test_sync_engine.py::test_sync_engine_double \
  tests/unit_test/inference/test_select_model.py \
  tests/unit_test/test_args.py \
  -k 'not torch and not optimum and not ctranslate2' -q -p no:cacheprovider
  • 12 passed, 3 deselected.
  • ruff check ., changed-file formatting, codespell, and git diff --check pass.
  • Full-source mypy output is identical on the branch and unchanged baseline: 22 missing-optional-dependency errors in 14 files, including the existing pydantic_core import error. This is not a clean type-check pass.
  • Full make precommit, full pytest, accelerator, and pretrained-model suites were not run.

Checklist

  • Read the contribution guidelines.
  • Added regression tests.
  • Documented data URI payload encodings.

AI assistance was used to prepare this contribution.

@linjiaye929
linjiaye929 marked this pull request as ready for review October 12, 2026 10:31
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T10:35:29.202051Z 0edeb56 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Preserves binary bytes in percent-encoded data URIs and makes text passed to DataURI.make use its declared charset.

  • Percent-encoded data URIs keep their original bytes.

Acknowledgment: linjiaye929 explicitly documents that previously saved payloads with bytes that do not match their declared charset may decode differently. This compatibility change is intentional.

Reviews (1) · Last reviewed commit: "fix(api): preserve binary percent-encode..." · Reviewed by Greptile

This branch has not been deployed

No deployments
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.

1 participant