Skip to content

Frame multipart file chunks from the bytes actually copied - #330

Open
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/multipart-http
Open

Frame multipart file chunks from the bytes actually copied#330
OskarEichler wants to merge 1 commit into
ruby:masterfrom
OskarEichler:codex/multipart-http

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Reuse the existing Chunker for size-aware multipart file streams so each chunk header describes the bytes actually copied. Do not use the total file size as the next chunk length.

Reproduction

Set a multipart file to StringIO.new('abcde') positioned at 2, with a filename option and Transfer-Encoding: chunked. Before, the file chunk advertises 5 bytes but contains cde (3). An empty file emits a zero chunk before subsequent fields/boundary. After, both decode to the complete expected multipart body.

Verification

  • Ruby 4.0.6 through rbenv; existing bundle exec rake test: 201 tests, 914 assertions, zero failures/errors, both baseline and this isolated patch.
  • 28 focused checks decode the actual chunk stream and compare exact multipart bytes for StringIO and real files: empty, 5-byte and 40,000-byte bodies at beginning/middle/end offsets, with fields before and after the upload.
  • Ruby syntax and git diff --check pass. Supplemental Lint scan retains the same 25 pre-existing findings; no new findings by file/cop.
  • No test/spec files were added or modified, per this contribution's explicit no-new-tests constraint. Focused reproductions were run externally.

Compatibility and limitations

No API or dependency change. Chunk boundaries may differ, but multipart content remains the same. Empty files no longer prematurely terminate chunked transfer. The non-size-aware IO path is unchanged.

Based on master 23e859e92e10e43cad520fcacac3fd43640dba7b. Optional external OpenSSL tests and other Ruby/OS combinations were not run. Existing related issues/PRs were checked; this is a focused correctness change, not a claim of exhaustive coverage.

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