Skip to content

Fix multipart filename quoting#279

Open
jeet-dekivadia wants to merge 2 commits into
openai:mainfrom
jeet-dekivadia:codex/quote-multipart-filenames
Open

Fix multipart filename quoting#279
jeet-dekivadia wants to merge 2 commits into
openai:mainfrom
jeet-dekivadia:codex/quote-multipart-filenames

Conversation

@jeet-dekivadia

@jeet-dekivadia jeet-dekivadia commented May 30, 2026

Copy link
Copy Markdown

Summary

  • serialize multipart filename= values as quoted-strings instead of URI path segments
  • escape quote and backslash delimiters and drop CR/LF before writing multipart headers
  • cover literal spaces and crafted header-injection input

Why

Multipart filename= parameters are quoted-string values. URL-encoding them changes ordinary filenames such as d o g into d%20o%20g. Writing literal values instead also needs delimiter escaping and CR/LF removal so crafted filenames cannot inject multipart headers.

Validation

  • git diff --check
  • standalone Ruby probe for quote escaping and CR/LF removal
  • checked changed source lines against the repository's 120-character style

Remote Linux validation

  • git diff --check
  • bundle install
  • ./scripts/lint
  • ./scripts/test

Signed-off-by: Jeet Dekivadia <jeet.university@gmail.com>
@jeet-dekivadia jeet-dekivadia requested a review from a team as a code owner May 30, 2026 09:39

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8ecfd8681

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread lib/openai/internal/util.rb Outdated
#
# @return [String]
private def escape_multipart_filename(filename)
filename.to_s.gsub(/["\\]/) { "\\#{_1}" }.delete("\r\n")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve compatible encodings in multipart streams

When a filename contains non-ASCII characters and the uploaded content is binary, this emits a UTF-8 header chunk followed by an ASCII-8BIT file chunk; the Net requester wraps the enumerator in ReadIOAdapter, whose buffer concatenation then raises Encoding::CompatibilityError before the request is sent. This regresses uploads such as OpenAI::FilePart.new(binary_io, filename: "é.png"), which previously kept the filename ASCII via encode_path; please keep multipart chunks in a consistent binary/ascii-compatible encoding or otherwise avoid mixing UTF-8 header chunks with binary body chunks.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 3aa13fe. Escaped multipart filenames are now emitted as ASCII-8BIT, and the regression covers a non-ASCII filename followed by binary content through ReadIOAdapter.

Signed-off-by: Jeet Dekivadia <jeet.university@gmail.com>
@jeet-dekivadia

Copy link
Copy Markdown
Author

Quick follow-up for review context. The second commit keeps multipart filename header chunks binary-compatible with file body writes, while still serializing filename parameters as quoted strings, escaping quote and backslash delimiters, and removing CR/LF before header emission.

Validation from preparation: git diff --check; bundle install; ./scripts/lint; ./scripts/test; plus small Ruby probes for quote escaping, CR/LF removal, and ASCII-8BIT concatenation with binary content. I kept the scope limited to multipart header serialization and focused regressions.

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