Fix UTF-8 encoding issue with binary attachment content in JSON serialization

- Add normalize_json_encodings! method to handle binary attachment content
- Automatically base64 encode binary strings before JSON serialization
- Fixes 'source sequence is illegal/malformed utf-8' error when sending
  attachments with raw binary content via JSON (non-multipart) requests
- Preserves backward compatibility with both symbol and string keys
- Resolves issue #528

Author: AaronDDM

lib/nylas/handler/http_client.rb

@@ -103,6 +103,7 @@ def build_request(
       is_multipart = !payload.nil? && (payload["multipart"] || payload[:multipart])
 
       if !payload.nil? && !is_multipart
+        normalize_json_encodings!(payload)
         payload = payload&.to_json
         resulting_headers["Content-type"] = "application/json"
       elsif is_multipart
@@ -146,39 +147,24 @@ def httparty_execute(method:, url:, headers:, payload:, timeout:)
         timeout: timeout
       }
 
-      temp_files_to_cleanup = []
-
-      begin
-        # Handle multipart uploads
-        if payload.is_a?(Hash) && file_upload?(payload)
-          options[:multipart] = true
-          options[:body], temp_files_to_cleanup = prepare_multipart_payload(payload)
-        elsif payload
-          options[:body] = payload
-        end
+      # Handle multipart uploads
+      if payload.is_a?(Hash) && file_upload?(payload)
+        options[:multipart] = true
+        options[:body] = prepare_multipart_payload(payload)
+      elsif payload
+        options[:body] = payload
+      end
 
-        response = HTTParty.send(method, url, options)
+      response = HTTParty.send(method, url, options)
 
-        # Create a compatible response object that mimics RestClient::Response
-        result = create_response_wrapper(response)
+      # Create a compatible response object that mimics RestClient::Response
+      result = create_response_wrapper(response)
 
-        # Call the block with the response in the same format as rest-client
-        if block_given?
-          yield response, nil, result
-        else
-          response
-        end
-      ensure
-        # Clean up any temporary files we created
-        temp_files_to_cleanup.each do |tempfile|
-          tempfile.close unless tempfile.closed?
-          begin
-            tempfile.unlink
-          rescue StandardError
-            nil
-          end
-          # Don't fail if file is already deleted
-        end
+      # Call the block with the response in the same format as rest-client
+      if block_given?
+        yield response, nil, result
+      else
+        response
       end
     end
 
@@ -241,8 +227,7 @@ def prepare_multipart_payload(payload)
         modified_payload[key] = string_io
       end
 
-      # Return modified payload and empty array (no temp files to cleanup)
-      [modified_payload, []]
+      modified_payload
     end
 
     # Normalize string encodings in multipart payload to prevent HTTParty encoding conflicts
@@ -257,6 +242,31 @@ def normalize_multipart_encodings!(payload)
       end
     end
 
+    # Normalize JSON encodings for attachment content to ensure binary data is base64 encoded.
+    # This handles cases where users pass raw binary content directly instead of file objects.
+    def normalize_json_encodings!(payload)
+      return unless payload.is_a?(Hash)
+
+      # Handle attachment content encoding for JSON serialization
+      attachments = payload[:attachments] || payload["attachments"]
+      return unless attachments
+
+      attachments.each do |attachment|
+        content = attachment[:content] || attachment["content"]
+        next unless content.is_a?(String)
+
+        # If content appears to be binary (non-UTF-8), base64 encode it
+        next unless content.encoding == Encoding::ASCII_8BIT || !content.valid_encoding?
+
+        encoded_content = Base64.strict_encode64(content)
+        if attachment.key?(:content)
+          attachment[:content] = encoded_content
+        else
+          attachment["content"] = encoded_content
+        end
+      end
+    end
+
     # Create a StringIO object that behaves more like a File for HTTParty compatibility
     def create_file_like_stringio(content)
       # Content is already normalized to ASCII-8BIT by normalize_multipart_encodings!

spec/nylas/handler/http_client_spec.rb

@@ -656,27 +656,24 @@ class TestHttpClient
   describe "#prepare_multipart_payload" do
     it "normalizes message payloads to ASCII-8BIT encoding for HTTParty compatibility" do
       payload = { "message" => "Hello World" }
-      result, temp_files = http_client.send(:prepare_multipart_payload, payload)
+      result = http_client.send(:prepare_multipart_payload, payload)
 
       expect(result["message"]).to eq("Hello World")
       expect(result["message"].encoding).to eq(Encoding::ASCII_8BIT)
-      expect(temp_files).to be_empty
     end
 
     it "leaves non-message fields unchanged" do
       payload = { "other_field" => "value", "data" => "content" }
-      result, temp_files = http_client.send(:prepare_multipart_payload, payload)
+      result = http_client.send(:prepare_multipart_payload, payload)
 
       expect(result).to eq(payload)
-      expect(temp_files).to be_empty
     end
 
     it "handles payloads without message field" do
       payload = { "data" => "content" }
-      result, temp_files = http_client.send(:prepare_multipart_payload, payload)
+      result = http_client.send(:prepare_multipart_payload, payload)
 
       expect(result).to eq(payload)
-      expect(temp_files).to be_empty
     end
 
     it "converts binary content attachments to StringIO objects" do
@@ -690,7 +687,7 @@ class TestHttpClient
         "other_field" => "value"
       }
 
-      result, temp_files = http_client.send(:prepare_multipart_payload, payload)
+      result = http_client.send(:prepare_multipart_payload, payload)
 
       # Message should be preserved
       expect(result["message"]).to eq('{"subject":"test"}')
@@ -706,9 +703,6 @@ class TestHttpClient
 
       # Other fields should be unchanged
       expect(result["other_field"]).to eq("value")
-
-      # No temp files need cleanup with StringIO
-      expect(temp_files).to be_empty
     end
 
     it "handles multiple binary content attachments" do
@@ -724,7 +718,7 @@ class TestHttpClient
         "file1" => content2
       }
 
-      result, temp_files = http_client.send(:prepare_multipart_payload, payload)
+      result = http_client.send(:prepare_multipart_payload, payload)
 
       expect(result["file0"]).to be_a(StringIO)
       expect(result["file0"].read).to eq("content 1")
@@ -733,9 +727,6 @@ class TestHttpClient
       expect(result["file1"]).to be_a(StringIO)
       expect(result["file1"].read).to eq("content 2")
       expect(result["file1"].original_filename).to eq("file2.txt")
-
-      # No temp files need cleanup with StringIO
-      expect(temp_files).to be_empty
     end
   end
 end