Merge pull request #823 from jnunemaker/mixed-encodings

Force binary encoding throughout

Author: jnunemaker

lib/httparty/request/body.rb

@@ -42,20 +42,20 @@ def multipart?
       def generate_multipart
         normalized_params = params.flat_map { |key, value| HashConversions.normalize_keys(key, value) }
 
-        multipart = normalized_params.inject(''.dup) do |memo, (key, value)|
-          memo << "--#{boundary}#{NEWLINE}"
-          memo << %(Content-Disposition: form-data; name="#{key}")
+        multipart = normalized_params.inject(''.b) do |memo, (key, value)|
+          memo << "--#{boundary}#{NEWLINE}".b
+          memo << %(Content-Disposition: form-data; name="#{key}").b
           # value.path is used to support ActionDispatch::Http::UploadedFile
           # https://github.com/jnunemaker/httparty/pull/585
-          memo << %(; filename="#{file_name(value).gsub(/["\r\n]/, MULTIPART_FORM_DATA_REPLACEMENT_TABLE)}") if file?(value)
-          memo << NEWLINE
-          memo << "Content-Type: #{content_type(value)}#{NEWLINE}" if file?(value)
-          memo << NEWLINE
+          memo << %(; filename="#{file_name(value).gsub(/["\r\n]/, MULTIPART_FORM_DATA_REPLACEMENT_TABLE)}").b if file?(value)
+          memo << NEWLINE.b
+          memo << "Content-Type: #{content_type(value)}#{NEWLINE}".b if file?(value)
+          memo << NEWLINE.b
           memo << content_body(value)
-          memo << NEWLINE
+          memo << NEWLINE.b
         end
 
-        multipart << "--#{boundary}--#{NEWLINE}"
+        multipart << "--#{boundary}--#{NEWLINE}".b
       end
 
       def has_file?(value)
@@ -83,11 +83,12 @@ def normalize_query(query)
       def content_body(object)
         if file?(object)
           object = (file = object).read
-          object.force_encoding(Encoding::UTF_8) if object.respond_to?(:force_encoding)
+          object.force_encoding(Encoding::BINARY) if object.respond_to?(:force_encoding)
           file.rewind if file.respond_to?(:rewind)
+          object.to_s
+        else
+          object.to_s.b
         end
-
-        object.to_s
       end
 
       def content_type(object)

spec/httparty/request/body_spec.rb

@@ -40,7 +40,7 @@
         let(:expected_file_contents) { "GIF89a\u0001\u0000\u0001\u0000\u0000\xFF\u0000,\u0000\u0000\u0000\u0000\u0001\u0000\u0001\u0000\u0000\u0002\u0000;" }
         let(:expected_content_type) { 'image/gif' }
         let(:multipart_params) do
-          "--------------------------c772861a5109d5ef\r\n" \
+          ("--------------------------c772861a5109d5ef\r\n" \
           "Content-Disposition: form-data; name=\"user[avatar]\"; filename=\"#{expected_file_name}\"\r\n" \
           "Content-Type: #{expected_content_type}\r\n" \
           "\r\n" \
@@ -57,7 +57,7 @@
           "Content-Disposition: form-data; name=\"user[enabled]\"\r\n" \
           "\r\n" \
           "true\r\n" \
-          "--------------------------c772861a5109d5ef--\r\n"
+          "--------------------------c772861a5109d5ef--\r\n").b
         end
 
         it { is_expected.to eq multipart_params }
@@ -76,7 +76,7 @@
             }
           end
           let(:multipart_params) do
-            "--------------------------c772861a5109d5ef\r\n" \
+            ("--------------------------c772861a5109d5ef\r\n" \
             "Content-Disposition: form-data; name=\"user[first_name]\"\r\n" \
             "\r\n" \
             "John\r\n" \
@@ -88,7 +88,7 @@
             "Content-Disposition: form-data; name=\"user[enabled]\"\r\n" \
             "\r\n" \
             "true\r\n" \
-            "--------------------------c772861a5109d5ef--\r\n"
+            "--------------------------c772861a5109d5ef--\r\n").b
           end
 
           it { is_expected.to eq multipart_params }
@@ -122,7 +122,7 @@
             }
           end
           let(:multipart_params) do
-            "--------------------------c772861a5109d5ef\r\n" \
+            ("--------------------------c772861a5109d5ef\r\n" \
             "Content-Disposition: form-data; name=\"user[attachment_file]\"; filename=\"#{expected_file_name}\"\r\n" \
             "Content-Type: text/plain\r\n" \
             "\r\n" \
@@ -131,7 +131,7 @@
             "Content-Disposition: form-data; name=\"user[enabled]\"\r\n" \
             "\r\n" \
             "true\r\n" \
-            "--------------------------c772861a5109d5ef--\r\n"
+            "--------------------------c772861a5109d5ef--\r\n").b
           end
 
           it { is_expected.to eq multipart_params }
@@ -148,7 +148,24 @@
             }
           end
 
-          it { expect { subject }.not_to raise_error }
+          it 'does not raise encoding errors' do
+            expect { subject }.not_to raise_error
+          end
+
+          it 'produces valid binary multipart body' do
+            result = subject
+            expect(result.encoding).to eq(Encoding::BINARY)
+            expect(result).to include("Jöhn Döé".b)
+          end
+
+          it 'concatenates binary file data with UTF-8 text without corruption' do
+            result = subject
+            # Should contain both the UTF-8 user field and binary GIF data
+            expect(result).to include('Content-Disposition: form-data; name="user"'.b)
+            expect(result).to include("Jöhn Döé".b)
+            expect(result).to include('Content-Disposition: form-data; name="avatar"'.b)
+            expect(result).to include("GIF89a".b) # GIF file header
+          end
         end
       end
     end