fix: copy source with query param

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/storage/protocols/s3/copy-source-parser.ts

@@ -1,13 +1,38 @@
 import { ERRORS } from '@internal/errors'
 
+const VERSION_ID_QUERY_DELIMITER = '?versionId='
+
+function splitCopySourceVersion(copySource: string): {
+  encodedPath: string
+  sourceVersion?: string
+} {
+  const versionQueryIdx = copySource.lastIndexOf(VERSION_ID_QUERY_DELIMITER)
+
+  if (versionQueryIdx === -1) {
+    return {
+      encodedPath: copySource,
+    }
+  }
+
+  const sourceVersion = copySource.slice(versionQueryIdx + VERSION_ID_QUERY_DELIMITER.length)
+  if (!sourceVersion) {
+    throw ERRORS.InvalidParameter('CopySource')
+  }
+
+  return {
+    encodedPath: copySource.slice(0, versionQueryIdx),
+    sourceVersion,
+  }
+}
+
 export function parseCopySource(copySource: string): {
   bucketName: string
   objectKey: string
   sourceVersion?: string
 } {
   const normalizedCopySource = copySource.startsWith('/') ? copySource.slice(1) : copySource
-  const [encodedPath, ...queryParts] = normalizedCopySource.split('?')
-  const queryParams = queryParts.join('?')
+  // Preserve raw '?' characters in partially encoded keys and only peel off a trailing versionId suffix.
+  const { encodedPath, sourceVersion } = splitCopySourceVersion(normalizedCopySource)
 
   let decodedPath = ''
   try {
@@ -16,6 +41,10 @@ export function parseCopySource(copySource: string): {
     throw ERRORS.InvalidParameter('CopySource')
   }
 
+  if (decodedPath.startsWith('/')) {
+    decodedPath = decodedPath.slice(1)
+  }
+
   const separatorIdx = decodedPath.indexOf('/')
   if (separatorIdx <= 0) {
     throw ERRORS.MissingParameter('CopySource')
@@ -27,13 +56,6 @@ export function parseCopySource(copySource: string): {
     throw ERRORS.MissingParameter('CopySource')
   }
 
-  const searchParams = new URLSearchParams(queryParams)
-  const sourceVersion = searchParams.get('versionId') || undefined
-
-  if (searchParams.has('versionId') && !sourceVersion) {
-    throw ERRORS.InvalidParameter('CopySource')
-  }
-
   return {
     bucketName,
     objectKey,

src/test/s3-copy-source-parser.test.ts

@@ -4,6 +4,17 @@ import { ErrorCode, StorageBackendError } from '@internal/errors'
 import { parseCopySource } from '../storage/protocols/s3/copy-source-parser'
 
 describe('parseCopySource', () => {
+  test('preserves raw question marks and hashes in partially encoded keys', () => {
+    const objectKey = 'folder/일이삼?x=1#frag.png'
+    const result = parseCopySource(`bucket/${encodeURI(objectKey)}`)
+
+    expect(result).toEqual({
+      bucketName: 'bucket',
+      objectKey,
+      sourceVersion: undefined,
+    })
+  })
+
   test('preserves question marks in versionId query values', () => {
     const result = parseCopySource(
       'bucket/folder%20one/%EC%9D%BC%EC%9D%B4%EC%82%BC.png?versionId=v1?part=2'
@@ -28,6 +39,16 @@ describe('parseCopySource', () => {
     })
   })
 
+  test('accepts fully URL-encoded leading-slash CopySource values', () => {
+    const result = parseCopySource(encodeURIComponent('/bucket/folder one/일이삼/🙂?#%.png'))
+
+    expect(result).toEqual({
+      bucketName: 'bucket',
+      objectKey: 'folder one/일이삼/🙂?#%.png',
+      sourceVersion: undefined,
+    })
+  })
+
   test('rejects an empty versionId query value', () => {
     expect(() => parseCopySource('bucket/key?versionId=')).toThrow(
       expect.objectContaining<Partial<StorageBackendError>>({

src/test/s3-protocol.test.ts

@@ -2703,6 +2703,68 @@ describe('S3 Protocol', () => {
         expect(listedKeys).toEqual(expect.arrayContaining([sourceKey, destinationKey]))
       })
 
+      it('can copy objects when partially encoded CopySource keeps raw ? and # in the key', async () => {
+        const bucketName = await createBucket(client)
+        const sourceKey = `copy-src-reserved-${randomUUID()}-일이삼?x=1#frag.png`
+        const destinationKey = `copy-dst-reserved-${randomUUID()}-🙂.jpg`
+
+        await client.send(
+          new PutObjectCommand({
+            Bucket: bucketName,
+            Key: sourceKey,
+            Body: Buffer.alloc(1024 * 128),
+          })
+        )
+
+        const copyObjectResp = await client.send(
+          new CopyObjectCommand({
+            Bucket: bucketName,
+            Key: destinationKey,
+            CopySource: encodeURI(`${bucketName}/${sourceKey}`),
+          })
+        )
+        expect(copyObjectResp.$metadata.httpStatusCode).toBe(200)
+
+        const listObjectsResp = await client.send(
+          new ListObjectsV2Command({
+            Bucket: bucketName,
+          })
+        )
+        const listedKeys = (listObjectsResp.Contents || []).map((item) => item.Key)
+        expect(listedKeys).toEqual(expect.arrayContaining([sourceKey, destinationKey]))
+      })
+
+      it('can copy objects using fully URL-encoded leading-slash CopySource', async () => {
+        const bucketName = await createBucket(client)
+        const sourceKey = getUnicodeObjectName()
+        const destinationKey = `copy-dst-leading-encoded-${randomUUID()}-🙂.jpg`
+
+        await client.send(
+          new PutObjectCommand({
+            Bucket: bucketName,
+            Key: sourceKey,
+            Body: Buffer.alloc(1024 * 128),
+          })
+        )
+
+        const copyObjectResp = await client.send(
+          new CopyObjectCommand({
+            Bucket: bucketName,
+            Key: destinationKey,
+            CopySource: encodeURIComponent(`/${bucketName}/${sourceKey}`),
+          })
+        )
+        expect(copyObjectResp.$metadata.httpStatusCode).toBe(200)
+
+        const listObjectsResp = await client.send(
+          new ListObjectsV2Command({
+            Bucket: bucketName,
+          })
+        )
+        const listedKeys = (listObjectsResp.Contents || []).map((item) => item.Key)
+        expect(listedKeys).toEqual(expect.arrayContaining([sourceKey, destinationKey]))
+      })
+
       it('can upload part copy using Unicode keys in CopySource', async () => {
         const bucketName = await createBucket(client)
         const sourceKey = `copy-part-src-${randomUUID()}-일이삼-🙂.jpg`
@@ -2779,6 +2841,44 @@ describe('S3 Protocol', () => {
         expect(listPartsResp.Parts?.length).toBe(1)
       })
 
+      it('can upload part copy when partially encoded CopySource keeps raw ? and # in the key', async () => {
+        const bucketName = await createBucket(client)
+        const sourceKey = `copy-part-src-reserved-${randomUUID()}-일이삼?x=1#frag.png`
+        const destinationKey = `copy-part-dst-reserved-${randomUUID()}-🙂.jpg`
+
+        await uploadFile(client, bucketName, sourceKey, 8)
+
+        const createMultiPartUpload = new CreateMultipartUploadCommand({
+          Bucket: bucketName,
+          Key: destinationKey,
+          ContentType: 'image/jpg',
+          CacheControl: 'max-age=2000',
+        })
+        const createMultipartResp = await client.send(createMultiPartUpload)
+        expect(createMultipartResp.UploadId).toBeTruthy()
+
+        const uploadPartCopyResp = await client.send(
+          new UploadPartCopyCommand({
+            Bucket: bucketName,
+            Key: destinationKey,
+            UploadId: createMultipartResp.UploadId,
+            PartNumber: 1,
+            CopySource: encodeURI(`${bucketName}/${sourceKey}`),
+            CopySourceRange: 'bytes=0-4096',
+          })
+        )
+        expect(uploadPartCopyResp.CopyPartResult?.ETag).toBeTruthy()
+
+        const listPartsResp = await client.send(
+          new ListPartsCommand({
+            Bucket: bucketName,
+            Key: destinationKey,
+            UploadId: createMultipartResp.UploadId,
+          })
+        )
+        expect(listPartsResp.Parts?.length).toBe(1)
+      })
+
       it('should not upload if the name contains invalid characters', async () => {
         const bucketName = await createBucket(client)
         const invalidObjectName = getInvalidObjectName()