Skip to content

Fix byte-limited reads for Enumerator-backed request bodies - #275

Closed
fallintoplace wants to merge 1 commit into
openai:mainfrom
fallintoplace:fix/read-io-adapter-byte-read
Closed

Fix byte-limited reads for Enumerator-backed request bodies#275
fallintoplace wants to merge 1 commit into
openai:mainfrom
fallintoplace:fix/read-io-adapter-byte-read

Conversation

@fallintoplace

@fallintoplace fallintoplace commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make Enumerator-backed ReadIOAdapter#read(max_len) return at most max_len bytes
  • use byte-oriented buffering so multibyte strings follow IO#read semantics
  • preserve buffered bytes when an unbounded read follows a partial read
  • match IO behavior for zero-length reads, EOF, negative lengths, and out_string
  • add regression coverage for bounded reads, multibyte data, buffering, and output buffers

Why

This adapter feeds streamed request bodies into IO consumers such as IO.copy_stream. Returning more bytes than requested can break those consumers and corrupt multipart upload boundaries; character-oriented slicing also makes the result depend on string encoding instead of the requested byte count.

Fixes #274.

Test plan

  • bundle exec rake TEST=test/openai/internal/util_test.rb test
  • bundle exec rake lint

@fallintoplace
fallintoplace marked this pull request as ready for review May 19, 2026 17:41
@fallintoplace
fallintoplace requested a review from a team as a code owner May 19, 2026 17:41
@jbeckwith-oai jbeckwith-oai added the generator Touches generated SDK files label Jul 16, 2026

Copy link
Copy Markdown
Contributor

Thank you so much for the careful and comprehensive work here, @fallintoplace. Your analysis of byte-oriented max_len reads, preserving buffered data across bounded and unbounded reads, EOF and out_string semantics, and negative lengths became the backbone of the canonical fix.

Since this PR was opened, ReadIOAdapter moved into its SDK-owned file and main advanced substantially. We carried your work forward in #354, rebased it onto current main, combined it with the mixed-encoding fix for #317, preserved native IO/StringIO delegation, and added end-to-end and randomized contract validation. That replacement is now fully green across the Ruby matrix, lint, signatures, packaging, and CodeQL.

I’m closing this PR only because #354 supersedes it—not because the contribution wasn’t valuable. It materially shaped the final implementation and gave us the complete contract we needed. Really appreciate the time, rigor, and care you put into this. Thank you! 🙏

jbeckwith-oai added a commit to fallintoplace/openai-ruby that referenced this pull request Aug 10, 2026
## Summary

- preserve arbitrary binary bytes when multipart request bodies yield a
mix of UTF-8 and binary chunks
- honor `IO#read(max_len, out_string)` byte limits, buffering, EOF,
negative-length, and output-buffer semantics for Enumerator-backed
bodies
- retain native `IO` and `StringIO` delegation, including implicit
`to_int` length conversion
- update RBI/RBS contracts and add focused regression coverage

This fixes openai#317 and consolidates the complete behavior needed from openai#275,
openai#294, and openai#324 on the current SDK-owned `ReadIOAdapter` implementation.

## Ownership

This is handwritten SDK runtime behavior introduced as an SDK-owned
component in openai#345. It does not change OpenAPI inputs, Castiron facts,
renderer rules, templates, or generator configuration.

## Validation

- `mise exec ruby@4.0.6 -- ./scripts/test` — 531 runs, 1,736 assertions,
0 failures
- `mise exec ruby@4.0.6 -- bundle exec rake lint` — RuboCop, Sorbet, and
1,211 RBS files clean
- `mise exec ruby@4.0.6 -- bundle exec rake build:gem`
- installed-gem adapter probe
- Ruby 3.3.12 mixed-encoding compatibility probe
- 1,000 randomized read sequences compared against `StringIO`
- real local HTTP multipart upload with a 1 MiB binary file and UTF-8
prompt, verified byte-for-byte by SHA-256
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

generator Touches generated SDK files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReadIOAdapter#read returns too many bytes for Enumerator bodies

2 participants