fix: encode multipart array and nested fields

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<Proc>]
+        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<String>)]
         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

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

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]]

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/ }},