From c5b29419dbdd0b3886ba9f545caa062533935a43 Mon Sep 17 00:00:00 2001 From: kptdobe Date: Wed, 3 Dec 2025 10:45:53 +0100 Subject: [PATCH 1/5] fix: CopySource needs to be encoded --- src/storage/object/copy.js | 13 +++++- test/storage/object/copy.test.js | 74 ++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/storage/object/copy.js b/src/storage/object/copy.js index cbacb6f0..533abd0e 100644 --- a/src/storage/object/copy.js +++ b/src/storage/object/copy.js @@ -41,10 +41,21 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) key: sourceKey, }, true); + // Skip if source doesn't exist (e.g., it's a folder without an actual object) + if (source?.status === 404) { + return { $metadata: { httpStatusCode: 404 } }; + } + + // URL-encode the key parts while preserving slashes for S3 folder structure + const encodedSourcePath = `${daCtx.org}/${sourceKey}` + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/'); + const input = { Bucket: daCtx.bucket, Key: `${daCtx.org}/${Key}`, - CopySource: `${daCtx.bucket}/${daCtx.org}/${sourceKey}`, + CopySource: `${daCtx.bucket}/${encodedSourcePath}`, ContentType: source?.contentType || 'application/octet-stream', }; diff --git a/test/storage/object/copy.test.js b/test/storage/object/copy.test.js index 2c5fca40..91f30d01 100644 --- a/test/storage/object/copy.test.js +++ b/test/storage/object/copy.test.js @@ -401,6 +401,80 @@ describe('Object copy', () => { ); }); + it('Copies files with special characters in names', async () => { + const mockS3Client = class { + // eslint-disable-next-line class-methods-use-this + send(command) { + return command; + } + + middlewareStack = { + add: () => {}, + }; + }; + + // Mock getObject to return content type for HEAD requests + const mockGetObject = async (env, { bucket, org, key }, head) => { + if (head && bucket === 'root-bucket' && org === 'myorg') { + if (key === 'mysrc/icon=gift-box, style=two-toned.svg' + || key === 'mysrc/boost saver_img1.jpg' + || key === 'mysrc/file%20with%20encoded.png') { + return { + contentType: 'image/svg+xml', + status: 200, + }; + } + } + return null; + }; + + // eslint-disable-next-line no-shadow + const { copyFile } = await esmock('../../../src/storage/object/copy.js', { + '@aws-sdk/client-s3': { + S3Client: mockS3Client, + }, + '../../../src/storage/object/get.js': { + default: mockGetObject, + }, + }); + + const env = { + dacollab: { + fetch: () => {}, + }, + }; + const daCtx = { + bucket: 'root-bucket', + org: 'myorg', + origin: 'https://test.com', + users: [{ email: 'test@example.com' }], + }; + daCtx.aclCtx = await getAclCtx(env, daCtx.org, daCtx.users, '/'); + const details = { + source: 'mysrc', + destination: 'mydst', + }; + + // Test file with commas, equals signs, and spaces + const resp1 = await copyFile({}, env, daCtx, 'mysrc/icon=gift-box, style=two-toned.svg', details, false); + assert.strictEqual(resp1.constructor.name, 'CopyObjectCommand'); + assert.strictEqual(resp1.input.CopySource, 'root-bucket/myorg/mysrc/icon%3Dgift-box%2C%20style%3Dtwo-toned.svg'); + assert.strictEqual(resp1.input.Key, 'myorg/mydst/icon=gift-box, style=two-toned.svg'); + + // Test file with spaces + const resp2 = await copyFile({}, env, daCtx, 'mysrc/boost saver_img1.jpg', details, false); + assert.strictEqual(resp2.constructor.name, 'CopyObjectCommand'); + assert.strictEqual(resp2.input.CopySource, 'root-bucket/myorg/mysrc/boost%20saver_img1.jpg'); + assert.strictEqual(resp2.input.Key, 'myorg/mydst/boost saver_img1.jpg'); + + // Test file with already-encoded characters + const resp3 = await copyFile({}, env, daCtx, 'mysrc/file%20with%20encoded.png', details, false); + assert.strictEqual(resp3.constructor.name, 'CopyObjectCommand'); + // The %20 should be double-encoded to %2520 + assert.strictEqual(resp3.input.CopySource, 'root-bucket/myorg/mysrc/file%2520with%2520encoded.png'); + assert.strictEqual(resp3.input.Key, 'myorg/mydst/file%20with%20encoded.png'); + }); + it('Copy content when destination already exists', async () => { const error = { $metadata: { httpStatusCode: 412 }, From 7aabc587d6fb304a61260e426c38da74deebb277 Mon Sep 17 00:00:00 2001 From: kptdobe Date: Wed, 3 Dec 2025 13:08:29 +0100 Subject: [PATCH 2/5] chore: cleanup --- src/storage/object/copy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/object/copy.js b/src/storage/object/copy.js index 533abd0e..efe74462 100644 --- a/src/storage/object/copy.js +++ b/src/storage/object/copy.js @@ -25,7 +25,7 @@ import { hasPermission } from '../../utils/auth.js'; const MAX_KEYS = 900; export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) => { - const Key = `${sourceKey.replace(details.source, details.destination)}`; + const Key = sourceKey.replace(details.source, details.destination); if (!hasPermission(daCtx, sourceKey, 'read') || !hasPermission(daCtx, Key, 'write')) { return { From 9f3565743f45194610def3fe8d2f6a66731cd7da Mon Sep 17 00:00:00 2001 From: kptdobe Date: Wed, 3 Dec 2025 13:22:17 +0100 Subject: [PATCH 3/5] chore: extract org --- src/storage/object/copy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/object/copy.js b/src/storage/object/copy.js index efe74462..51b9b7f8 100644 --- a/src/storage/object/copy.js +++ b/src/storage/object/copy.js @@ -47,7 +47,7 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) } // URL-encode the key parts while preserving slashes for S3 folder structure - const encodedSourcePath = `${daCtx.org}/${sourceKey}` + const encodedKey = sourceKey .split('/') .map((segment) => encodeURIComponent(segment)) .join('/'); @@ -55,7 +55,7 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) const input = { Bucket: daCtx.bucket, Key: `${daCtx.org}/${Key}`, - CopySource: `${daCtx.bucket}/${encodedSourcePath}`, + CopySource: `${daCtx.bucket}/${daCtx.org}/${encodedKey}`, ContentType: source?.contentType || 'application/octet-stream', }; From 95f3e0d5d2f4e4fc57b6ac44aef5c45767e8be51 Mon Sep 17 00:00:00 2001 From: kptdobe Date: Wed, 3 Dec 2025 13:26:15 +0100 Subject: [PATCH 4/5] chore: add test for coverage --- test/storage/object/copy.test.js | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/storage/object/copy.test.js b/test/storage/object/copy.test.js index 91f30d01..af770dee 100644 --- a/test/storage/object/copy.test.js +++ b/test/storage/object/copy.test.js @@ -401,6 +401,54 @@ describe('Object copy', () => { ); }); + it('Skips copying when source does not exist (folder without object)', async () => { + const mockS3Client = class { + middlewareStack = { + add: () => {}, + }; + }; + + // Mock getObject to return 404 for a folder that doesn't exist as an object + const mockGetObject = async (env, { bucket, org, key }, head) => { + if (head && bucket === 'root-bucket' && org === 'myorg' && key === 'mysrc/virtual-folder') { + return { + status: 404, + }; + } + return null; + }; + + // eslint-disable-next-line no-shadow + const { copyFile } = await esmock('../../../src/storage/object/copy.js', { + '@aws-sdk/client-s3': { + S3Client: mockS3Client, + }, + '../../../src/storage/object/get.js': { + default: mockGetObject, + }, + }); + + const env = { + dacollab: { + fetch: () => {}, + }, + }; + const daCtx = { + bucket: 'root-bucket', + org: 'myorg', + origin: 'https://test.com', + users: [{ email: 'test@example.com' }], + }; + daCtx.aclCtx = await getAclCtx(env, daCtx.org, daCtx.users, '/'); + const details = { + source: 'mysrc', + destination: 'mydst', + }; + + const resp = await copyFile({}, env, daCtx, 'mysrc/virtual-folder', details, false); + assert.strictEqual(resp.$metadata.httpStatusCode, 404); + }); + it('Copies files with special characters in names', async () => { const mockS3Client = class { // eslint-disable-next-line class-methods-use-this From b93a14372c59e435d2de8e003c4a1964a493dc99 Mon Sep 17 00:00:00 2001 From: kptdobe Date: Wed, 3 Dec 2025 14:20:02 +0100 Subject: [PATCH 5/5] chore: encodeURI is enough --- src/storage/object/copy.js | 8 +------- test/storage/object/copy.test.js | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/storage/object/copy.js b/src/storage/object/copy.js index 51b9b7f8..a7e3d1a8 100644 --- a/src/storage/object/copy.js +++ b/src/storage/object/copy.js @@ -46,16 +46,10 @@ export const copyFile = async (config, env, daCtx, sourceKey, details, isRename) return { $metadata: { httpStatusCode: 404 } }; } - // URL-encode the key parts while preserving slashes for S3 folder structure - const encodedKey = sourceKey - .split('/') - .map((segment) => encodeURIComponent(segment)) - .join('/'); - const input = { Bucket: daCtx.bucket, Key: `${daCtx.org}/${Key}`, - CopySource: `${daCtx.bucket}/${daCtx.org}/${encodedKey}`, + CopySource: `${daCtx.bucket}/${daCtx.org}/${encodeURI(sourceKey)}`, ContentType: source?.contentType || 'application/octet-stream', }; diff --git a/test/storage/object/copy.test.js b/test/storage/object/copy.test.js index af770dee..d385fd1c 100644 --- a/test/storage/object/copy.test.js +++ b/test/storage/object/copy.test.js @@ -506,7 +506,7 @@ describe('Object copy', () => { // Test file with commas, equals signs, and spaces const resp1 = await copyFile({}, env, daCtx, 'mysrc/icon=gift-box, style=two-toned.svg', details, false); assert.strictEqual(resp1.constructor.name, 'CopyObjectCommand'); - assert.strictEqual(resp1.input.CopySource, 'root-bucket/myorg/mysrc/icon%3Dgift-box%2C%20style%3Dtwo-toned.svg'); + assert.strictEqual(resp1.input.CopySource, 'root-bucket/myorg/mysrc/icon=gift-box,%20style=two-toned.svg'); assert.strictEqual(resp1.input.Key, 'myorg/mydst/icon=gift-box, style=two-toned.svg'); // Test file with spaces