Repository navigation
fix(python): base64-encode raw bytes passed to base64 file input fields - #200
Conversation
github-actions
Bot
added
csharp
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 19 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes base64 encoding for raw byte inputs in the Python SDK, with updated generated code, tests, and migration documentation.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes Python SDK base64 file input handling so raw bytes/bytearray/memoryview are base64-encoded instead of being passed through or str-coerced; updates template, generated SDK, feature tests and docs, with tests pinning the corrected behavior.
View guided diff | Turn on auto-fix | Re-trigger cubic
2ynn
removed
csharp
Generator Snapshot Testing
Snapshot results: https://github.com/speakeasy-api/openapi-generation-snapshots/issues/157#issuecomment-6768889383 Snapshot results are summarized above. Additional run context is linked for maintainers with access. |
AshGodfrey
left a comment
AshGodfrey
left a comment
There was a problem hiding this comment.
Requesting changes on four behaviours this PR introduces; the helper on main does none of them. For each one: is it an intentional trade-off? If so, a sentence in the PR body or the docstring is enough and I will re-review. Otherwise there is a suggestion inline.
All four were reproduced by running the main and branch versions of encode_base64_file_input side by side under pydantic 2.14.2 on a model with a Base64EncodedString field.
| ) | ||
| return base64.b64encode(binary).decode("ascii") | ||
| value = binary | ||
| if isinstance(value, (bytes, bytearray, memoryview)): |
There was a problem hiding this comment.
Pre-encoded bytes are now double-encoded, silently. base64.b64encode(raw) returns bytes, and passing that result straight in worked on main through pydantic's lax str coercion:
M(f=base64.b64encode(b"hello world payload")).f
main: 'aGVsbG8gd29ybGQgcGF5bG9hZA==' # decodes back to the payload
branch: 'YUdWc2JHOGdkMjl5YkdRZ2NHRjViRzloWkE9PQ=='
Nothing raises, so the server stores base64 text as the file content. Is this intended?
There was a problem hiding this comment.
Yes this was intentional decision on my part:
PR description called out:
Already-encoded base64 held as bytes must now be passed as str (value.decode("ascii")); passing it as bytes encodes it a second time. That input was never part of the declared Union[str, Base64FileInput] type, and the changeset states the migration
In this PR, a bytes-like input is interpreted as raw content and always gets base64-encoded. Note that the SDK cannot tell already-encoded bytes from raw bytes( e.g. b"YWJj" could be base64 for abc or the raw content YWJj). This means that if a caller passes an already base64-encoded byte (e.g. b"aGVsbG8=") it will get encoded a second time ("YUdWc2JHOD0="). To send an already-encoded base64 it should be passed as a str instead (value.decode("ascii")).
The reason why I don't consider this a breaking change is that passing bytes was never part of the contract (it's a type error and only got through because pydantic's lax mode converts bytes to str), the caeveat being that if anyone relied on the old behavior they would get silently wrong data after upgrading, with no error.
| ) | ||
| return base64.b64encode(binary).decode("ascii") | ||
| value = binary | ||
| if isinstance(value, (bytes, bytearray, memoryview)): |
There was a problem hiding this comment.
Union member order now decides what a bytes value becomes. For a field typed Union[Base64EncodedString, bytes] (a oneOf of a file-mode format: byte string and a format: binary string), b"..." resolved to the bytes member on main regardless of order. On this branch it becomes the base64 str when the base64 member is listed first and stays bytes when it is second: the before-validator makes the str member a strict-mode match, so pydantic's smart union takes it.
Rare shape, so if it is accepted as-is a note is enough; flagging it because the result flips on spec ordering rather than on the input.
There was a problem hiding this comment.
Fixed in 56b60f2. Values encoded from bytes-like input are now returned as a private str subclass. Pydantic scores a str subclass as a strict but non-exact match, so for bytes input an exact bytes member wins regardless of member order: Union[Base64EncodedString, bytes, int] and Union[bytes, Base64EncodedString, int] both keep b"abc" as bytes ({"value":"abc"}), as on main. The stored field value is still a plain str. PathLike and stream input still return a plain str, so they keep their exact match against members such as Any.
Regression test: test_request_bodies_base64_file_input_union_members covers both orders. Checked on pydantic 2.11.2 (the SDK's minimum) through 2.14.3.
Two rarer mixed-union cases stay order- or type-dependent, because smart union has only three match levels. They are listed under "Known limitations: mixed unions" in the PR body.
| return base64.b64encode(binary).decode("ascii") | ||
| value = binary | ||
| if isinstance(value, (bytes, bytearray, memoryview)): | ||
| return base64.b64encode(bytes(value)).decode("ascii") |
There was a problem hiding this comment.
Any memoryview is accepted, whatever its item format. memoryview(array.array('i', [1, 2])) was a validation error on main; here it sends AQAAAAIAAAA=, the native-endian buffer, so the wire bytes depend on the host's int size and byte order.
Suggest restricting to byte-sized formats and raising otherwise, e.g. if isinstance(value, memoryview) and value.format not in ("B", "b", "c"): raise TypeError(...), or casting explicitly with value.cast("B") if accepting them is the intent.
There was a problem hiding this comment.
Fixed in 56b60f2. A memoryview must have a byte-sized item format (B, b or c). Byte-order prefixes are ignored, so ctypes buffers ('<B') and cast("@B") still work. Any other format is rejected.
It raises ValueError rather than TypeError. A TypeError from a before-validator escapes pydantic unwrapped, which aborts union resolution: for Union[List[int], Base64EncodedString], memoryview(array('i', [1, 2])) would raise instead of falling through to List[int]. With ValueError it resolves to [1, 2] there, and a plain field raises ValidationError, as on main. The check also applies to a memoryview returned by a stream's read().
| return base64.b64encode(binary).decode("ascii") | ||
| value = binary | ||
| if isinstance(value, (bytes, bytearray, memoryview)): | ||
| return base64.b64encode(bytes(value)).decode("ascii") |
There was a problem hiding this comment.
bytes(value) copies every bytearray and contiguous memoryview; b64encode accepts both directly. Only a non-contiguous view needs the copy:
if isinstance(value, memoryview) and not value.c_contiguous:
value = bytes(value)
return base64.b64encode(value).decode("ascii")For a large upload the current form holds the original, a full copy, and the base64 output at once.
There was a problem hiding this comment.
Fixed in 56b60f2. bytes, bytearray and contiguous memoryview values go to b64encode directly; only a non-contiguous memoryview is copied first.
TristanSpeakEasy
left a comment
TristanSpeakEasy
left a comment
There was a problem hiding this comment.
Looks good overall, the raw-bytes behaviour and the migration for pre-encoded bytes are clear. I agree with the mixed-union concern and left a concrete example below, but I don't think that edge case needs to block this fix.
| ) | ||
| return base64.b64encode(binary).decode("ascii") | ||
| value = binary | ||
| if isinstance(value, (bytes, bytearray, memoryview)): |
There was a problem hiding this comment.
I can reproduce the existing union concern, and it changes the JSON too: Union[Base64EncodedString, bytes] now turns b"abc" into "YWJj", while reversing the members sends "abc". Both orders sent "abc" before. For a field that allows either binary or base64 content, an existing binary input can therefore silently switch representation. Worth preserving the explicit bytes branch's precedence when both are available and adding a regression test for both orders. Not blocking, but this is separate from the documented migration for pre-encoded bytes on base64-only fields.
There was a problem hiding this comment.
Fixed in 56b60f2. bytes input now resolves to the bytes member in either order, so b"abc" sends "abc" for both Union[Base64EncodedString, bytes] and Union[bytes, Base64EncodedString], as before. test_request_bodies_base64_file_input_union_members covers both orders.
Two edge cases remain, listed under "Known limitations: mixed unions" in the PR body:
bytearrayin the same union is sent through the base64 member as"YWJj"in both orders (mainsent"abc"). Thebytesmember only acceptsbytearrayin lax mode, so the base64 member matches first.- With an
Anymember (oneOf: [{}, <file-mode string>]),bytesinput follows member order:Anyfirst keeps the raw bytes ("abc"), base64 first sends"YWJj".str, path and stream input resolve the same in both orders.
Smart union has three match levels, so the base64 member cannot rank below bytes and above Any at the same time.
Fields opted into `x-speakeasy-base64-input-mode: file` accept a
pre-encoded base64 `str`, a `PathLike`, or a binary stream. Raw
`bytes`, `bytearray` and `memoryview` values fell through
`encode_base64_file_input` unchanged and reached pydantic's lax `str`
coercion, which decodes them as UTF-8:
- non-UTF-8 payloads (most binary content) failed validation with
`string_unicode`
- ASCII payloads were accepted and sent on the wire unencoded
- `memoryview` failed with `string_type`
`encode_base64_file_input` now base64-encodes bytes-like values,
including non-contiguous `memoryview` slices, and `Base64FileInput`
includes `bytes`, `bytearray` and `memoryview` so type checkers accept
them in the input TypedDicts. Paths and binary streams are read into
bytes and go through the same encoding step, so streams whose `read()`
returns a `memoryview` are also accepted instead of raising `TypeError`.
The input type decides the encoding: `str` is sent as already-encoded
base64, bytes-like values are treated as raw content. Content alone
cannot tell the two apart, since short alphanumeric payloads are also
valid base64. Already-encoded base64 held as `bytes` must now be passed
as `str` (`value.decode("ascii")`); passing it as `bytes` encodes it a
second time. That input was never part of the declared
`Union[str, Base64FileInput]` type.
…e input request Send `b"YWJj"` and a strided `memoryview` through the SDK request so the wire value pins that the input type, not base64-looking content, decides encoding, and that the generated input surface accepts `memoryview`.
…ews in base64 file input - Return values encoded from bytes-like input as a str subclass so pydantic scores them as a strict, non-exact match. In a union that also has a `bytes` member, `bytes` input resolves to that member regardless of member order. PathLike and IO input still returns a plain str, so the base64 member stays an exact match and still outranks members such as `Any`. - Reject memoryview values whose item format is not byte-sized (ignoring byte-order prefixes) with ValueError, so pydantic reports a validation error and other union members are still tried. - Pass bytearray and contiguous memoryview values to b64encode directly; only non-contiguous views are copied. - Document the encoding contract in the helper docstring.
1ebcc9a to
56b60f2
Compare
There was a problem hiding this comment.
0 issues found across 4 reviewed files. 1 file intentionally excluded from review.
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Fixes Python SDK base64 file input handling so raw bytes/bytearray/memoryview are base64-encoded instead of passed through or str-coerced; updates template, generated SDK, feature test and docs, with tests pinning the corrected behavior.
View guided diff | Turn on auto-fix | Re-trigger cubic
All five threads are substantively addressed at 56b60f2
Why
Fields opted into
x-speakeasy-base64-input-mode: fileaccept a pre-encoded base64str, aPathLike, or a binary stream. Rawbytes,bytearrayandmemoryviewvalues were passed throughencode_base64_file_inputunchanged. Pydantic's laxstrcoercion then decoded them as UTF-8:string_unicodememoryviewfailed withstring_typeBinary streams whose
read()returns amemoryviewraisedTypeError.What changed
encode_base64_file_inputbase64-encodes bytes-like values. Paths and binary streams are read into bytes and go through the same encoding step.memoryviewvalues must have a byte-sized item format (B,b,c, with or without a byte-order prefix). Other formats raiseValueError, which pydantic reports as a validation error, so the wire bytes never depend on the host's int size or byte order.bytearrayand contiguousmemoryviewvalues are encoded without an intermediate copy; only non-contiguous views are copied.bytesmember (e.g.Union[Base64EncodedString, bytes, int], generated from aoneOfof a file-modeformat: bytestring, aformat: binarystring and another type),bytesinput resolves to thebytesmember regardless of member order, as onmain. Values encoded from bytes-like input are returned as astrsubclass that pydantic scores as a strict but non-exact match, so an exactbytesmember wins.Base64FileInputincludesbytes,bytearrayandmemoryview, so type checkers accept them in the input TypedDicts.types/base64fileinput.pyand itsgen.lockchecksum change.request-bodies-base64-file-input-raw-bytes(pythonv2 only; skipped on the other targets, which do not implement the extension).Encoding contract
The input type decides the encoding:
stris sent as already-encoded base64, bytes-like values are treated as raw content. Content alone cannot tell the two apart, since short alphanumeric payloads are also valid base64.Warning
bytes(e.g. the result ofbase64.b64encode(...)) must now be passed asstr(value.decode("ascii")). Passing it asbytesencodes it a second time, silently. That input was never part of the declaredUnion[str, Base64FileInput]type, and the changeset states the migration.Known limitations: mixed unions
Rare shapes where a base64 file-input member shares a union with a member that also accepts bytes-like input. Pydantic's smart union has three match levels, so the helper cannot rank the base64 member above every other member and below
bytesat the same time.bytearraywith abytesmember: forUnion[Base64EncodedString, bytes],bytearray(b"abc")is sent through the base64 member as"YWJj"(mainsent"abc"). Thebytesmember only acceptsbytearrayin lax mode, so the base64 member matches first. Exactbytesinput is unaffected.byteswith anAnymember: for a union generated fromoneOf: [{}, <file-mode string>],bytesinput follows member order:Union[Any, Base64EncodedString]keeps the rawbytes("abc", as onmain),Union[Base64EncodedString, Any]sends"YWJj".str,PathLikeand stream input resolve the same in both orders.Why not a new extension value
filealready means "pass content, the SDK encodes it", and file content isbyteselsewhere in the Python SDK (File.content: Union[bytes, IO[bytes], io.IOBase]). Raw bytes had no working behaviour underfile, so keeping it and adding a separate value would leave existing specs on the broken path. A new value would also not avoid thebytes-as-pre-encoded break: SDK users would hit it as soon as a spec author switched values.Testing
pytest tests/test_request_bodies_additional.py -k base64in the generatedprimarypythonv2 SDK: 5 passed.bytes, ASCIIbytearray(sent encoded), base64-lookingb"YWJj"(sent as"WVdKag==", not passed through), and a stridedmemoryview.'<B')memoryview, a stream returningmemoryview, a non-bytememoryviewraisingValueError, and"YWJj"passing through unchanged.bytes,strandintinput againstUnion[Base64EncodedString, bytes, int]in both member orders,PathLikeand stream input againstUnion[Any, Base64EncodedString], and a non-bytememoryviewfalling through toList[int]inUnion[List[int], Base64EncodedString].mypy,pyright,pylinton the generatedbase64fileinput.py: clean../scripts/build-review-sdk.sh pythonv2: integrity validation passed.prettier --checkon touchedfeatures.tsfiles;go build ./internal/features/.make test-pythonv2not run; the change is confined to the base64 input helper.Public-safety check
.claude/skills/public-repo-communication/SKILL.md).git diff --check.