Fix large attachment handling with string keys and custom content_ids

lib/nylas/handler/http_client.rb

@@ -188,10 +188,19 @@ def file_upload?(payload)
       # Check if payload was prepared by FileUtils.build_form_request for multipart uploads
       # This handles binary content attachments that are strings with added singleton methods
       has_message_field = payload.key?("message") && payload["message"].is_a?(String)
-      has_attachment_fields = payload.keys.any? { |key| key.is_a?(String) && key.match?(/^file\d+$/) }
 
-      # If we have both a "message" field and "file{N}" fields, this indicates
-      # the payload was prepared by FileUtils.build_form_request for multipart upload
+      # Check for attachment fields - these can have custom content_id values (not just "file{N}")
+      # FileUtils.build_form_request creates entries with string values that have singleton methods
+      # like original_filename and content_type defined on them
+      has_attachment_fields = payload.any? do |key, value|
+        next false unless key.is_a?(String) && key != "message"
+        # Check if the value is a string with attachment-like singleton methods
+        # (original_filename or content_type), which indicates it's a file content
+        value.is_a?(String) && (value.respond_to?(:original_filename) || value.respond_to?(:content_type))
+      end
+
+      # If we have both a "message" field and attachment fields with file metadata,
+      # this indicates the payload was prepared by FileUtils.build_form_request
       has_message_field && has_attachment_fields
     end
 

lib/nylas/utils/file_utils.rb

@@ -28,12 +28,13 @@ def self.build_form_request(request_body)
 
       attachments.each_with_index do |attachment, index|
         file = attachment[:content] || attachment["content"]
+        file_path = attachment[:file_path] || attachment["file_path"]
         if file.respond_to?(:closed?) && file.closed?
-          unless attachment[:file_path]
+          unless file_path
             raise ArgumentError, "The file at index #{index} is closed and no file_path was provided."
           end
 
-          file = File.open(attachment[:file_path], "rb")
+          file = File.open(file_path, "rb")
         end
 
         # Setting original filename and content type if available. See rest-client#lib/restclient/payload.rb
@@ -87,7 +88,9 @@ def self.handle_message_payload(request_body)
 
       # Use form data only if the attachment size is greater than 3mb
       attachments = payload[:attachments]
-      attachment_size = attachments&.sum { |attachment| attachment[:size] || 0 } || 0
+      # Support both string and symbol keys for attachment size to handle
+      # user-provided hashes that may use either key type
+      attachment_size = attachments&.sum { |attachment| attachment[:size] || attachment["size"] || 0 } || 0
 
       # Handle the attachment encoding depending on the size
       if attachment_size >= FORM_DATA_ATTACHMENT_SIZE

spec/nylas/handler/http_client_spec.rb

@@ -291,6 +291,64 @@ class TestHttpClient
 
       expect(http_client.send(:file_upload?, payload)).to be false
     end
+
+    # Bug fix tests: handle custom content_id values
+    context "when attachments use custom content_id values" do
+      it "detects file uploads with custom content_id values" do
+        # This test reproduces the bug where custom content_id values
+        # like "my-attachment" weren't being detected as file uploads
+        mock_file = instance_double("file")
+        allow(mock_file).to receive(:respond_to?).with(:read).and_return(true)
+
+        payload = {
+          "message" => '{"subject":"test"}',
+          "my-custom-attachment" => mock_file
+        }
+
+        expect(http_client.send(:file_upload?, payload)).to be true
+      end
+
+      it "detects file uploads with alphanumeric content_id values" do
+        mock_file = instance_double("file")
+        allow(mock_file).to receive(:respond_to?).with(:read).and_return(true)
+
+        payload = {
+          "message" => '{"subject":"test"}',
+          "attachment_001" => mock_file,
+          "document_pdf" => mock_file
+        }
+
+        expect(http_client.send(:file_upload?, payload)).to be true
+      end
+
+      it "detects file uploads with content_id containing special characters" do
+        mock_file = instance_double("file")
+        allow(mock_file).to receive(:respond_to?).with(:read).and_return(true)
+
+        payload = {
+          "message" => '{"subject":"test"}',
+          "inline-image-123" => mock_file
+        }
+
+        expect(http_client.send(:file_upload?, payload)).to be true
+      end
+
+      it "detects binary string attachments with custom content_id values" do
+        # When FileUtils.build_form_request passes file content as strings
+        # with singleton methods, file_upload? should still detect them
+        binary_content = "binary file content".dup
+        binary_content.define_singleton_method(:original_filename) { "test.bin" }
+        binary_content.define_singleton_method(:content_type) { "application/octet-stream" }
+
+        payload = {
+          "message" => '{"subject":"test"}',
+          "my-inline-image" => binary_content
+        }
+
+        # Currently this fails because the pattern only matches /^file\d+$/
+        expect(http_client.send(:file_upload?, payload)).to be true
+      end
+    end
   end
 
   describe "#execute" do

spec/nylas/utils/file_utils_spec.rb

@@ -241,6 +241,70 @@
   describe "#handle_message_payload" do
     let(:mock_file) { instance_double("file") }
 
+    # Bug fix tests: handle string keys in attachment hashes
+    context "when attachment hashes use string keys" do
+      it "returns form data when attachment size (string key) is greater than 3MB" do
+        # This test reproduces the bug where users pass attachment hashes with string keys
+        # The size calculation was only checking symbol keys, causing large attachments
+        # to be incorrectly sent as JSON instead of multipart form data
+        large_attachment = {
+          "size" => 5_400_000,  # 5.4MB - using string key
+          "content" => mock_file,
+          "filename" => "large_file.txt",
+          "content_type" => "text/plain"
+        }
+        request_body = { attachments: [large_attachment] }
+
+        allow(mock_file).to receive(:read).and_return("file content")
+        allow(File).to receive(:size).and_return(large_attachment["size"])
+
+        payload, opened_files = described_class.handle_message_payload(request_body)
+
+        expect(payload).to include("multipart" => true)
+        expect(opened_files).to include(mock_file)
+      end
+
+      it "returns form data when attachment size (string key) with string top-level keys is greater than 3MB" do
+        # Test with completely string-keyed request body
+        large_attachment = {
+          "size" => 5_400_000,
+          "content" => mock_file,
+          "filename" => "large_file.txt",
+          "content_type" => "text/plain"
+        }
+        request_body = { "attachments" => [large_attachment] }
+
+        allow(mock_file).to receive(:read).and_return("file content")
+        allow(File).to receive(:size).and_return(large_attachment["size"])
+
+        payload, opened_files = described_class.handle_message_payload(request_body)
+
+        expect(payload).to include("multipart" => true)
+        expect(opened_files).to include(mock_file)
+      end
+
+      it "handles mixed string/symbol keys in attachment hashes for size calculation" do
+        # Test with multiple attachments having different key styles
+        attachment1 = {
+          "size" => 2_000_000,  # String key
+          "content" => mock_file
+        }
+        attachment2 = {
+          size: 2_000_000,  # Symbol key
+          content: mock_file
+        }
+        request_body = { attachments: [attachment1, attachment2] }
+
+        allow(mock_file).to receive(:read).and_return("file content")
+        allow(File).to receive(:size).and_return(2_000_000)
+
+        # Total is 4MB, should trigger multipart
+        payload, _opened_files = described_class.handle_message_payload(request_body)
+
+        expect(payload).to include("multipart" => true)
+      end
+    end
+
     it "returns form data when attachment size is greater than 3MB" do
       large_attachment = {
         size: 4 * 1024 * 1024,