diff --git a/apps/sim/lib/uploads/providers/s3/client.test.ts b/apps/sim/lib/uploads/providers/s3/client.test.ts index 48c017d2cb..75eea9a3dd 100644 --- a/apps/sim/lib/uploads/providers/s3/client.test.ts +++ b/apps/sim/lib/uploads/providers/s3/client.test.ts @@ -12,6 +12,7 @@ const { mockPutObjectCommand, mockGetObjectCommand, mockDeleteObjectCommand, + mockCompleteMultipartUploadCommand, mockGetSignedUrl, mockEnv, mockS3Config, @@ -51,6 +52,7 @@ const { mockPutObjectCommand: vi.fn().mockImplementation(class {}), mockGetObjectCommand: vi.fn().mockImplementation(class {}), mockDeleteObjectCommand: vi.fn().mockImplementation(class {}), + mockCompleteMultipartUploadCommand: vi.fn().mockImplementation(class {}), mockGetSignedUrl: vi.fn(), mockEnv, } @@ -61,6 +63,7 @@ vi.mock('@aws-sdk/client-s3', () => ({ PutObjectCommand: mockPutObjectCommand, GetObjectCommand: mockGetObjectCommand, DeleteObjectCommand: mockDeleteObjectCommand, + CompleteMultipartUploadCommand: mockCompleteMultipartUploadCommand, })) vi.mock('@aws-sdk/s3-request-presigner', () => ({ @@ -92,6 +95,7 @@ vi.mock('@/lib/uploads/config', () => ({ })) import { + completeS3MultipartUpload, deleteFromS3, downloadFromS3, getPresignedUrl, @@ -398,4 +402,70 @@ describe('S3 Client', () => { }) }) }) + + describe('completeS3MultipartUpload fallback location', () => { + const parts = [{ ETag: 'etag-1', PartNumber: 1 }] + + it('uses the SDK-provided Location when present', async () => { + mockSend.mockResolvedValueOnce({ Location: 'https://provided.example.com/object' }) + + const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts) + + expect(result.location).toBe('https://provided.example.com/object') + expect(result.key).toBe('kb/uuid-file.txt') + expect(result.path).toBe('/api/files/serve/kb%2Fuuid-file.txt') + }) + + it('falls back to an AWS virtual-hosted URL when Location is absent', async () => { + mockSend.mockResolvedValueOnce({}) + + const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts) + + expect(result.location).toBe( + 'https://test-kb-bucket.s3.test-region.amazonaws.com/kb/uuid-file.txt' + ) + }) + + it('builds a path-style fallback URL for a custom endpoint with forcePathStyle', async () => { + mockS3Config.endpoint = 'https://minio.example.com' + mockS3Config.forcePathStyle = true + mockSend.mockResolvedValueOnce({}) + + const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts) + + expect(result.location).toBe('https://minio.example.com/test-kb-bucket/kb/uuid-file.txt') + }) + + it('builds a virtual-hosted fallback URL for a custom endpoint without forcePathStyle', async () => { + mockS3Config.endpoint = 'https://account.r2.cloudflarestorage.com' + mockS3Config.forcePathStyle = false + mockSend.mockResolvedValueOnce({}) + + const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts) + + expect(result.location).toBe( + 'https://test-kb-bucket.account.r2.cloudflarestorage.com/kb/uuid-file.txt' + ) + }) + + it('strips a trailing slash from the custom endpoint before appending the key', async () => { + mockS3Config.endpoint = 'https://minio.example.com/' + mockS3Config.forcePathStyle = true + mockSend.mockResolvedValueOnce({}) + + const result = await completeS3MultipartUpload('kb/uuid-file.txt', 'upload-1', parts) + + expect(result.location).toBe('https://minio.example.com/test-kb-bucket/kb/uuid-file.txt') + }) + + it('percent-encodes special characters per path segment, preserving slashes', async () => { + mockSend.mockResolvedValueOnce({}) + + const result = await completeS3MultipartUpload('kb/uuid-my file.txt', 'upload-1', parts) + + expect(result.location).toBe( + 'https://test-kb-bucket.s3.test-region.amazonaws.com/kb/uuid-my%20file.txt' + ) + }) + }) }) diff --git a/apps/sim/lib/uploads/providers/s3/client.ts b/apps/sim/lib/uploads/providers/s3/client.ts index 42b0c62fac..7ddae7bbf8 100644 --- a/apps/sim/lib/uploads/providers/s3/client.ts +++ b/apps/sim/lib/uploads/providers/s3/client.ts @@ -394,18 +394,25 @@ export async function getS3MultipartPartUrls( * addressing mode — path-style for MinIO/Ceph (`forcePathStyle`), virtual-hosted * (bucket as a subdomain) for R2 and friends. Falls back to the AWS * virtual-hosted host when no custom endpoint is set. + * + * The key is percent-encoded per path segment (preserving `/` separators) so + * keys containing spaces or reserved characters still yield a valid URL. */ function buildObjectFallbackUrl(bucket: string, region: string, key: string): string { + const encodedKey = key + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') if (S3_CONFIG.endpoint) { const base = S3_CONFIG.endpoint.replace(/\/+$/, '') if (S3_CONFIG.forcePathStyle) { - return `${base}/${bucket}/${key}` + return `${base}/${bucket}/${encodedKey}` } const url = new URL(base) url.hostname = `${bucket}.${url.hostname}` - return `${url.origin}/${key}` + return `${url.origin}/${encodedKey}` } - return `https://${bucket}.s3.${region}.amazonaws.com/${key}` + return `https://${bucket}.s3.${region}.amazonaws.com/${encodedKey}` } /**