From 7aa254edeb5d74cb1734d3500d95b5f3a9c356be Mon Sep 17 00:00:00 2001 From: Aaron de Mello Date: Tue, 3 Feb 2026 15:36:03 -0500 Subject: [PATCH] Fix large attachment handling with string keys and custom content_ids This fixes two bugs that caused attachments over 3MB (e.g., 5.4MB) to fail with "sending a json request as multipart/form-data" errors: 1. Size calculation only checked symbol keys (`:size`) but users may pass attachment hashes with string keys (`"size"`). This caused large files to be incorrectly sent as JSON instead of multipart form-data. 2. The `file_upload?` detection only matched keys like `file0`, `file1`, but users can specify custom `content_id` values. This caused multipart detection to fail for attachments with custom content IDs. Fixes: - Support both string and symbol keys when calculating attachment size - Detect attachments by checking for singleton methods (original_filename, content_type) rather than key name patterns --- lib/nylas/handler/http_client.rb | 15 ++++-- lib/nylas/utils/file_utils.rb | 9 ++-- spec/nylas/handler/http_client_spec.rb | 58 +++++++++++++++++++++++ spec/nylas/utils/file_utils_spec.rb | 64 ++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 6 deletions(-) diff --git a/lib/nylas/handler/http_client.rb b/lib/nylas/handler/http_client.rb index 0eb14ec5..e18d920f 100644 --- a/lib/nylas/handler/http_client.rb +++ b/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 diff --git a/lib/nylas/utils/file_utils.rb b/lib/nylas/utils/file_utils.rb index 5d90ad5b..1c513936 100644 --- a/lib/nylas/utils/file_utils.rb +++ b/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 diff --git a/spec/nylas/handler/http_client_spec.rb b/spec/nylas/handler/http_client_spec.rb index 980f8c54..c8ca8db9 100644 --- a/spec/nylas/handler/http_client_spec.rb +++ b/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 diff --git a/spec/nylas/utils/file_utils_spec.rb b/spec/nylas/utils/file_utils_spec.rb index fbc16ec6..ea9f7423 100644 --- a/spec/nylas/utils/file_utils_spec.rb +++ b/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,