From 9d04855c556e76bcc44b062549f3b4ed06e763b2 Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Sun, 9 Aug 2026 12:57:43 -0700 Subject: [PATCH] fix: encode multipart array and nested fields Encode arrays and nested objects recursively with bracket notation while preserving scalar multipart fields. Escape all Content-Disposition parameters and cover generated request models for transcription, image editing, and file expiration. --- lib/openai/internal/util.rb | 50 +++++++++----- rbi/openai/internal/util.rbi | 18 +++++ sig/openai/internal/util.rbs | 12 ++++ test/openai/internal/util_test.rb | 106 +++++++++++++++++++++++++++++- 4 files changed, 167 insertions(+), 19 deletions(-) diff --git a/lib/openai/internal/util.rb b/lib/openai/internal/util.rb index d6af40af..861fb14a 100644 --- a/lib/openai/internal/util.rb +++ b/lib/openai/internal/util.rb @@ -496,14 +496,14 @@ def encode_query_params(query) # @api private # - # Multipart filenames are quoted-strings, not URI path segments. Escape header - # delimiters and remove CR/LF without encoding ordinary filename characters. + # Multipart disposition parameters are quoted-strings, not URI path segments. + # Escape header delimiters and remove CR/LF without encoding ordinary characters. # - # @param filename [Pathname, String] + # @param value [Pathname, String, Symbol] # # @return [String] - private def escape_multipart_filename(filename) - filename.to_s.gsub(/["\\]/) { "\\#{_1}" }.delete("\r\n").b + private def escape_multipart_header_param(value) + value.to_s.gsub(/["\\]/) { "\\#{_1}" }.delete("\r\n").b end # @api private @@ -518,15 +518,16 @@ def encode_query_params(query) y << "Content-Disposition: form-data" unless key.nil? - y << "; name=\"#{key}\"" + name = escape_multipart_header_param(key) + y << "; name=\"#{name}\"" end case val in OpenAI::FilePart unless val.filename.nil? - filename = escape_multipart_filename(val.filename) + filename = escape_multipart_header_param(val.filename) y << "; filename=\"#{filename}\"" in Pathname | IO - filename = escape_multipart_filename(::File.basename(val.to_path)) + filename = escape_multipart_header_param(::File.basename(val.to_path)) y << "; filename=\"#{filename}\"" else end @@ -535,6 +536,28 @@ def encode_query_params(query) write_multipart_content(y, val: val, closing: closing) end + # @api private + # + # @param y [Enumerator::Yielder] + # @param boundary [String] + # @param key [Symbol, String] + # @param val [Object] + # @param closing [Array] + private def write_multipart_value(y, boundary:, key:, val:, closing:) + case val + in Hash + val.each do |name, value| + write_multipart_value(y, boundary: boundary, key: "#{key}[#{name}]", val: value, closing: closing) + end + in Array + val.each do |value| + write_multipart_value(y, boundary: boundary, key: "#{key}[]", val: value, closing: closing) + end + else + write_multipart_chunk(y, boundary: boundary, key: key, val: val, closing: closing) + end + end + # @api private # # https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.1.md#special-considerations-for-multipart-content @@ -543,7 +566,6 @@ def encode_query_params(query) # # @return [Array(String, Enumerable)] private def encode_multipart_streaming(body) - # rubocop:disable Style/CaseEquality # RFC 1521 Section 7.2.1 says we should have 70 char maximum for boundary length boundary = SecureRandom.urlsafe_base64(46) @@ -552,14 +574,7 @@ def encode_query_params(query) case body in Hash body.each do |key, val| - case val - in Array if val.all? { primitive?(_1) || OpenAI::Internal::Type::FileInput === _1 } - val.each do |v| - write_multipart_chunk(y, boundary: boundary, key: key, val: v, closing: closing) - end - else - write_multipart_chunk(y, boundary: boundary, key: key, val: val, closing: closing) - end + write_multipart_value(y, boundary: boundary, key: key, val: val, closing: closing) end else write_multipart_chunk(y, boundary: boundary, key: nil, val: body, closing: closing) @@ -569,7 +584,6 @@ def encode_query_params(query) fused_io = fused_enum(strio) { closing.each(&:call) } [boundary, fused_io] - # rubocop:enable Style/CaseEquality end # @api private diff --git a/rbi/openai/internal/util.rbi b/rbi/openai/internal/util.rbi index 27538fbd..dc53bbee 100644 --- a/rbi/openai/internal/util.rbi +++ b/rbi/openai/internal/util.rbi @@ -306,6 +306,11 @@ module OpenAI ) end + # @api private + sig { params(value: T.any(Pathname, String, Symbol)).returns(String) } + private def escape_multipart_header_param(value) + end + # @api private sig do params( @@ -319,6 +324,19 @@ module OpenAI private def write_multipart_chunk(y, boundary:, key:, val:, closing:) end + # @api private + sig do + params( + y: Enumerator::Yielder, + boundary: String, + key: T.any(Symbol, String), + val: T.anything, + closing: T::Array[T.proc.void] + ).void + end + private def write_multipart_value(y, boundary:, key:, val:, closing:) + end + # @api private # # https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.1.md#special-considerations-for-multipart-content diff --git a/sig/openai/internal/util.rbs b/sig/openai/internal/util.rbs index 5a5ebdd6..286f83cd 100644 --- a/sig/openai/internal/util.rbs +++ b/sig/openai/internal/util.rbs @@ -111,6 +111,10 @@ module OpenAI ?content_type: String? ) -> void + def self?.escape_multipart_header_param: ( + Pathname | String | Symbol value + ) -> String + def self?.write_multipart_chunk: ( Enumerator::Yielder y, boundary: String, @@ -119,6 +123,14 @@ module OpenAI closing: ::Array[^-> void] ) -> void + def self?.write_multipart_value: ( + Enumerator::Yielder y, + boundary: String, + key: Symbol | String, + val: top, + closing: ::Array[^-> void] + ) -> void + def self?.encode_multipart_streaming: ( top body ) -> [String, Enumerable[String]] diff --git a/test/openai/internal/util_test.rb b/test/openai/internal/util_test.rb index 829b04f9..466b4aef 100644 --- a/test/openai/internal/util_test.rb +++ b/test/openai/internal/util_test.rb @@ -284,12 +284,116 @@ def test_multipart_filename_encoding_with_binary_content assert_includes(body, "filename=\"\u00E9.png\"".b) end + def test_multipart_field_name_quoting + _headers, stream = OpenAI::Internal::Util.encode_content( + {"content-type" => "multipart/form-data"}, + {"a \"b\"\\c\r\nEvil: 1" => "x"} + ) + body = stream.respond_to?(:read) ? stream.read : stream.to_a.join + + assert_includes(body, %q(name="a \"b\"\\\\cEvil: 1")) + refute_includes(body, "\r\nEvil:") + end + + def test_primitive_arrays_use_bracketed_field_names + body, = OpenAI::Audio::TranscriptionCreateParams.dump_request( + file: OpenAI::FilePart.new("audio", filename: "audio.wav"), + model: :"whisper-1", + timestamp_granularities: [:word, :segment] + ) + encoded = OpenAI::Internal::Util.encode_content( + {"content-type" => "multipart/form-data"}, + body + ) + cgi = FakeCGI.new(*encoded) + + assert_equal(%w[word segment], cgi.params.fetch("timestamp_granularities[]")) + refute_includes(cgi.params, "timestamp_granularities") + end + + def test_file_arrays_use_bracketed_field_names + files = [ + OpenAI::FilePart.new(StringIO.new("a"), filename: "a.png"), + OpenAI::FilePart.new(StringIO.new("b"), filename: "b.png") + ] + body, = OpenAI::ImageEditParams.dump_request(image: files, prompt: "Edit both images") + encoded = OpenAI::Internal::Util.encode_content( + {"content-type" => "multipart/form-data"}, + body + ) + parts = FakeCGI.new(*encoded).params + + assert_equal(["image[]"], parts.keys.grep(/^image/)) + assert_equal(%w[a.png b.png], parts.fetch("image[]").map(&:original_filename)) + assert_equal(%w[a b], parts.fetch("image[]").map(&:read)) + end + + def test_scalar_files_keep_unbracketed_field_names + body, = OpenAI::ImageEditParams.dump_request( + image: OpenAI::FilePart.new(StringIO.new("image"), filename: "image.png"), + prompt: "Edit one image" + ) + encoded = OpenAI::Internal::Util.encode_content({"content-type" => "multipart/form-data"}, body) + parts = FakeCGI.new(*encoded).params + + assert_equal(["image"], parts.keys.grep(/^image/)) + assert_equal(["image.png"], parts.fetch("image").map(&:original_filename)) + assert_equal(["image"], parts.fetch("image").map(&:read)) + end + + def test_generated_nested_values_use_bracket_notation + body, = OpenAI::FileCreateParams.dump_request( + file: OpenAI::FilePart.new("{}", filename: "batch.jsonl"), + purpose: :batch, + expires_after: {anchor: :created_at, seconds: 3600} + ) + encoded = OpenAI::Internal::Util.encode_content({"content-type" => "multipart/form-data"}, body) + parts = FakeCGI.new(*encoded).params + + assert_equal(["created_at"], parts.fetch("expires_after[anchor]")) + assert_equal(["3600"], parts.fetch("expires_after[seconds]")) + refute_includes(parts, "expires_after") + end + + def test_nested_values_use_bracket_notation + encoded = OpenAI::Internal::Util.encode_content( + {"content-type" => "multipart/form-data"}, + { + expires_after: {anchor: :created_at, seconds: 3600}, + items: [ + {name: "first", tags: %w[a b]}, + {name: "second", tags: %w[c]} + ] + } + ) + cgi = FakeCGI.new(*encoded) + + assert_equal( + { + "expires_after[anchor]" => ["created_at"], + "expires_after[seconds]" => ["3600"], + "items[][name]" => %w[first second], + "items[][tags][]" => %w[a b c] + }, + cgi.params + ) + end + + def test_empty_collections_are_omitted + encoded = OpenAI::Internal::Util.encode_content( + {"content-type" => "multipart/form-data"}, + {empty_array: [], empty_hash: {}, nested: {empty: []}, present: 1} + ) + cgi = FakeCGI.new(*encoded) + + assert_equal({"present" => ["1"]}, cgi.params) + end + def test_hash_encode headers = {"content-type" => "multipart/form-data"} cases = { {a: 2, b: 3} => {"a" => "2", "b" => "3"}, {a: 2, b: nil} => {"a" => "2", "b" => "null"}, - {a: 2, b: [1, 2, 3]} => {"a" => "2", "b" => "1"}, {strio: StringIO.new("a")} => {"strio" => "a"}, {strio: OpenAI::FilePart.new("a")} => {"strio" => "a"}, {pathname: Pathname(__FILE__)} => {"pathname" => -> { _1.read in /^class OpenAI/ }},