From 8a4ec226efd47cd8186d3d38ae6129647d12db48 Mon Sep 17 00:00:00 2001 From: mlatief Date: Mon, 30 Dec 2024 10:30:24 +0200 Subject: [PATCH 01/40] fix: allow Unicode object names This allows all Unicode characters that are also valid in XML 1.0 documents. See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html See: https://www.w3.org/TR/REC-xml/#charsets --- migrations/tenant/57-unicode-object-names.sql | 7 + src/http/plugins/xml.ts | 18 ++- src/internal/database/migrations/types.ts | 3 +- src/internal/errors/codes.ts | 2 +- src/storage/database/knex.ts | 6 +- src/storage/limits.ts | 12 +- src/test/common.ts | 36 +++++ src/test/object.test.ts | 91 ++++++++++- src/test/s3-protocol.test.ts | 146 ++++++++++++++++++ src/test/tus.test.ts | 134 +++++++++++++++- 10 files changed, 446 insertions(+), 9 deletions(-) create mode 100644 migrations/tenant/57-unicode-object-names.sql diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql new file mode 100644 index 000000000..bc88f4090 --- /dev/null +++ b/migrations/tenant/57-unicode-object-names.sql @@ -0,0 +1,7 @@ +ALTER TABLE "storage"."objects" + ADD CONSTRAINT objects_name_check + CHECK ( + name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND POSITION(U&'\FFFE' IN name) = 0 + AND POSITION(U&'\FFFF' IN name) = 0 + ); diff --git a/src/http/plugins/xml.ts b/src/http/plugins/xml.ts index 0101d925d..2a7209f18 100644 --- a/src/http/plugins/xml.ts +++ b/src/http/plugins/xml.ts @@ -6,6 +6,18 @@ import xml from 'xml2js' type XmlParserOptions = { disableContentParser?: boolean; parseAsArray?: string[] } type RequestError = Error & { statusCode?: number } +function decodeXmlNumericCharacterReferences(value: string): string { + return value.replace(/&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, (match: string, rawValue: string) => { + const isHex = rawValue[0].toLowerCase() === 'x' + const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) + if (codePoint > 0x10ffff) { + return match + } + + return String.fromCodePoint(codePoint) + }) +} + function forcePathAsArray(node: unknown, pathSegments: string[]): void { if (pathSegments.length === 0 || node === null || node === undefined) { return @@ -57,7 +69,11 @@ export const xmlParser = fastifyPlugin( { explicitArray: false, trim: true, - valueProcessors: [xml.processors.parseNumbers, xml.processors.parseBooleans], + valueProcessors: [ + decodeXmlNumericCharacterReferences, + xml.processors.parseNumbers, + xml.processors.parseBooleans, + ], }, (err: Error | null, parsed: unknown) => { if (err) { diff --git a/src/internal/database/migrations/types.ts b/src/internal/database/migrations/types.ts index 35bd839c0..a568083d2 100644 --- a/src/internal/database/migrations/types.ts +++ b/src/internal/database/migrations/types.ts @@ -56,4 +56,5 @@ export const DBMigration = { 'drop-index-object-level': 54, 'prevent-direct-deletes': 55, 'fix-optimized-search-function': 56, -} + 'unicode-object-names': 57, +} as const diff --git a/src/internal/errors/codes.ts b/src/internal/errors/codes.ts index 1902daa37..4ab74a538 100644 --- a/src/internal/errors/codes.ts +++ b/src/internal/errors/codes.ts @@ -324,7 +324,7 @@ export const ERRORS = { code: ErrorCode.InvalidKey, resource: key, httpStatusCode: 400, - message: `Invalid key: ${key}`, + message: `Invalid key: ${encodeURIComponent(key)}`, originalError: e, }), diff --git a/src/storage/database/knex.ts b/src/storage/database/knex.ts index 546657109..e3e6915a9 100644 --- a/src/storage/database/knex.ts +++ b/src/storage/database/knex.ts @@ -1145,7 +1145,11 @@ export class DBError extends StorageBackendError implements RenderableError { code: pgError.code, }) default: - return ERRORS.DatabaseError(`database error, code: ${pgError.code}`, pgError).withMetadata({ + const errorMessage = + pgError.code === '23514' && pgError.constraint === 'objects_name_check' + ? 'Invalid object name' + : pgError.message + return ERRORS.DatabaseError(errorMessage, pgError).withMetadata({ query, code: pgError.code, }) diff --git a/src/storage/limits.ts b/src/storage/limits.ts index 35ca85732..6392bb12c 100644 --- a/src/storage/limits.ts +++ b/src/storage/limits.ts @@ -52,9 +52,15 @@ export async function isImageTransformationEnabled(tenantId: string) { * @param key */ export function isValidKey(key: string): boolean { - // only allow s3 safe characters and characters which require special handling for now - // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html - return key.length > 0 && /^(\w|\/|!|-|\.|\*|'|\(|\)| |&|\$|@|=|;|:|\+|,|\?)*$/.test(key) + // Allow any sequence of Unicode characters with UTF-8 encoding, + // except characters not allowed in XML 1.0. + // See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html + // See: https://www.w3.org/TR/REC-xml/#charsets + // + const regex = + /[\0-\x08\x0B\f\x0E-\x1F\uFFFE\uFFFF]|[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?:[^\uD800-\uDBFF]|^)[\uDC00-\uDFFF]/ + + return key.length > 0 && !regex.test(key) } /** diff --git a/src/test/common.ts b/src/test/common.ts index 5cf75d5af..092e23cb5 100644 --- a/src/test/common.ts +++ b/src/test/common.ts @@ -10,6 +10,42 @@ export const adminApp = app({}) const ENV = process.env const projectRoot = path.join(__dirname, '..', '..') +/** + * Should support all Unicode characters with UTF-8 encoding according to AWS S3 object naming guide, including: + * - Safe characters: 0-9 a-z A-Z !-_.*'() + * - Characters that might require special handling: &$@=;/:+,? and Space and ASCII characters \t, \n, and \r. + * - Characters: \{}^%`[]"<>~#| and non-printable ASCII characters (128–255 decimal characters). + * + * The following characters are not allowed: + * - ASCII characters 0x00–0x1F, except 0x09, 0x0A, and 0x0D. + * - Unicode \u{FFFE} and \u{FFFF}. + * - Lone surrogates characters. + * See: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html + * See: https://www.w3.org/TR/REC-xml/#charsets + */ +export function getUnicodeObjectName(): string { + const objectName = 'test' + .concat("!-_*.'()") + // Characters that might require special handling + .concat('&$@=;:+,? \x09\x0A\x0D') + // Characters to avoid + .concat('\\{}^%`[]"<>~#|\xFF') + // MinIO max. length for each '/' separated segment is 255 + .concat('/') + .concat([...Array(127).keys()].map((i) => String.fromCodePoint(i + 128)).join('')) + .concat('/') + // Some special Unicode characters + .concat('\u2028\u202F\u{0001FFFF}') + // Some other Unicode characters + .concat('일이삼\u{0001f642}') + + return objectName +} + +export function getInvalidObjectName(): string { + return 'test\x01\x02\x03.txt' +} + export function useMockQueue() { const queueSpy: jest.SpyInstance | undefined = undefined beforeEach(() => { diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 77944d8b5..06a20d561 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -12,7 +12,7 @@ import app from '../app' import { getConfig, JwksConfig, JwksConfigKeyOCT, mergeConfig } from '../config' import { backends, Obj } from '../storage' import { ObjectAdminDelete } from '../storage/events' -import { useMockObject, useMockQueue } from './common' +import { getInvalidObjectName, getUnicodeObjectName, useMockObject, useMockQueue } from './common' import { withDeleteEnabled } from './utils/storage' const { jwtSecret, serviceKeyAsync, tenantId } = getConfig() @@ -2881,3 +2881,92 @@ describe('x-robots-tag header', () => { }) }) }) + +describe('Object key names with Unicode characters', () => { + test('can upload, get, list, and delete', async () => { + const prefix = `test-utf8-${randomUUID()}` + const objectName = getUnicodeObjectName() + const authorization = `Bearer ${await serviceKeyAsync}` + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const headers = Object.assign({}, form.getHeaders(), { + authorization, + 'x-upsert': 'true', + }) + + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`, + headers: { + ...headers, + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + expect(getResponse.headers['last-modified']).toBe('Thu, 12 Aug 2021 16:00:00 GMT') + expect(getResponse.headers['cache-control']).toBe('no-cache') + + const listResponse = await appInstance.inject({ + method: 'POST', + url: `/object/list/bucket2`, + headers: { + authorization, + }, + payload: { prefix }, + }) + expect(listResponse.statusCode).toBe(200) + const listResponseJSON = JSON.parse(listResponse.body) + expect(listResponseJSON).toHaveLength(1) + expect(listResponseJSON[0].name).toBe(objectName.split('/')[0]) + + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(deleteResponse.statusCode).toBe(200) + }) + + test('should not upload if the name contains invalid characters', async () => { + const invalidObjectName = getInvalidObjectName() + const authorization = `Bearer ${await serviceKeyAsync}` + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const headers = Object.assign({}, form.getHeaders(), { + authorization, + 'x-upsert': 'true', + }) + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${encodeURIComponent(invalidObjectName)}`, + headers: { + ...headers, + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(400) + expect(S3Backend.prototype.uploadObject).not.toHaveBeenCalled() + expect(uploadResponse.body).toBe( + JSON.stringify({ + statusCode: '400', + error: 'InvalidKey', + message: `Invalid key: ${encodeURIComponent(invalidObjectName)}`, + }) + ) + }) +}) diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index dcb311969..e3ff0c8ad 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -34,6 +34,7 @@ import { FastifyInstance } from 'fastify' import { ReadableStreamBuffer } from 'stream-buffers' import app from '../app' import { getConfig, mergeConfig } from '../config' +import { getInvalidObjectName, getUnicodeObjectName } from './common' const { s3ProtocolAccessKeySecret, s3ProtocolAccessKeyId, storageS3Region } = getConfig() @@ -1798,5 +1799,150 @@ describe('S3 Protocol', () => { expect(response.ContentLanguage).toBeUndefined() }) }) + + describe('Object key names with Unicode characters', () => { + it('can be used with MultipartUpload commands', async () => { + const bucketName = await createBucket(client) + const objectName = getUnicodeObjectName() + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucketName, + Key: objectName, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const createMultipartResp = await client.send(createMultiPartUpload) + expect(createMultipartResp.UploadId).toBeTruthy() + const uploadId = createMultipartResp.UploadId + + const listMultipartUploads = new ListMultipartUploadsCommand({ + Bucket: bucketName, + }) + const listMultipartResp = await client.send(listMultipartUploads) + expect(listMultipartResp.Uploads?.length).toBe(1) + expect(listMultipartResp.Uploads?.[0].Key).toBe(objectName) + + const data = Buffer.alloc(1024 * 1024 * 2) + const uploadPart = new UploadPartCommand({ + Bucket: bucketName, + Key: objectName, + ContentLength: data.length, + UploadId: uploadId, + Body: data, + PartNumber: 1, + }) + const uploadPartResp = await client.send(uploadPart) + expect(uploadPartResp.ETag).toBeTruthy() + + const listParts = new ListPartsCommand({ + Bucket: bucketName, + Key: objectName, + UploadId: uploadId, + }) + const listPartsResp = await client.send(listParts) + expect(listPartsResp.Parts?.length).toBe(1) + + const completeMultiPartUpload = new CompleteMultipartUploadCommand({ + Bucket: bucketName, + Key: objectName, + UploadId: uploadId, + MultipartUpload: { + Parts: [ + { + PartNumber: 1, + ETag: uploadPartResp.ETag, + }, + ], + }, + }) + const completeMultipartResp = await client.send(completeMultiPartUpload) + expect(completeMultipartResp.$metadata.httpStatusCode).toBe(200) + expect(completeMultipartResp.Key).toEqual(objectName) + }) + + it('can be used with Put, List, and Delete Object commands', async () => { + const bucketName = await createBucket(client) + const objectName = getUnicodeObjectName() + const putObject = new PutObjectCommand({ + Bucket: bucketName, + Key: objectName, + Body: Buffer.alloc(1024 * 1024 * 1), + }) + const putObjectResp = await client.send(putObject) + expect(putObjectResp.$metadata.httpStatusCode).toEqual(200) + + const listObjects = new ListObjectsCommand({ + Bucket: bucketName, + }) + const listObjectsResp = await client.send(listObjects) + expect(listObjectsResp.Contents?.length).toBe(1) + expect(listObjectsResp.Contents?.[0].Key).toBe(objectName) + + const listObjectsV2 = new ListObjectsV2Command({ + Bucket: bucketName, + }) + const listObjectsV2Resp = await client.send(listObjectsV2) + expect(listObjectsV2Resp.Contents?.length).toBe(1) + expect(listObjectsV2Resp.Contents?.[0].Key).toBe(objectName) + + const getObject = new GetObjectCommand({ + Bucket: bucketName, + Key: objectName, + }) + const getObjectResp = await client.send(getObject) + const getObjectRespData = await getObjectResp.Body?.transformToByteArray() + expect(getObjectRespData).toBeTruthy() + expect(getObjectResp.ETag).toBeTruthy() + + const deleteObjects = new DeleteObjectsCommand({ + Bucket: bucketName, + Delete: { + Objects: [ + { + Key: objectName, + }, + ], + }, + }) + const deleteObjectsResp = await client.send(deleteObjects) + expect(deleteObjectsResp.Errors).toBeFalsy() + expect(deleteObjectsResp.Deleted).toEqual([ + { + Key: objectName, + }, + ]) + + const getObjectDeleted = new GetObjectCommand({ + Bucket: bucketName, + Key: objectName, + }) + try { + await client.send(getObjectDeleted) + throw new Error('Should not reach here') + } catch (e) { + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(404) + } + }) + + it('should not upload if the name contains invalid characters', async () => { + const bucketName = await createBucket(client) + const invalidObjectName = getInvalidObjectName() + try { + const putObject = new PutObjectCommand({ + Bucket: bucketName, + Key: invalidObjectName, + Body: Buffer.alloc(1024 * 1024 * 1), + }) + await client.send(putObject) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toBe('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(400) + expect((e as S3ServiceException).name).toEqual('InvalidKey') + expect((e as S3ServiceException).message).toEqual( + `Invalid key: ${encodeURIComponent(invalidObjectName)}` + ) + } + }) + }) }) }) diff --git a/src/test/tus.test.ts b/src/test/tus.test.ts index 6c49e99b8..1cab7c909 100644 --- a/src/test/tus.test.ts +++ b/src/test/tus.test.ts @@ -16,7 +16,7 @@ import { DetailedError } from 'tus-js-client' import app from '../app' import { getConfig } from '../config' import { backends, Storage, StorageKnexDB } from '../storage' -import { checkBucketExists } from './common' +import { checkBucketExists, getInvalidObjectName, getUnicodeObjectName } from './common' const { serviceKeyAsync, tenantId, storageS3Bucket, storageBackendType } = getConfig() const oneChunkFile = fs.createReadStream(path.resolve(__dirname, 'assets', 'sadcat.jpg')) @@ -531,4 +531,136 @@ describe('Tus multipart', () => { } }) }) + + describe('Object key names with Unicode characters', () => { + it('can be uploaded with the TUS protocol', async () => { + const objectName = randomUUID() + '-' + getUnicodeObjectName() + const authorization = `Bearer ${await serviceKeyAsync}` + + const bucket = await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const result = await new Promise((resolve, reject) => { + const upload = new tus.Upload(oneChunkFile, { + endpoint: `${localServerAddress}/upload/resumable`, + onShouldRetry: () => false, + uploadDataDuringCreation: false, + headers: { + authorization, + 'x-upsert': 'true', + }, + metadata: { + bucketName: bucketName, + objectName: objectName, + contentType: 'image/jpeg', + cacheControl: '3600', + metadata: JSON.stringify({ + test1: 'test1', + test2: 'test2', + }), + }, + onError: function (error) { + reject(error) + }, + onSuccess: () => { + resolve(true) + }, + }) + + upload.start() + }) + + expect(result).toEqual(true) + + const dbAsset = await storage.from(bucket.id).findObject(objectName, '*') + expect(dbAsset).toEqual({ + bucket_id: bucket.id, + created_at: expect.any(Date), + id: expect.any(String), + last_accessed_at: expect.any(Date), + metadata: { + cacheControl: 'max-age=3600', + contentLength: 29526, + eTag: '"53e1323c929d57b09b95fbe6d531865c-1"', + httpStatusCode: 200, + lastModified: expect.any(String), + mimetype: 'image/jpeg', + size: 29526, + }, + user_metadata: { + test1: 'test1', + test2: 'test2', + }, + name: objectName, + owner: null, + owner_id: null, + path_tokens: objectName.split('/'), + updated_at: expect.any(Date), + version: expect.any(String), + }) + + const getResponse = await app().inject({ + method: 'GET', + url: `/object/${bucketName}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('"53e1323c929d57b09b95fbe6d531865c-1"') + expect(getResponse.headers['cache-control']).toBe('max-age=3600') + expect(getResponse.headers['content-length']).toBe('29526') + expect(getResponse.headers['content-type']).toBe('image/jpeg') + }) + + it('should not upload if the name contains invalid characters', async () => { + await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const invalidObjectName = randomUUID() + '-' + getInvalidObjectName() + const authorization = `Bearer ${await serviceKeyAsync}` + try { + await new Promise((resolve, reject) => { + const upload = new tus.Upload(oneChunkFile, { + endpoint: `${localServerAddress}/upload/resumable`, + onShouldRetry: () => false, + uploadDataDuringCreation: false, + headers: { + authorization, + 'x-upsert': 'true', + }, + metadata: { + bucketName: bucketName, + objectName: invalidObjectName, + contentType: 'image/jpeg', + cacheControl: '3600', + }, + onError: function (error) { + reject(error) + }, + onSuccess: () => { + resolve(true) + }, + }) + + upload.start() + }) + + throw new Error('it should error with invalid key') + } catch (e) { + expect((e as Error).message).not.toEqual('it should error with invalid key') + const err = e as DetailedError + expect(err.originalResponse.getStatus()).toEqual(400) + expect(err.originalResponse.getBody()).toEqual( + `Invalid key: ${encodeURIComponent(invalidObjectName)}` + ) + } + }) + }) }) From 4b7f534cc364bb398e987b50d1f557861b63b6bb Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Wed, 25 Feb 2026 01:35:03 +0100 Subject: [PATCH 02/40] test: add unicode key and xml entity edge-case coverage Signed-off-by: ferhat elmas --- src/http/plugins/xml.ts | 4 +- src/storage/backend/s3/adapter.ts | 4 +- src/storage/object.ts | 11 +- src/storage/protocols/s3/s3-handler.ts | 68 +++-- src/test/bucket.test.ts | 31 ++- src/test/limits.test.ts | 33 +++ src/test/object.test.ts | 103 ++++++++ src/test/s3-adapter.test.ts | 43 ++- src/test/s3-protocol.test.ts | 350 ++++++++++++++++++++++++- src/test/tus.test.ts | 39 +-- src/test/xml-plugin.test.ts | 15 ++ 11 files changed, 649 insertions(+), 52 deletions(-) create mode 100644 src/test/limits.test.ts create mode 100644 src/test/xml-plugin.test.ts diff --git a/src/http/plugins/xml.ts b/src/http/plugins/xml.ts index 2a7209f18..bdeb8a016 100644 --- a/src/http/plugins/xml.ts +++ b/src/http/plugins/xml.ts @@ -6,7 +6,7 @@ import xml from 'xml2js' type XmlParserOptions = { disableContentParser?: boolean; parseAsArray?: string[] } type RequestError = Error & { statusCode?: number } -function decodeXmlNumericCharacterReferences(value: string): string { +export function decodeXmlNumericEntities(value: string): string { return value.replace(/&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, (match: string, rawValue: string) => { const isHex = rawValue[0].toLowerCase() === 'x' const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) @@ -70,7 +70,7 @@ export const xmlParser = fastifyPlugin( explicitArray: false, trim: true, valueProcessors: [ - decodeXmlNumericCharacterReferences, + decodeXmlNumericEntities, xml.processors.parseNumbers, xml.processors.parseBooleans, ], diff --git a/src/storage/backend/s3/adapter.ts b/src/storage/backend/s3/adapter.ts index 5c8fc4736..b5607e911 100644 --- a/src/storage/backend/s3/adapter.ts +++ b/src/storage/backend/s3/adapter.ts @@ -568,7 +568,9 @@ export class S3Backend implements StorageBackendAdapter { Key: withOptionalVersion(key, version), UploadId, PartNumber, - CopySource: `${storageS3Bucket}/${withOptionalVersion(sourceKey, sourceKeyVersion)}`, + CopySource: encodeURIComponent( + `${storageS3Bucket}/${withOptionalVersion(sourceKey, sourceKeyVersion)}` + ), CopySourceRange: bytesRange ? `bytes=${bytesRange.fromByte}-${bytesRange.toByte}` : undefined, }) diff --git a/src/storage/object.ts b/src/storage/object.ts index 9a8781c50..122c1716d 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -23,6 +23,7 @@ const { requestUrlLengthLimit } = getConfig() interface CopyObjectParams { sourceKey: string + sourceVersion?: string destinationBucket: string destinationKey: string owner?: string @@ -294,6 +295,7 @@ export class ObjectStorage { */ async copyObject({ sourceKey, + sourceVersion, destinationBucket, destinationKey, owner, @@ -324,6 +326,11 @@ export class ObjectStorage { 'bucket_id,metadata,user_metadata,version' ) + if (sourceVersion && originObject.version !== sourceVersion) { + throw ERRORS.NoSuchKey(sourceKey) + } + + // eslint-disable-next-line @typescript-eslint/no-unused-vars const baseMetadata = originObject.metadata || {} const destinationMetadata = copyMetadata ? baseMetadata @@ -343,7 +350,7 @@ export class ObjectStorage { const copyResult = await this.backend.copyObject( this.location.getRootLocation(), s3SourceKey, - originObject.version, + sourceVersion || originObject.version, s3DestinationKey, newVersion, destinationMetadata, @@ -792,6 +799,8 @@ export class ObjectStorage { owner?: string, options?: { upsert?: boolean } ) { + mustBeValidKey(objectName) + // check if user has INSERT permissions await this.uploader.canUpload({ bucketId: this.bucketId, diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index 10f22424d..8fa8b4df5 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -1051,14 +1051,11 @@ export class S3ProtocolHandler { throw ERRORS.MissingParameter('CopySource') } - const sourceBucket = ( - CopySource.startsWith('/') ? CopySource.replace('/', '').split('/') : CopySource.split('/') - ).shift() - - const sourceKey = (CopySource.startsWith('/') ? CopySource.replace('/', '') : CopySource) - .split('/') - .slice(1) - .join('/') + const { + bucketName: sourceBucket, + objectKey: sourceKey, + sourceVersion, + } = parseCopySource(CopySource) if (!sourceBucket) { throw ERRORS.InvalidBucketName('') @@ -1075,6 +1072,7 @@ export class S3ProtocolHandler { const copyResult = await this.storage.from(sourceBucket).copyObject({ sourceKey, + sourceVersion, destinationBucket: Bucket, destinationKey: Key, owner: this.owner, @@ -1186,14 +1184,11 @@ export class S3ProtocolHandler { throw ERRORS.MissingParameter('CopySourceRange') } - const sourceBucketName = ( - CopySource.startsWith('/') ? CopySource.replace('/', '').split('/') : CopySource.split('/') - ).shift() - - const sourceKey = (CopySource.startsWith('/') ? CopySource.replace('/', '') : CopySource) - .split('/') - .slice(1) - .join('/') + const { + bucketName: sourceBucketName, + objectKey: sourceKey, + sourceVersion, + } = parseCopySource(CopySource) if (!sourceBucketName) { throw ERRORS.NoSuchBucket('') @@ -1210,6 +1205,10 @@ export class S3ProtocolHandler { 'id,name,version,metadata' ) + if (sourceVersion && copySource.version !== sourceVersion) { + throw ERRORS.NoSuchKey(sourceKey) + } + let copySize = copySource.metadata?.size || 0 let rangeBytes: { fromByte: number; toByte: number } | undefined = undefined @@ -1268,7 +1267,7 @@ export class S3ProtocolHandler { objectName: copySource.name, tenantId: this.tenantId, }), - copySource.version, + sourceVersion || copySource.version, rangeBytes ) @@ -1401,6 +1400,41 @@ function isUSASCII(str: string): boolean { return true } +function parseCopySource(copySource: string): { + bucketName: string + objectKey: string + sourceVersion?: string +} { + const normalizedCopySource = copySource.startsWith('/') ? copySource.slice(1) : copySource + const [encodedPath, queryParams = ''] = normalizedCopySource.split('?') + const [encodedBucketName, ...encodedObjectKeyParts] = encodedPath.split('/') + + if (!encodedBucketName) { + throw ERRORS.InvalidBucketName('') + } + + if (!encodedObjectKeyParts.length) { + throw ERRORS.MissingParameter('CopySource') + } + + try { + const searchParams = new URLSearchParams(queryParams) + const sourceVersion = searchParams.get('versionId') || undefined + + if (searchParams.has('versionId') && !sourceVersion) { + throw ERRORS.InvalidParameter('CopySource') + } + + return { + bucketName: decodeURIComponent(encodedBucketName), + objectKey: decodeURIComponent(encodedObjectKeyParts.join('/')), + sourceVersion, + } + } catch { + throw ERRORS.InvalidParameter('CopySource') + } +} + function encodeContinuationToken(name: string) { return Buffer.from(`l:${name}`).toString('base64') } diff --git a/src/test/bucket.test.ts b/src/test/bucket.test.ts index 20c9c546e..174e30a3b 100644 --- a/src/test/bucket.test.ts +++ b/src/test/bucket.test.ts @@ -183,6 +183,17 @@ describe('testing GET all buckets', () => { }) test('user is able to get buckets with limit, offset, search and sorting', async () => { + const allBucketsResponse = await appInstance.inject({ + method: 'GET', + url: `/bucket?sortColumn=name&sortOrder=asc&search=bucket`, + headers: { + authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`, + }, + }) + expect(allBucketsResponse.statusCode).toBe(200) + const allBuckets = JSON.parse(allBucketsResponse.body) + expect(allBuckets.length).toBeGreaterThanOrEqual(4) + const response = await appInstance.inject({ method: 'GET', url: `/bucket?limit=1&offset=3&sortColumn=name&sortOrder=asc&search=bucket`, @@ -193,9 +204,10 @@ describe('testing GET all buckets', () => { expect(response.statusCode).toBe(200) const responseJSON = JSON.parse(response.body) expect(responseJSON.length).toEqual(1) + const expectedBucket = allBuckets[3] expect(responseJSON[0]).toMatchObject({ - id: 'bucket4', - name: 'bucket4', + id: expectedBucket.id, + name: expectedBucket.name, type: expect.any(String), public: false, file_size_limit: null, @@ -449,11 +461,10 @@ describe('testing public bucket functionality', () => { describe('testing count objects in bucket', () => { const { tenantId } = getConfig() - const testObjectCount = 27 + const testObjectNames = ['object-1.txt', 'object-2.txt', 'object-3.txt'] const testOwnerId = randomUUID() let db: StorageKnexDB let testBucketId: string - let testObjectNames: string[] beforeAll(async () => { const serviceKeyUser = await getServiceKeyUser(tenantId) @@ -470,10 +481,6 @@ describe('testing count objects in bucket', () => { }) testBucketId = `count-objects-${randomUUID()}` - testObjectNames = Array.from({ length: testObjectCount }, (_, idx) => { - return `fixtures/count-object-${idx}` - }) - await db.createBucket({ id: testBucketId, name: testBucketId, @@ -490,7 +497,7 @@ describe('testing count objects in bucket', () => { name, owner: testOwnerId, bucket_id: testBucketId, - metadata: { size: 1 }, + metadata: { size: 1234 }, user_metadata: null, version: undefined, }) @@ -505,13 +512,13 @@ describe('testing count objects in bucket', () => { }) it('should return correct object count', async () => { - await expect(db.countObjectsInBucket(testBucketId)).resolves.toBe(testObjectCount) + await expect(db.countObjectsInBucket(testBucketId)).resolves.toBe(testObjectNames.length) }) it('should return limited object count', async () => { - await expect(db.countObjectsInBucket(testBucketId, 22)).resolves.toBe(22) + await expect(db.countObjectsInBucket(testBucketId, 2)).resolves.toBe(2) }) it('should return full object count if limit is greater than total', async () => { - await expect(db.countObjectsInBucket(testBucketId, 999)).resolves.toBe(testObjectCount) + await expect(db.countObjectsInBucket(testBucketId, 999)).resolves.toBe(testObjectNames.length) }) it('should return 0 object count if there are no objects with provided bucket id', async () => { await expect(db.countObjectsInBucket('this-is-not-a-bucket-at-all', 999)).resolves.toBe(0) diff --git a/src/test/limits.test.ts b/src/test/limits.test.ts new file mode 100644 index 000000000..6204a6897 --- /dev/null +++ b/src/test/limits.test.ts @@ -0,0 +1,33 @@ +import { isValidKey } from '../storage/limits' + +describe('isValidKey', () => { + test('accepts unicode object names', () => { + expect(isValidKey('folder/일이삼/🙂/a b.txt')).toBe(true) + }) + + test('accepts tab, newline, and carriage return', () => { + expect(isValidKey('a\tb\nc\rd')).toBe(true) + }) + + test('rejects empty keys', () => { + expect(isValidKey('')).toBe(false) + }) + + test('rejects ASCII control characters except tab/newline/carriage return', () => { + expect(isValidKey('invalid\x01name')).toBe(false) + }) + + test('rejects non-characters U+FFFE and U+FFFF', () => { + expect(isValidKey(`invalid${'\uFFFE'}`)).toBe(false) + expect(isValidKey(`invalid${'\uFFFF'}`)).toBe(false) + }) + + test('rejects lone surrogate code units', () => { + expect(isValidKey(`bad${'\uD83D'}`)).toBe(false) + expect(isValidKey(`bad${'\uDC00'}`)).toBe(false) + }) + + test('accepts valid surrogate pairs', () => { + expect(isValidKey('ok🙂name')).toBe(true) + }) +}) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 06a20d561..1147ce8fd 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2969,4 +2969,107 @@ describe('Object key names with Unicode characters', () => { }) ) }) + + test('can generate and use a signed download URL', async () => { + const objectName = `signed-download-${randomUUID()}-일이삼-🙂.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadHeaders = Object.assign({}, form.getHeaders(), { + authorization, + 'x-upsert': 'true', + }) + + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + ...uploadHeaders, + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + + const signResponse = await appInstance.inject({ + method: 'POST', + url: `/object/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + payload: { + expiresIn: 60, + }, + }) + expect(signResponse.statusCode).toBe(200) + + const signedURL = signResponse.json<{ signedURL: string }>().signedURL + expect(signedURL).toContain('?token=') + const token = signedURL.split('?token=').pop() + expect(token).toBeTruthy() + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/sign/bucket2/${encodeURIComponent(objectName)}?token=${token}`, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + expect(getResponse.headers['last-modified']).toBe('Thu, 12 Aug 2021 16:00:00 GMT') + }) + + test('can generate and use a signed upload URL', async () => { + const objectName = `signed-upload-${randomUUID()}-일이삼-🙂.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const signedUploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(signedUploadResponse.statusCode).toBe(200) + + const token = signedUploadResponse.json<{ token: string }>().token + expect(token).toBeTruthy() + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}?token=${token}`, + headers: { + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + expect(S3Backend.prototype.uploadObject).toHaveBeenCalled() + + const db = await getSuperuserPostgrestClient() + const objectResponse = await db + .from('objects') + .select('*') + .where({ + name: objectName, + bucket_id: 'bucket2', + }) + .first() + expect(objectResponse?.name).toBe(objectName) + }) + + test('should not generate signed upload URL for invalid key', async () => { + const invalidObjectName = getInvalidObjectName() + const authorization = `Bearer ${await serviceKeyAsync}` + + const signedUploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/upload/sign/bucket2/${encodeURIComponent(invalidObjectName)}`, + headers: { + authorization, + }, + }) + expect(signedUploadResponse.statusCode).toBe(400) + expect(signedUploadResponse.body).toContain('Invalid key') + }) }) diff --git a/src/test/s3-adapter.test.ts b/src/test/s3-adapter.test.ts index 0d5288e5f..fd6807775 100644 --- a/src/test/s3-adapter.test.ts +++ b/src/test/s3-adapter.test.ts @@ -1,6 +1,6 @@ 'use strict' -import { S3Client } from '@aws-sdk/client-s3' +import { S3Client, UploadPartCopyCommand } from '@aws-sdk/client-s3' import { Readable } from 'stream' import { S3Backend } from '../storage/backend/s3/adapter' @@ -74,4 +74,45 @@ describe('S3Backend', () => { expect(result.metadata.mimetype).toBe('image/png') }) }) + + describe('uploadPartCopy', () => { + test('should URL-encode CopySource for unicode keys', async () => { + const lastModified = new Date('2024-01-01T00:00:00.000Z') + mockSend.mockResolvedValue({ + CopyPartResult: { + ETag: '"copy-etag"', + LastModified: lastModified, + }, + }) + + const backend = new S3Backend({ + region: 'us-east-1', + endpoint: 'http://localhost:9000', + }) + + const sourceKey = 'source/path/일이삼-🙂.jpg' + const destinationKey = 'dest/path/copied-🙂.jpg' + + const result = await backend.uploadPartCopy( + 'test-bucket', + destinationKey, + '', + 'upload-id', + 1, + sourceKey, + undefined, + { fromByte: 0, toByte: 1024 } + ) + + expect(mockSend).toHaveBeenCalledTimes(1) + const command = mockSend.mock.calls[0][0] as UploadPartCopyCommand + expect(command).toBeInstanceOf(UploadPartCopyCommand) + expect(command.input.CopySource).toBe(encodeURIComponent(`test-bucket/${sourceKey}`)) + expect(command.input.CopySourceRange).toBe('bytes=0-1024') + expect(result).toEqual({ + eTag: '"copy-etag"', + lastModified, + }) + }) + }) }) diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index e3ff0c8ad..c933253d4 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -28,6 +28,7 @@ import { Upload } from '@aws-sdk/lib-storage' import { createPresignedPost } from '@aws-sdk/s3-presigned-post' import { getSignedUrl } from '@aws-sdk/s3-request-presigner' import { wait } from '@internal/concurrency' +import { getPostgresConnection, getServiceKeyUser } from '@internal/database' import axios from 'axios' import { randomUUID } from 'crypto' import { FastifyInstance } from 'fastify' @@ -36,7 +37,7 @@ import app from '../app' import { getConfig, mergeConfig } from '../config' import { getInvalidObjectName, getUnicodeObjectName } from './common' -const { s3ProtocolAccessKeySecret, s3ProtocolAccessKeyId, storageS3Region } = getConfig() +const { s3ProtocolAccessKeySecret, s3ProtocolAccessKeyId, storageS3Region, tenantId } = getConfig() async function createBucket(client: S3Client, name?: string, publicRead = true) { let bucketName: string @@ -77,6 +78,37 @@ async function uploadFile( return await uploader.done() } +async function getObjectVersion(bucketId: string, objectName: string): Promise { + const superUser = await getServiceKeyUser(tenantId) + const connection = await getPostgresConnection({ + superUser, + user: superUser, + tenantId, + host: 'localhost', + }) + const tx = await connection.transaction() + + try { + const object = (await tx + .from('objects') + .select('version') + .where({ + bucket_id: bucketId, + name: objectName, + }) + .first()) as { version?: string } | undefined + + if (!object?.version) { + throw new Error(`Object version not found for ${bucketId}/${objectName}`) + } + + return object.version + } finally { + await tx.rollback() + await connection.dispose() + } +} + jest.setTimeout(10 * 1000) describe('S3 Protocol', () => { @@ -1141,6 +1173,43 @@ describe('S3 Protocol', () => { expect(headObj.CacheControl).toBe('max-age=2009') }) + it('will copy an object when CopySource includes versionId', async () => { + const bucketName = await createBucket(client) + const sourceKey = `test-copy-versioned-${randomUUID()}.jpg` + await uploadFile(client, bucketName, sourceKey, 1) + const sourceVersion = await getObjectVersion(bucketName, sourceKey) + + const copyObjectCommand = new CopyObjectCommand({ + Bucket: bucketName, + Key: `test-copied-versioned-${randomUUID()}.jpg`, + CopySource: `${bucketName}/${sourceKey}?versionId=${sourceVersion}`, + }) + + const resp = await client.send(copyObjectCommand) + expect(resp.CopyObjectResult?.ETag).toBeTruthy() + }) + + it('will not copy an object when CopySource versionId does not match', async () => { + const bucketName = await createBucket(client) + const sourceKey = `test-copy-versioned-${randomUUID()}.jpg` + await uploadFile(client, bucketName, sourceKey, 1) + + const copyObjectCommand = new CopyObjectCommand({ + Bucket: bucketName, + Key: `test-copied-versioned-${randomUUID()}.jpg`, + CopySource: `${bucketName}/${sourceKey}?versionId=${randomUUID()}`, + }) + + try { + await client.send(copyObjectCommand) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(404) + expect((e as S3ServiceException).message).toEqual('Object not found') + } + }) + it('will not be able to copy an object that doesnt exist', async () => { const bucketName1 = await createBucket(client) await uploadFile(client, bucketName1, 'test-copy-1.jpg', 1) @@ -1160,6 +1229,44 @@ describe('S3 Protocol', () => { expect((e as S3ServiceException).message).toEqual('Object not found') } }) + + it('will reject malformed url-encoded CopySource', async () => { + const bucketName = await createBucket(client) + + const copyObjectCommand = new CopyObjectCommand({ + Bucket: bucketName, + Key: 'test-copied-malformed.jpg', + CopySource: `${bucketName}/test-copy-%ZZ.jpg`, + }) + + try { + await client.send(copyObjectCommand) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(400) + expect((e as S3ServiceException).message).toEqual('Invalid Parameter CopySource') + } + }) + + it('will reject CopySource without an object key', async () => { + const bucketName = await createBucket(client) + + const copyObjectCommand = new CopyObjectCommand({ + Bucket: bucketName, + Key: 'test-copied-missing-source-key.jpg', + CopySource: bucketName, + }) + + try { + await client.send(copyObjectCommand) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(400) + expect((e as S3ServiceException).message).toEqual('Missing Required Parameter CopySource') + } + }) }) describe('ListMultipartUploads', () => { @@ -1506,6 +1613,166 @@ describe('S3 Protocol', () => { const parts = await client.send(listPartsCmd) expect(parts.Parts?.length).toBe(1) }) + + it('will copy a part when CopySource includes versionId', async () => { + const bucket = await createBucket(client) + const sourceKey = `${randomUUID()}.jpg` + const newKey = `new-${randomUUID()}.jpg` + + await uploadFile(client, bucket, sourceKey, 12) + const sourceVersion = await getObjectVersion(bucket, sourceKey) + + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const resp = await client.send(createMultiPartUpload) + expect(resp.UploadId).toBeTruthy() + + const copyPart = new UploadPartCopyCommand({ + Bucket: bucket, + Key: newKey, + UploadId: resp.UploadId, + PartNumber: 1, + CopySource: `${bucket}/${sourceKey}?versionId=${sourceVersion}`, + CopySourceRange: `bytes=0-${1024 * 4}`, + }) + + const copyResp = await client.send(copyPart) + expect(copyResp.CopyPartResult?.ETag).toBeTruthy() + + const listPartsCmd = new ListPartsCommand({ + Bucket: bucket, + Key: newKey, + UploadId: resp.UploadId, + }) + + const parts = await client.send(listPartsCmd) + expect(parts.Parts?.length).toBe(1) + }) + + it('will not copy a part when CopySource versionId does not match', async () => { + const bucket = await createBucket(client) + const sourceKey = `${randomUUID()}.jpg` + const newKey = `new-${randomUUID()}.jpg` + + await uploadFile(client, bucket, sourceKey, 12) + + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const resp = await client.send(createMultiPartUpload) + expect(resp.UploadId).toBeTruthy() + + const copyPart = new UploadPartCopyCommand({ + Bucket: bucket, + Key: newKey, + UploadId: resp.UploadId, + PartNumber: 1, + CopySource: `${bucket}/${sourceKey}?versionId=${randomUUID()}`, + CopySourceRange: `bytes=0-${1024 * 4}`, + }) + + try { + await client.send(copyPart) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(404) + expect((e as S3ServiceException).message).toEqual('Object not found') + } + }) + + it('will reject malformed url-encoded CopySource', async () => { + const bucket = await createBucket(client) + const newKey = `new-${randomUUID()}.jpg` + + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const resp = await client.send(createMultiPartUpload) + expect(resp.UploadId).toBeTruthy() + const uploadId = resp.UploadId + + const copyPart = new UploadPartCopyCommand({ + Bucket: bucket, + Key: newKey, + UploadId: uploadId, + PartNumber: 1, + CopySource: `${bucket}/test-copy-%ZZ.jpg`, + CopySourceRange: `bytes=0-${1024 * 4}`, + }) + + try { + await client.send(copyPart) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(400) + expect((e as S3ServiceException).message).toEqual('Invalid Parameter CopySource') + } finally { + if (uploadId) { + await client.send( + new AbortMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + UploadId: uploadId, + }) + ) + } + } + }) + + it('will reject CopySource without an object key', async () => { + const bucket = await createBucket(client) + const newKey = `new-${randomUUID()}.jpg` + + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const resp = await client.send(createMultiPartUpload) + expect(resp.UploadId).toBeTruthy() + const uploadId = resp.UploadId + + const copyPart = new UploadPartCopyCommand({ + Bucket: bucket, + Key: newKey, + UploadId: uploadId, + PartNumber: 1, + CopySource: bucket, + CopySourceRange: `bytes=0-${1024 * 4}`, + }) + + try { + await client.send(copyPart) + throw new Error('Should not reach here') + } catch (e) { + expect((e as Error).message).not.toEqual('Should not reach here') + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(400) + expect((e as S3ServiceException).message).toEqual('Missing Required Parameter CopySource') + } finally { + if (uploadId) { + await client.send( + new AbortMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + UploadId: uploadId, + }) + ) + } + } + }) }) describe('S3 Presigned URL', () => { @@ -1923,6 +2190,87 @@ describe('S3 Protocol', () => { } }) + it('can copy objects using Unicode keys in CopySource', async () => { + const bucketName = await createBucket(client) + const sourceKey = `copy-src-${randomUUID()}-일이삼-🙂.jpg` + const destinationKey = `copy-dst-${randomUUID()}-일이삼-🙂.jpg` + const destinationKeyWithLeadingSlashSource = `copy-dst-leading-${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 copyObjectRespLeadingSlash = await client.send( + new CopyObjectCommand({ + Bucket: bucketName, + Key: destinationKeyWithLeadingSlashSource, + CopySource: `/${encodeURI(`${bucketName}/${sourceKey}`)}`, + }) + ) + expect(copyObjectRespLeadingSlash.$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, destinationKeyWithLeadingSlashSource]) + ) + }) + + it('can upload part copy using Unicode keys in CopySource', async () => { + const bucketName = await createBucket(client) + const sourceKey = `copy-part-src-${randomUUID()}-일이삼-🙂.jpg` + const destinationKey = `copy-part-dst-${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() diff --git a/src/test/tus.test.ts b/src/test/tus.test.ts index 1cab7c909..391985999 100644 --- a/src/test/tus.test.ts +++ b/src/test/tus.test.ts @@ -553,8 +553,8 @@ describe('Tus multipart', () => { 'x-upsert': 'true', }, metadata: { - bucketName: bucketName, - objectName: objectName, + bucketName, + objectName, contentType: 'image/jpeg', cacheControl: '3600', metadata: JSON.stringify({ @@ -562,7 +562,7 @@ describe('Tus multipart', () => { test2: 'test2', }), }, - onError: function (error) { + onError(error) { reject(error) }, onSuccess: () => { @@ -602,18 +602,23 @@ describe('Tus multipart', () => { version: expect.any(String), }) - const getResponse = await app().inject({ - method: 'GET', - url: `/object/${bucketName}/${encodeURIComponent(objectName)}`, - headers: { - authorization, - }, - }) - expect(getResponse.statusCode).toBe(200) - expect(getResponse.headers['etag']).toBe('"53e1323c929d57b09b95fbe6d531865c-1"') - expect(getResponse.headers['cache-control']).toBe('max-age=3600') - expect(getResponse.headers['content-length']).toBe('29526') - expect(getResponse.headers['content-type']).toBe('image/jpeg') + const appInstance = app() + try { + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/${bucketName}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('"53e1323c929d57b09b95fbe6d531865c-1"') + expect(getResponse.headers['cache-control']).toBe('max-age=3600') + expect(getResponse.headers['content-length']).toBe('29526') + expect(getResponse.headers['content-type']).toBe('image/jpeg') + } finally { + await appInstance.close() + } }) it('should not upload if the name contains invalid characters', async () => { @@ -636,12 +641,12 @@ describe('Tus multipart', () => { 'x-upsert': 'true', }, metadata: { - bucketName: bucketName, + bucketName, objectName: invalidObjectName, contentType: 'image/jpeg', cacheControl: '3600', }, - onError: function (error) { + onError(error) { reject(error) }, onSuccess: () => { diff --git a/src/test/xml-plugin.test.ts b/src/test/xml-plugin.test.ts new file mode 100644 index 000000000..2987ea601 --- /dev/null +++ b/src/test/xml-plugin.test.ts @@ -0,0 +1,15 @@ +import { decodeXmlNumericEntities } from '../http/plugins/xml' + +describe('decodeXmlNumericEntities', () => { + test('decodes hexadecimal entities including astral code points', () => { + expect(decodeXmlNumericEntities('a🙂b')).toBe('a🙂b') + }) + + test('decodes decimal entities', () => { + expect(decodeXmlNumericEntities('a🙂b')).toBe('a🙂b') + }) + + test('keeps out-of-range entities unchanged', () => { + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + }) +}) From 4b95b1bb784eaa97080a37472f51c0c2c60ea9b8 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 10:42:54 +0100 Subject: [PATCH 03/40] fix: more tests and edge case handling Signed-off-by: ferhat elmas --- src/storage/object.ts | 17 ++- src/storage/protocols/s3/s3-handler.ts | 50 +++++--- src/test/object-list-v2.test.ts | 96 ++++++++++++++ src/test/object.test.ts | 14 ++- src/test/s3-protocol.test.ts | 167 +++++++++++++++++++++++++ src/test/test-hygiene.test.ts | 56 +++++++++ src/test/tus.test.ts | 44 +++++++ 7 files changed, 413 insertions(+), 31 deletions(-) create mode 100644 src/test/test-hygiene.test.ts diff --git a/src/storage/object.ts b/src/storage/object.ts index 122c1716d..0cd4d3305 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -864,13 +864,14 @@ const CONTINUATION_TOKEN_PART_MAP: Record = { } function encodeContinuationToken(tokenInfo: ContinuationToken) { - let result = '' + const result: string[] = [] for (const [k, v] of Object.entries(CONTINUATION_TOKEN_PART_MAP)) { - if (tokenInfo[v]) { - result += `${k}:${tokenInfo[v]}\n` + const value = tokenInfo[v] + if (value) { + result.push(`${k}:${encodeURIComponent(value)}`) } } - return Buffer.from(result.slice(0, -1)).toString('base64') + return Buffer.from(result.join('\n')).toString('base64') } function decodeContinuationToken(token: string): ContinuationToken { @@ -884,7 +885,13 @@ function decodeContinuationToken(token: string): ContinuationToken { if (!partMatch || partMatch.length !== 3 || !(partMatch[1] in CONTINUATION_TOKEN_PART_MAP)) { throw new Error('Invalid continuation token') } - result[CONTINUATION_TOKEN_PART_MAP[partMatch[1]]] = partMatch[2] + let value = partMatch[2] + try { + value = decodeURIComponent(value) + } catch { + // Backward compatibility: previously cursor values were stored unescaped. + } + result[CONTINUATION_TOKEN_PART_MAP[partMatch[1]]] = value } return result } diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index 8fa8b4df5..e5f2f0ed5 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -1406,33 +1406,39 @@ function parseCopySource(copySource: string): { sourceVersion?: string } { const normalizedCopySource = copySource.startsWith('/') ? copySource.slice(1) : copySource - const [encodedPath, queryParams = ''] = normalizedCopySource.split('?') - const [encodedBucketName, ...encodedObjectKeyParts] = encodedPath.split('/') + const [encodedPath, ...queryParts] = normalizedCopySource.split('?') + const queryParams = queryParts.join('?') - if (!encodedBucketName) { - throw ERRORS.InvalidBucketName('') + let decodedPath = '' + try { + decodedPath = decodeURIComponent(encodedPath) + } catch { + throw ERRORS.InvalidParameter('CopySource') } - if (!encodedObjectKeyParts.length) { + const separatorIdx = decodedPath.indexOf('/') + if (separatorIdx <= 0) { throw ERRORS.MissingParameter('CopySource') } - try { - const searchParams = new URLSearchParams(queryParams) - const sourceVersion = searchParams.get('versionId') || undefined + const bucketName = decodedPath.slice(0, separatorIdx) + const objectKey = decodedPath.slice(separatorIdx + 1) + if (!objectKey) { + throw ERRORS.MissingParameter('CopySource') + } - if (searchParams.has('versionId') && !sourceVersion) { - throw ERRORS.InvalidParameter('CopySource') - } + const searchParams = new URLSearchParams(queryParams) + const sourceVersion = searchParams.get('versionId') || undefined - return { - bucketName: decodeURIComponent(encodedBucketName), - objectKey: decodeURIComponent(encodedObjectKeyParts.join('/')), - sourceVersion, - } - } catch { + if (searchParams.has('versionId') && !sourceVersion) { throw ERRORS.InvalidParameter('CopySource') } + + return { + bucketName, + objectKey, + sourceVersion, + } } function encodeContinuationToken(name: string) { @@ -1440,11 +1446,15 @@ function encodeContinuationToken(name: string) { } function decodeContinuationToken(token: string) { - const decoded = Buffer.from(token, 'base64').toString().split(':') + const decoded = Buffer.from(token, 'base64').toString() + if (!decoded.startsWith('l:')) { + throw new Error('Invalid continuation token') + } - if (decoded.length === 0) { + const value = decoded.slice(2) + if (!value) { throw new Error('Invalid continuation token') } - return decoded[1] + return value } diff --git a/src/test/object-list-v2.test.ts b/src/test/object-list-v2.test.ts index 768eaf32f..b7d461256 100644 --- a/src/test/object-list-v2.test.ts +++ b/src/test/object-list-v2.test.ts @@ -578,6 +578,7 @@ describe('objects - list v2 sorting tests', () => { }) const LIST_V2_WILDCARD_BUCKET = `list-v2-wildcard-${randomUUID()}` +const LIST_V2_CURSOR_BUCKET = `list-v2-cursor-${randomUUID()}` describe('objects - list v2 prefix wildcard handling', () => { beforeAll(async () => { @@ -700,3 +701,98 @@ describe('objects - list v2 prefix wildcard handling', () => { expect(data.objects.map((obj) => obj.name)).toEqual([literalMatch]) }) }) + +describe('objects - list v2 cursor encoding', () => { + beforeAll(async () => { + appInstance = app() + await appInstance.inject({ + method: 'POST', + url: `/bucket`, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + payload: { + name: LIST_V2_CURSOR_BUCKET, + }, + }) + await appInstance.close() + }) + + afterAll(async () => { + appInstance = app() + await appInstance.inject({ + method: 'POST', + url: `/bucket/${LIST_V2_CURSOR_BUCKET}/empty`, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + + await appInstance.inject({ + method: 'DELETE', + url: `/bucket/${LIST_V2_CURSOR_BUCKET}`, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + + await appInstance.close() + }) + + test('paginates when keys contain newline and percent characters', async () => { + const runId = randomUUID() + const prefix = `cursor-${runId}-` + const keys = [`${prefix}first-\n-🙂-%.txt`, `${prefix}second-\n-일이삼-:.txt`] + + for (const key of keys) { + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/${LIST_V2_CURSOR_BUCKET}/${encodeURIComponent(key)}`, + payload: createUpload('utf8.txt', 'cursor test'), + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(uploadResponse.statusCode).toBe(200) + } + + const page1Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page1Response.statusCode).toBe(200) + const page1 = page1Response.json() + expect(page1.objects).toHaveLength(1) + expect(page1.hasNext).toBe(true) + expect(page1.nextCursor).toBeTruthy() + + const page2Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + cursor: page1.nextCursor, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page2Response.statusCode).toBe(200) + const page2 = page2Response.json() + expect(page2.objects).toHaveLength(1) + expect(page2.hasNext).toBe(false) + + const listed = [page1.objects[0]?.name, page2.objects[0]?.name].filter(Boolean).sort() + expect(listed).toEqual([...keys].sort()) + }) +}) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 1147ce8fd..a4ea99280 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -1873,7 +1873,7 @@ describe('testing uploading with generated signed upload URL', () => { }) const BUCKET_ID = 'bucket2' - const OBJECT_NAME = 'public/sadcat-upload1.png' + const OBJECT_NAME = `public/sadcat-upload-${randomUUID()}.png` const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}` const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a' @@ -1917,10 +1917,11 @@ describe('testing uploading with generated signed upload URL', () => { const headers = Object.assign({}, form.getHeaders(), { 'content-type': 'image/jpeg', }) + const objectName = `public/sadcat-upload-${randomUUID()}.png` const response = await appInstance.inject({ method: 'PUT', - url: `/object/upload/sign/bucket2/public/sadcat-upload1.png`, + url: `/object/upload/sign/bucket2/${objectName}`, headers, payload: form, }) @@ -1934,10 +1935,11 @@ describe('testing uploading with generated signed upload URL', () => { const headers = Object.assign({}, form.getHeaders(), { 'content-type': 'image/jpeg', }) + const objectName = `public/sadcat-upload-${randomUUID()}.png` const response = await appInstance.inject({ method: 'PUT', - url: `/object/upload/sign/bucket2/public/sadcat-upload1.png?token=xxx`, + url: `/object/upload/sign/bucket2/${objectName}?token=xxx`, headers, payload: form, }) @@ -1953,7 +1955,7 @@ describe('testing uploading with generated signed upload URL', () => { }) const BUCKET_ID = 'bucket2' - const OBJECT_NAME = 'public/sadcat-upload1.png' + const OBJECT_NAME = `public/sadcat-upload-${randomUUID()}.png` const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}` const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a' @@ -1976,7 +1978,7 @@ describe('testing uploading with generated signed upload URL', () => { } const BUCKET_ID = 'bucket2' - const OBJECT_NAME = 'signed/sadcat-upload-signed-2.png' + const OBJECT_NAME = `signed/sadcat-upload-signed-${randomUUID()}.png` const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}` // Upload a file first @@ -2021,7 +2023,7 @@ describe('testing uploading with generated signed upload URL', () => { } const BUCKET_ID = 'bucket2' - const OBJECT_NAME = 'signed/sadcat-upload-signed-3.png' + const OBJECT_NAME = `signed/sadcat-upload-signed-${randomUUID()}.png` const urlToSign = `${BUCKET_ID}/${OBJECT_NAME}` const owner = '317eadce-631a-4429-a0bb-f19a7a517b4a' diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index c933253d4..e63f04e0d 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -2190,6 +2190,104 @@ describe('S3 Protocol', () => { } }) + it('can paginate ListObjectsV2 with Unicode keys', async () => { + const bucketName = await createBucket(client) + const keys = [ + `utf8-page-${randomUUID()}-🙂.jpg`, + `utf8-page-${randomUUID()}-일이삼.jpg`, + `utf8-page-${randomUUID()}-éè.jpg`, + ] + + await Promise.all( + keys.map((key) => + client.send( + new PutObjectCommand({ + Bucket: bucketName, + Key: key, + Body: Buffer.alloc(64), + }) + ) + ) + ) + + const page1 = await client.send( + new ListObjectsV2Command({ + Bucket: bucketName, + MaxKeys: 1, + }) + ) + expect(page1.Contents?.length).toBe(1) + expect(page1.NextContinuationToken).toBeTruthy() + + const page2 = await client.send( + new ListObjectsV2Command({ + Bucket: bucketName, + MaxKeys: 1, + ContinuationToken: page1.NextContinuationToken, + }) + ) + expect(page2.Contents?.length).toBe(1) + expect(page2.NextContinuationToken).toBeTruthy() + + const page3 = await client.send( + new ListObjectsV2Command({ + Bucket: bucketName, + MaxKeys: 1, + ContinuationToken: page2.NextContinuationToken, + }) + ) + expect(page3.Contents?.length).toBe(1) + + const pagedKeys = [ + page1.Contents?.[0]?.Key, + page2.Contents?.[0]?.Key, + page3.Contents?.[0]?.Key, + ].filter(Boolean) + expect(pagedKeys).toHaveLength(3) + expect(new Set(pagedKeys).size).toBe(3) + expect([...pagedKeys].sort()).toEqual([...keys].sort()) + }) + + it('can paginate ListMultipartUploads with Unicode and reserved characters in keys', async () => { + const bucketName = await createBucket(client) + const keys = [`mp-${randomUUID()}-🙂:one.jpg`, `mp-${randomUUID()}-일이삼:two.jpg`] + + await Promise.all( + keys.map((key) => + client.send( + new CreateMultipartUploadCommand({ + Bucket: bucketName, + Key: key, + ContentType: 'image/jpg', + }) + ) + ) + ) + + const page1 = await client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + MaxUploads: 1, + }) + ) + expect(page1.Uploads?.length).toBe(1) + expect(page1.NextKeyMarker).toBeTruthy() + + const page2 = await client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + MaxUploads: 1, + KeyMarker: page1.NextKeyMarker, + }) + ) + expect(page2.Uploads?.length).toBe(1) + + const pagedKeys = [page1.Uploads?.[0].Key, page2.Uploads?.[0].Key].filter(Boolean) + expect(pagedKeys).toHaveLength(2) + expect(new Set(pagedKeys).size).toBe(2) + expect([...pagedKeys].sort()).toEqual([...keys].sort()) + }) + it('can copy objects using Unicode keys in CopySource', async () => { const bucketName = await createBucket(client) const sourceKey = `copy-src-${randomUUID()}-일이삼-🙂.jpg` @@ -2233,6 +2331,37 @@ describe('S3 Protocol', () => { ) }) + it('can copy objects using fully URL-encoded CopySource', async () => { + const bucketName = await createBucket(client) + const sourceKey = getUnicodeObjectName() + const destinationKey = `copy-dst-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` @@ -2271,6 +2400,44 @@ describe('S3 Protocol', () => { expect(listPartsResp.Parts?.length).toBe(1) }) + it('can upload part copy using fully URL-encoded CopySource', async () => { + const bucketName = await createBucket(client) + const sourceKey = getUnicodeObjectName() + const destinationKey = `copy-part-dst-encoded-${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: encodeURIComponent(`${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() diff --git a/src/test/test-hygiene.test.ts b/src/test/test-hygiene.test.ts new file mode 100644 index 000000000..16fab66c4 --- /dev/null +++ b/src/test/test-hygiene.test.ts @@ -0,0 +1,56 @@ +import fs from 'fs' +import path from 'path' + +function collectTestFiles(dir: string): string[] { + const entries = fs.readdirSync(dir, { withFileTypes: true }) + const files: string[] = [] + + for (const entry of entries) { + const fullPath = path.join(dir, entry.name) + + if (entry.isDirectory()) { + files.push(...collectTestFiles(fullPath)) + continue + } + + if (entry.isFile() && fullPath.endsWith('.test.ts')) { + files.push(fullPath) + } + } + + return files +} + +describe('test hygiene', () => { + it('does not leak ad-hoc app instances in test files', () => { + const testFiles = collectTestFiles(__dirname) + const violations: string[] = [] + const forbiddenPattern = `app().${'inject('}` + const appFactoryPattern = /\b(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*app\(/g + + for (const file of testFiles) { + const content = fs.readFileSync(file, 'utf8') + const lines = content.split('\n') + + lines.forEach((line, index) => { + if (line.includes(forbiddenPattern)) { + violations.push(`${path.relative(process.cwd(), file)}:${index + 1}`) + } + }) + + let match = appFactoryPattern.exec(content) + while (match) { + const appVar = match[1] + if (!content.includes(`${appVar}.close(`)) { + const declarationLine = content.slice(0, match.index).split('\n').length + violations.push(`${path.relative(process.cwd(), file)}:${declarationLine}`) + } + match = appFactoryPattern.exec(content) + } + + appFactoryPattern.lastIndex = 0 + } + + expect(violations).toEqual([]) + }) +}) diff --git a/src/test/tus.test.ts b/src/test/tus.test.ts index 391985999..1458b8534 100644 --- a/src/test/tus.test.ts +++ b/src/test/tus.test.ts @@ -320,6 +320,50 @@ describe('Tus multipart', () => { }) }) + it('will allow uploading using signed upload url with a Unicode object key', async () => { + const bucket = await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const objectName = `${randomUUID()}-${getUnicodeObjectName()}` + const signedUpload = await storage + .from(bucketName) + .signUploadObjectUrl(objectName, `${bucketName}/${objectName}`, 3600) + + const result = await new Promise((resolve, reject) => { + const upload = new tus.Upload(oneChunkFile, { + endpoint: `${localServerAddress}/upload/resumable/sign`, + onShouldRetry: () => false, + uploadDataDuringCreation: false, + headers: { + 'x-signature': signedUpload.token, + }, + metadata: { + bucketName, + objectName, + contentType: 'image/jpeg', + cacheControl: '3600', + }, + onError(error) { + reject(error) + }, + onSuccess: () => { + resolve(true) + }, + }) + + upload.start() + }) + + expect(result).toEqual(true) + + const dbAsset = await storage.from(bucket.id).findObject(objectName, '*') + expect(dbAsset?.name).toBe(objectName) + expect(dbAsset?.bucket_id).toBe(bucket.id) + }) + it('will allow uploading using signed upload url without authorization token, honouring the owner id', async () => { const bucket = await storage.createBucket({ id: bucketName, From c36b32fe12472844e2709cc603c417c1cb172920 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 11:14:30 +0100 Subject: [PATCH 04/40] fix: make migration idempotent Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 28 ++++++++++++++----- src/scripts/migrations-types.ts | 2 +- src/storage/object.ts | 1 - 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index bc88f4090..c4d0ce499 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -1,7 +1,21 @@ -ALTER TABLE "storage"."objects" - ADD CONSTRAINT objects_name_check - CHECK ( - name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' - AND POSITION(U&'\FFFE' IN name) = 0 - AND POSITION(U&'\FFFF' IN name) = 0 - ); +DO $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 'objects_name_check' + AND n.nspname = 'storage' + AND t.relname = 'objects' + ) THEN + ALTER TABLE "storage"."objects" + ADD CONSTRAINT objects_name_check + CHECK ( + name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND POSITION(U&'\FFFE' IN name) = 0 + AND POSITION(U&'\FFFF' IN name) = 0 + ); + END IF; +END +$$; diff --git a/src/scripts/migrations-types.ts b/src/scripts/migrations-types.ts index 374142166..24fdcf9f9 100644 --- a/src/scripts/migrations-types.ts +++ b/src/scripts/migrations-types.ts @@ -39,7 +39,7 @@ function main() { const template = `export const DBMigration = { ${migrationsEnum.join('\n')} -} +} as const ` const destinationPath = path.resolve( diff --git a/src/storage/object.ts b/src/storage/object.ts index 0cd4d3305..433fdc738 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -330,7 +330,6 @@ export class ObjectStorage { throw ERRORS.NoSuchKey(sourceKey) } - // eslint-disable-next-line @typescript-eslint/no-unused-vars const baseMetadata = originObject.metadata || {} const destinationMetadata = copyMetadata ? baseMetadata From 0cfa647a24f96ecf499d2262d9796a80b2244ef4 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 11:32:50 +0100 Subject: [PATCH 05/40] fix: more tests for gaps Signed-off-by: ferhat elmas --- src/test/object.test.ts | 29 ++++++++++++ src/test/s3-protocol.test.ts | 90 ++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index a4ea99280..82f90ab58 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2943,6 +2943,35 @@ describe('Object key names with Unicode characters', () => { expect(deleteResponse.statusCode).toBe(200) }) + test('can upload and get via binary upload with a Unicode key', async () => { + const objectName = `binary-${randomUUID()}-일이삼-🙂.jpg` + const authorization = `Bearer ${await serviceKeyAsync}` + const path = './src/test/assets/sadcat.jpg' + const { size } = fs.statSync(path) + + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + 'Content-Length': size, + 'Content-Type': 'image/jpeg', + }, + payload: fs.createReadStream(path), + }) + expect(uploadResponse.statusCode).toBe(200) + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + }) + test('should not upload if the name contains invalid characters', async () => { const invalidObjectName = getInvalidObjectName() const authorization = `Bearer ${await serviceKeyAsync}` diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index e63f04e0d..670ba3da6 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -559,6 +559,39 @@ describe('S3 Protocol', () => { expect(resp.status).toBe(200) }) + it('can upload using multipart/form-data with a Unicode key', async () => { + const bucketName = await createBucket(client) + const key = `post-form-${randomUUID()}-中文-일이삼-🙂.jpg` + const signedURL = await createPresignedPost(client, { + Bucket: bucketName, + Key: key, + Expires: 5000, + Fields: { + 'Content-Type': 'image/jpg', + }, + }) + + const formData = new FormData() + Object.keys(signedURL.fields).forEach((fieldName) => { + formData.set(fieldName, signedURL.fields[fieldName]) + }) + + const data = Buffer.alloc(1024) + formData.set('file', new Blob([data]), 'upload.jpg') + + const resp = await axios.post(signedURL.url, formData, { + validateStatus: () => true, + }) + expect(resp.status).toBe(200) + + const listResp = await client.send( + new ListObjectsV2Command({ + Bucket: bucketName, + }) + ) + expect((listResp.Contents || []).map((item) => item.Key)).toEqual(expect.arrayContaining([key])) + }) + it('prevent uploading files larger than the maxFileSize limit', async () => { mergeConfig({ uploadFileSizeLimit: 1024, @@ -1189,6 +1222,24 @@ describe('S3 Protocol', () => { expect(resp.CopyObjectResult?.ETag).toBeTruthy() }) + it('will copy an object when CopySource is fully URL-encoded and includes versionId', async () => { + const bucketName = await createBucket(client) + const sourceKey = getUnicodeObjectName() + const destinationKey = `test-copied-versioned-encoded-${randomUUID()}.jpg` + + await uploadFile(client, bucketName, sourceKey, 1) + const sourceVersion = await getObjectVersion(bucketName, sourceKey) + + const copyObjectCommand = new CopyObjectCommand({ + Bucket: bucketName, + Key: destinationKey, + CopySource: `${encodeURIComponent(`${bucketName}/${sourceKey}`)}?versionId=${sourceVersion}`, + }) + + const resp = await client.send(copyObjectCommand) + expect(resp.CopyObjectResult?.ETag).toBeTruthy() + }) + it('will not copy an object when CopySource versionId does not match', async () => { const bucketName = await createBucket(client) const sourceKey = `test-copy-versioned-${randomUUID()}.jpg` @@ -1653,6 +1704,45 @@ describe('S3 Protocol', () => { expect(parts.Parts?.length).toBe(1) }) + it('will copy a part when CopySource is fully URL-encoded and includes versionId', async () => { + const bucket = await createBucket(client) + const sourceKey = getUnicodeObjectName() + const newKey = `new-versioned-encoded-${randomUUID()}.jpg` + + await uploadFile(client, bucket, sourceKey, 12) + const sourceVersion = await getObjectVersion(bucket, sourceKey) + + const createMultiPartUpload = new CreateMultipartUploadCommand({ + Bucket: bucket, + Key: newKey, + ContentType: 'image/jpg', + CacheControl: 'max-age=2000', + }) + const resp = await client.send(createMultiPartUpload) + expect(resp.UploadId).toBeTruthy() + + const copyPart = new UploadPartCopyCommand({ + Bucket: bucket, + Key: newKey, + UploadId: resp.UploadId, + PartNumber: 1, + CopySource: `${encodeURIComponent(`${bucket}/${sourceKey}`)}?versionId=${sourceVersion}`, + CopySourceRange: `bytes=0-${1024 * 4}`, + }) + + const copyResp = await client.send(copyPart) + expect(copyResp.CopyPartResult?.ETag).toBeTruthy() + + const listPartsCmd = new ListPartsCommand({ + Bucket: bucket, + Key: newKey, + UploadId: resp.UploadId, + }) + + const parts = await client.send(listPartsCmd) + expect(parts.Parts?.length).toBe(1) + }) + it('will not copy a part when CopySource versionId does not match', async () => { const bucket = await createBucket(client) const sourceKey = `${randomUUID()}.jpg` From 67636fc76563dc6e78d0f06f988ec2185d7ef166 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 11:42:56 +0100 Subject: [PATCH 06/40] fix: oriole compat Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index c4d0ce499..6f52e42e4 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -1,4 +1,6 @@ DO $$ +DECLARE + server_encoding text := current_setting('server_encoding'); BEGIN IF NOT EXISTS ( SELECT 1 @@ -9,13 +11,25 @@ BEGIN AND n.nspname = 'storage' AND t.relname = 'objects' ) THEN - ALTER TABLE "storage"."objects" - ADD CONSTRAINT objects_name_check - CHECK ( - name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' - AND POSITION(U&'\FFFE' IN name) = 0 - AND POSITION(U&'\FFFF' IN name) = 0 - ); + IF server_encoding = 'SQL_ASCII' THEN + EXECUTE $ddl$ + ALTER TABLE "storage"."objects" + ADD CONSTRAINT objects_name_check + CHECK ( + name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + ) + $ddl$; + ELSE + EXECUTE $ddl$ + ALTER TABLE "storage"."objects" + ADD CONSTRAINT objects_name_check + CHECK ( + name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND POSITION(U&'\FFFE' IN name) = 0 + AND POSITION(U&'\FFFF' IN name) = 0 + ) + $ddl$; + END IF; END IF; END $$; From 42639c356404dcf41d48698a976aeb72a5d81cf2 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 11:46:27 +0100 Subject: [PATCH 07/40] fix: lint Signed-off-by: ferhat elmas --- src/test/s3-protocol.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index 670ba3da6..87eb5d60e 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -589,7 +589,9 @@ describe('S3 Protocol', () => { Bucket: bucketName, }) ) - expect((listResp.Contents || []).map((item) => item.Key)).toEqual(expect.arrayContaining([key])) + expect((listResp.Contents || []).map((item) => item.Key)).toEqual( + expect.arrayContaining([key]) + ) }) it('prevent uploading files larger than the maxFileSize limit', async () => { From 7a0d66e26adce22b5963f717c5bb94378f1b2ca1 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 16:42:45 +0100 Subject: [PATCH 08/40] fix: add delete many test Signed-off-by: ferhat elmas --- src/test/object.test.ts | 59 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 82f90ab58..1f05d685e 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -1645,6 +1645,65 @@ describe('testing deleting multiple objects', () => { expect(results).toHaveLength(1) expect(results[0].name).toBe('authenticated/delete-multiple7.png') }) + + test('can delete multiple objects with Unicode keys', async () => { + const authorization = `Bearer ${await serviceKeyAsync}` + const path = './src/test/assets/sadcat.jpg' + const { size } = fs.statSync(path) + const prefixes = [ + `authenticated/delete-many-${randomUUID()}-일이삼-🙂.png`, + `authenticated/delete-many-${randomUUID()}-éè-中文.png`, + ] + + for (const prefix of prefixes) { + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket2/${encodeURIComponent(prefix)}`, + headers: { + authorization, + 'Content-Length': size, + 'Content-Type': 'image/jpeg', + }, + payload: fs.createReadStream(path), + }) + + expect(uploadResponse.statusCode).toBe(200) + } + + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: '/object/bucket2', + headers: { + authorization, + }, + payload: { + prefixes, + }, + }) + + expect(deleteResponse.statusCode).toBe(200) + expect(S3Backend.prototype.deleteObjects).toBeCalled() + const results = JSON.parse(deleteResponse.body) + expect(results).toHaveLength(2) + expect(results.map((item: { name: string }) => item.name).sort()).toEqual([...prefixes].sort()) + + for (const prefix of prefixes) { + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/bucket2/${encodeURIComponent(prefix)}`, + headers: { + authorization, + }, + }) + + expect(getResponse.statusCode).toBe(400) + expect(getResponse.json()).toMatchObject({ + statusCode: '404', + error: 'not_found', + message: 'Object not found', + }) + } + }) }) /** From fed0dd7080841448dc6fed0e6d995383dbe0c89c Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 17:02:37 +0100 Subject: [PATCH 09/40] fix: decode logic in xml Signed-off-by: ferhat elmas --- src/http/plugins/xml.ts | 17 ++++++++++++++++- src/test/xml-plugin.test.ts | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/http/plugins/xml.ts b/src/http/plugins/xml.ts index bdeb8a016..693fa0c26 100644 --- a/src/http/plugins/xml.ts +++ b/src/http/plugins/xml.ts @@ -7,10 +7,25 @@ type XmlParserOptions = { disableContentParser?: boolean; parseAsArray?: string[ type RequestError = Error & { statusCode?: number } export function decodeXmlNumericEntities(value: string): string { + const isValidXmlCodePoint = (codePoint: number) => { + if (!Number.isInteger(codePoint) || codePoint < 0 || codePoint > 0x10ffff) { + return false + } + + return ( + codePoint === 0x9 || + codePoint === 0xa || + codePoint === 0xd || + (codePoint >= 0x20 && codePoint <= 0xd7ff) || + (codePoint >= 0xe000 && codePoint <= 0xfffd) || + (codePoint >= 0x10000 && codePoint <= 0x10ffff) + ) + } + return value.replace(/&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, (match: string, rawValue: string) => { const isHex = rawValue[0].toLowerCase() === 'x' const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) - if (codePoint > 0x10ffff) { + if (!isValidXmlCodePoint(codePoint)) { return match } diff --git a/src/test/xml-plugin.test.ts b/src/test/xml-plugin.test.ts index 2987ea601..79cd24e3e 100644 --- a/src/test/xml-plugin.test.ts +++ b/src/test/xml-plugin.test.ts @@ -12,4 +12,20 @@ describe('decodeXmlNumericEntities', () => { test('keeps out-of-range entities unchanged', () => { expect(decodeXmlNumericEntities('a�b')).toBe('a�b') }) + + test('keeps NUL entities unchanged', () => { + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + }) + + test('keeps surrogate-half entities unchanged', () => { + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + expect(decodeXmlNumericEntities('a�b')).toBe('a�b') + }) + + test('keeps noncharacter entities unchanged', () => { + expect(decodeXmlNumericEntities('a￿b')).toBe('a￿b') + expect(decodeXmlNumericEntities('a￿b')).toBe('a￿b') + }) }) From c5484c2b592d45f894d708ca6042910b2120994b Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 17:21:32 +0100 Subject: [PATCH 10/40] fix: make delete many example more complex Signed-off-by: ferhat elmas --- src/test/object.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 1f05d685e..ec47bf881 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -1652,7 +1652,7 @@ describe('testing deleting multiple objects', () => { const { size } = fs.statSync(path) const prefixes = [ `authenticated/delete-many-${randomUUID()}-일이삼-🙂.png`, - `authenticated/delete-many-${randomUUID()}-éè-中文.png`, + `authenticated/delete-many-${randomUUID()}-éè-中文-?query&x=1#frag%25+plus;semi:colon,.png`, ] for (const prefix of prefixes) { From 0849e86ba945578c09bf0ed1b73ed79d5e95107d Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 20:12:57 +0100 Subject: [PATCH 11/40] fix: more tests for gaps Signed-off-by: ferhat elmas --- src/http/routes/object/getSignedURL.ts | 3 +- src/storage/object.ts | 11 +- src/test/object.test.ts | 246 +++++++++++++++++++++++++ src/test/s3-protocol.test.ts | 54 ++++++ src/test/tus.test.ts | 193 +++++++++++++++++++ 5 files changed, 501 insertions(+), 6 deletions(-) diff --git a/src/http/routes/object/getSignedURL.ts b/src/http/routes/object/getSignedURL.ts index bf0aa8416..a5d03fc39 100644 --- a/src/http/routes/object/getSignedURL.ts +++ b/src/http/routes/object/getSignedURL.ts @@ -67,7 +67,6 @@ export default async function routes(fastify: FastifyInstance) { const objectName = request.params['*'] const { expiresIn } = request.body - const urlPath = request.url.split('?').shift() const imageTransformationEnabled = await isImageTransformationEnabled(request.tenantId) const transformationOptions = imageTransformationEnabled @@ -82,7 +81,7 @@ export default async function routes(fastify: FastifyInstance) { const signedURL = await request.storage .from(bucketName) - .signObjectUrl(objectName, urlPath as string, expiresIn, transformationOptions) + .signObjectUrl(objectName, expiresIn, transformationOptions) return response.status(200).send({ signedURL }) } diff --git a/src/storage/object.ts b/src/storage/object.ts index 433fdc738..f5796fe44 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -705,7 +705,6 @@ export class ObjectStorage { */ async signObjectUrl( objectName: string, - url: string, expiresIn: number, metadata?: Record ) { @@ -722,8 +721,7 @@ export class ObjectStorage { // make sure it's never able to specify a role JWT claim delete metadata['role'] - const urlParts = url.split('/') - const urlToSign = decodeURI(urlParts.splice(3).join('/')) + const urlToSign = `${this.bucketId}/${objectName}` const { urlSigningKey } = await getJwtSecret(this.db.tenantId) const token = await signJWT({ url: urlToSign, ...metadata }, urlSigningKey, expiresIn) @@ -733,8 +731,13 @@ export class ObjectStorage { urlPath = 'render/image' } + const encodedUrlToSign = `${encodeURIComponent(this.bucketId)}/${objectName + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` + // @todo parse the url properly - return `/${urlPath}/sign/${urlToSign}?token=${token}` + return `/${urlPath}/sign/${encodedUrlToSign}?token=${token}` } /** diff --git a/src/test/object.test.ts b/src/test/object.test.ts index ec47bf881..ceec1c572 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -18,6 +18,7 @@ import { withDeleteEnabled } from './utils/storage' const { jwtSecret, serviceKeyAsync, tenantId } = getConfig() const anonKey = process.env.ANON_KEY || '' const S3Backend = backends.S3Backend +const TEST_OWNER_ID = '317eadce-631a-4429-a0bb-f19a7a517b4a' let appInstance: FastifyInstance let tnx: Knex.Transaction | undefined @@ -35,6 +36,19 @@ async function getSuperuserPostgrestClient() { return tnx } +async function seedObjectForRouteTest(name: string, bucketId = 'bucket2') { + const seedTx = await getSuperuserPostgrestClient() + await seedTx.from('objects').insert({ + bucket_id: bucketId, + name, + owner: TEST_OWNER_ID, + version: `seed-version-${randomUUID()}`, + metadata: { mimetype: 'image/png', size: 1234 }, + }) + await seedTx.commit() + tnx = undefined +} + useMockObject() useMockQueue() @@ -1479,6 +1493,56 @@ describe('testing copy object', () => { expect(response.statusCode).toBe(400) expect(S3Backend.prototype.copyObject).not.toHaveBeenCalled() }) + + test('can copy objects when keys include ASCII URL-reserved characters', async () => { + const sourceKey = `authenticated/copy-src-${randomUUID()}-q?foo=1&bar=%25+plus;semi:colon,.png` + const destinationKey = `authenticated/copy-dst-${randomUUID()}-q?foo=2&bar=%25+plus;semi:colon,.png` + await seedObjectForRouteTest(sourceKey) + + const response = await appInstance.inject({ + method: 'POST', + url: '/object/copy', + headers: { + authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`, + }, + payload: { + bucketId: 'bucket2', + sourceKey, + destinationKey, + }, + }) + + expect(response.statusCode).toBe(200) + expect(S3Backend.prototype.copyObject).toBeCalled() + const jsonResponse = response.json() + expect(jsonResponse.Key).toBe(`bucket2/${destinationKey}`) + expect(jsonResponse.name).toBe(destinationKey) + }) + + test('can copy objects when keys include Unicode and URL-reserved characters', async () => { + const sourceKey = `authenticated/copy-src-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,.png` + const destinationKey = `authenticated/copy-dst-${randomUUID()}-éè-中文-🙂-q?foo=2&bar=%25+plus;semi:colon,.png` + await seedObjectForRouteTest(sourceKey) + + const response = await appInstance.inject({ + method: 'POST', + url: '/object/copy', + headers: { + authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`, + }, + payload: { + bucketId: 'bucket2', + sourceKey, + destinationKey, + }, + }) + + expect(response.statusCode).toBe(200) + expect(S3Backend.prototype.copyObject).toBeCalled() + const jsonResponse = response.json() + expect(jsonResponse.Key).toBe(`bucket2/${destinationKey}`) + expect(jsonResponse.name).toBe(destinationKey) + }) }) /** @@ -2482,6 +2546,72 @@ describe('testing move object', () => { expect(S3Backend.prototype.copyObject).not.toHaveBeenCalled() expect(S3Backend.prototype.deleteObject).not.toHaveBeenCalled() }) + + test('can move objects when keys include ASCII URL-reserved characters', async () => { + const sourceKey = `authenticated/move-src-${randomUUID()}-q?foo=1&bar=%25+plus;semi:colon,.png` + const destinationKey = `authenticated/move-dst-${randomUUID()}-q?foo=2&bar=%25+plus;semi:colon,.png` + await seedObjectForRouteTest(sourceKey) + + const response = await appInstance.inject({ + method: 'POST', + url: '/object/move', + payload: { + bucketId: 'bucket2', + sourceKey, + destinationKey, + }, + headers: { + authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`, + }, + }) + + expect(response.statusCode).toBe(200) + expect(S3Backend.prototype.copyObject).toHaveBeenCalled() + expect(S3Backend.prototype.deleteObjects).toHaveBeenCalled() + expect(response.json().message).toBe('Successfully moved') + + const conn = await getSuperuserPostgrestClient() + const movedObject = await conn + .table('objects') + .select('name') + .where('bucket_id', 'bucket2') + .where('name', destinationKey) + .first() + expect(movedObject?.name).toBe(destinationKey) + }) + + test('can move objects when keys include Unicode and URL-reserved characters', async () => { + const sourceKey = `authenticated/move-src-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,.png` + const destinationKey = `authenticated/move-dst-${randomUUID()}-éè-中文-🙂-q?foo=2&bar=%25+plus;semi:colon,.png` + await seedObjectForRouteTest(sourceKey) + + const response = await appInstance.inject({ + method: 'POST', + url: '/object/move', + payload: { + bucketId: 'bucket2', + sourceKey, + destinationKey, + }, + headers: { + authorization: `Bearer ${process.env.AUTHENTICATED_KEY}`, + }, + }) + + expect(response.statusCode).toBe(200) + expect(S3Backend.prototype.copyObject).toHaveBeenCalled() + expect(S3Backend.prototype.deleteObjects).toHaveBeenCalled() + expect(response.json().message).toBe('Successfully moved') + + const conn = await getSuperuserPostgrestClient() + const movedObject = await conn + .table('objects') + .select('name') + .where('bucket_id', 'bucket2') + .where('name', destinationKey) + .first() + expect(movedObject?.name).toBe(destinationKey) + }) }) describe('testing list objects', () => { @@ -3031,6 +3161,38 @@ describe('Object key names with Unicode characters', () => { expect(getResponse.headers['etag']).toBe('abc') }) + test('can upload and read HEAD info with a Unicode and URL-reserved key', async () => { + const objectName = `head-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg` + const authorization = `Bearer ${await serviceKeyAsync}` + const path = './src/test/assets/sadcat.jpg' + const { size } = fs.statSync(path) + + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + 'Content-Length': size, + 'Content-Type': 'image/jpeg', + }, + payload: fs.createReadStream(path), + }) + expect(uploadResponse.statusCode).toBe(200) + + const headResponse = await appInstance.inject({ + method: 'HEAD', + url: `/object/authenticated/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(headResponse.statusCode).toBe(200) + expect(headResponse.headers['etag']).toBe('abc') + expect(headResponse.headers['last-modified']).toBeTruthy() + expect(headResponse.headers['content-length']).toBeTruthy() + expect(headResponse.headers['cache-control']).toBe('no-cache') + }) + test('should not upload if the name contains invalid characters', async () => { const invalidObjectName = getInvalidObjectName() const authorization = `Bearer ${await serviceKeyAsync}` @@ -3148,6 +3310,90 @@ describe('Object key names with Unicode characters', () => { expect(objectResponse?.name).toBe(objectName) }) + test('can generate and use a signed download URL with Unicode and URL-reserved characters', async () => { + const objectName = `signed-download-reserved-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadHeaders = Object.assign({}, form.getHeaders(), { + authorization, + 'x-upsert': 'true', + }) + + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: uploadHeaders, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + + const signResponse = await appInstance.inject({ + method: 'POST', + url: `/object/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + payload: { + expiresIn: 60, + }, + }) + expect(signResponse.statusCode).toBe(200) + + const signedURL = signResponse.json<{ signedURL: string }>().signedURL + const signedURLParsed = new URL(signedURL, 'http://localhost') + const token = signedURLParsed.searchParams.get('token') + expect(token).toBeTruthy() + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `${signedURLParsed.pathname}${signedURLParsed.search}`, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + }) + + test('can generate and use a signed upload URL with Unicode and URL-reserved characters', async () => { + const objectName = `signed-upload-reserved-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const signedUploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(signedUploadResponse.statusCode).toBe(200) + + const token = signedUploadResponse.json<{ token: string }>().token + expect(token).toBeTruthy() + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}?token=${token}`, + headers: { + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + + const db = await getSuperuserPostgrestClient() + const objectResponse = await db + .from('objects') + .select('*') + .where({ + name: objectName, + bucket_id: 'bucket2', + }) + .first() + expect(objectResponse?.name).toBe(objectName) + }) + test('should not generate signed upload URL for invalid key', async () => { const invalidObjectName = getInvalidObjectName() const authorization = `Bearer ${await serviceKeyAsync}` diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index 87eb5d60e..5f00e537f 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -2282,6 +2282,60 @@ describe('S3 Protocol', () => { } }) + it('can head an object with Unicode and URL-reserved characters in key', async () => { + const bucketName = await createBucket(client) + const objectName = `head-${randomUUID()}-일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg` + + await client.send( + new PutObjectCommand({ + Bucket: bucketName, + Key: objectName, + Body: Buffer.alloc(1024), + }) + ) + + const headResp = await client.send( + new HeadObjectCommand({ + Bucket: bucketName, + Key: objectName, + }) + ) + expect(headResp.$metadata.httpStatusCode).toEqual(200) + expect(headResp.ETag).toBeTruthy() + }) + + it('can delete an object with Unicode and URL-reserved characters using DeleteObjectCommand', async () => { + const bucketName = await createBucket(client) + const objectName = `delete-one-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg` + + await client.send( + new PutObjectCommand({ + Bucket: bucketName, + Key: objectName, + Body: Buffer.alloc(1024), + }) + ) + + await client.send( + new DeleteObjectCommand({ + Bucket: bucketName, + Key: objectName, + }) + ) + + try { + await client.send( + new GetObjectCommand({ + Bucket: bucketName, + Key: objectName, + }) + ) + throw new Error('Should not reach here') + } catch (e) { + expect((e as S3ServiceException).$metadata.httpStatusCode).toEqual(404) + } + }) + it('can paginate ListObjectsV2 with Unicode keys', async () => { const bucketName = await createBucket(client) const keys = [ diff --git a/src/test/tus.test.ts b/src/test/tus.test.ts index 1458b8534..f79013db9 100644 --- a/src/test/tus.test.ts +++ b/src/test/tus.test.ts @@ -576,6 +576,199 @@ describe('Tus multipart', () => { }) }) + describe('TUS control endpoints', () => { + function encodeTusMetadataValue(value: string) { + return Buffer.from(value, 'utf8').toString('base64') + } + + function buildTusMetadata(objectName: string) { + return [ + `bucketName ${encodeTusMetadataValue(bucketName)}`, + `objectName ${encodeTusMetadataValue(objectName)}`, + `contentType ${encodeTusMetadataValue('image/jpeg')}`, + `cacheControl ${encodeTusMetadataValue('3600')}`, + ].join(',') + } + + async function createTusUploadSession(objectName: string, authorization: string) { + const createResponse = await fetch(`${localServerAddress}/upload/resumable`, { + method: 'POST', + headers: { + authorization, + 'x-upsert': 'true', + 'tus-resumable': '1.0.0', + 'upload-length': '32', + 'upload-metadata': buildTusMetadata(objectName), + }, + }) + + expect(createResponse.status).toBe(201) + const location = createResponse.headers.get('location') + expect(location).toBeTruthy() + + return new URL(location || '', localServerAddress).toString() + } + + async function patchTusUploadSession(uploadUrl: string, authorization: string, body: Buffer) { + const patchResponse = await fetch(uploadUrl, { + method: 'PATCH', + headers: { + authorization, + 'tus-resumable': '1.0.0', + 'upload-offset': '0', + 'content-type': 'application/offset+octet-stream', + 'content-length': String(body.length), + }, + body, + }) + + expect(patchResponse.status).toBe(204) + expect(patchResponse.headers.get('upload-offset')).toBe(String(body.length)) + } + + test('supports HEAD and DELETE flow for ASCII object keys', async () => { + await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const authorization = `Bearer ${await serviceKeyAsync}` + const objectName = `${randomUUID()}-ascii-control-q?foo=1&bar=%25+plus;semi:colon,.jpg` + const uploadUrl = await createTusUploadSession(objectName, authorization) + + const headBeforeDelete = await fetch(uploadUrl, { + method: 'HEAD', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect(headBeforeDelete.status).toBe(200) + expect(headBeforeDelete.headers.get('upload-offset')).toBe('0') + expect(headBeforeDelete.headers.get('upload-length')).toBe('32') + + const deleteResponse = await fetch(uploadUrl, { + method: 'DELETE', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect([200, 204]).toContain(deleteResponse.status) + + const headAfterDelete = await fetch(uploadUrl, { + method: 'HEAD', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect([404, 410]).toContain(headAfterDelete.status) + }) + + test('supports HEAD and DELETE flow for Unicode object keys', async () => { + await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const authorization = `Bearer ${await serviceKeyAsync}` + const objectName = `${randomUUID()}-${getUnicodeObjectName()}` + const uploadUrl = await createTusUploadSession(objectName, authorization) + + const headBeforeDelete = await fetch(uploadUrl, { + method: 'HEAD', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect(headBeforeDelete.status).toBe(200) + expect(headBeforeDelete.headers.get('upload-offset')).toBe('0') + expect(headBeforeDelete.headers.get('upload-length')).toBe('32') + + const deleteResponse = await fetch(uploadUrl, { + method: 'DELETE', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect([200, 204]).toContain(deleteResponse.status) + + const headAfterDelete = await fetch(uploadUrl, { + method: 'HEAD', + headers: { + authorization, + 'tus-resumable': '1.0.0', + }, + }) + expect([404, 410]).toContain(headAfterDelete.status) + }) + + test('supports upload completion and object GET for ASCII URL-reserved keys', async () => { + const bucket = await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const authorization = `Bearer ${await serviceKeyAsync}` + const objectName = `${randomUUID()}-ascii-get-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg` + const uploadUrl = await createTusUploadSession(objectName, authorization) + const payload = Buffer.from('abcdefghijklmnopqrstuvwxyz012345') + + await patchTusUploadSession(uploadUrl, authorization, payload) + + const appInstance = app() + try { + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/${bucket.id}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['content-length']).toBe(String(payload.length)) + } finally { + await appInstance.close() + } + }) + + test('supports upload completion and object GET for Unicode URL-reserved keys', async () => { + const bucket = await storage.createBucket({ + id: bucketName, + name: bucketName, + public: true, + }) + + const authorization = `Bearer ${await serviceKeyAsync}` + const objectName = `${randomUUID()}-${getUnicodeObjectName()}-q?foo=1&bar=%25+plus;semi:colon,#frag.jpg` + const uploadUrl = await createTusUploadSession(objectName, authorization) + const payload = Buffer.from('abcdefghijklmnopqrstuvwxyz012345') + + await patchTusUploadSession(uploadUrl, authorization, payload) + + const appInstance = app() + try { + const getResponse = await appInstance.inject({ + method: 'GET', + url: `/object/${bucket.id}/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['content-length']).toBe(String(payload.length)) + } finally { + await appInstance.close() + } + }) + }) + describe('Object key names with Unicode characters', () => { it('can be uploaded with the TUS protocol', async () => { const objectName = randomUUID() + '-' + getUnicodeObjectName() From 19a6b3889921baf5ba242b3079b71de0481e3605 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 20:22:50 +0100 Subject: [PATCH 12/40] fix: invalid key surrogates Signed-off-by: ferhat elmas --- src/internal/errors/codes.ts | 42 +++++++++++++++++++++++++++++++++++- src/test/error-codes.test.ts | 26 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/test/error-codes.test.ts diff --git a/src/internal/errors/codes.ts b/src/internal/errors/codes.ts index 4ab74a538..c7d05a4ec 100644 --- a/src/internal/errors/codes.ts +++ b/src/internal/errors/codes.ts @@ -1,5 +1,45 @@ import { StorageBackendError } from './storage-error' +function toWellFormedString(value: string): string { + const maybeToWellFormed = (value as unknown as { toWellFormed?: () => string }).toWellFormed + if (typeof maybeToWellFormed === 'function') { + return maybeToWellFormed.call(value) + } + + let normalized = '' + for (let i = 0; i < value.length; i++) { + const currentCodeUnit = value.charCodeAt(i) + + if (currentCodeUnit >= 0xd800 && currentCodeUnit <= 0xdbff) { + const nextCodeUnit = value.charCodeAt(i + 1) + if (i + 1 < value.length && nextCodeUnit >= 0xdc00 && nextCodeUnit <= 0xdfff) { + normalized += value[i] + value[i + 1] + i += 1 + } else { + normalized += '\uFFFD' + } + continue + } + + if (currentCodeUnit >= 0xdc00 && currentCodeUnit <= 0xdfff) { + normalized += '\uFFFD' + continue + } + + normalized += value[i] + } + + return normalized +} + +function safeEncodeURIComponent(value: string): string { + try { + return encodeURIComponent(value) + } catch { + return encodeURIComponent(toWellFormedString(value)) + } +} + export enum ErrorCode { NoSuchBucket = 'NoSuchBucket', NoSuchKey = 'NoSuchKey', @@ -324,7 +364,7 @@ export const ERRORS = { code: ErrorCode.InvalidKey, resource: key, httpStatusCode: 400, - message: `Invalid key: ${encodeURIComponent(key)}`, + message: `Invalid key: ${safeEncodeURIComponent(key)}`, originalError: e, }), diff --git a/src/test/error-codes.test.ts b/src/test/error-codes.test.ts new file mode 100644 index 000000000..c62b4a5c3 --- /dev/null +++ b/src/test/error-codes.test.ts @@ -0,0 +1,26 @@ +import { ERRORS, ErrorCode, StorageBackendError } from '@internal/errors' + +describe('ERRORS.InvalidKey', () => { + it('does not throw for unpaired high surrogates', () => { + const malformedKey = 'bad-\uD800-key' + + expect(() => ERRORS.InvalidKey(malformedKey)).not.toThrow() + + const error = ERRORS.InvalidKey(malformedKey) + expect(error).toBeInstanceOf(StorageBackendError) + expect(error.code).toBe(ErrorCode.InvalidKey) + expect(error.httpStatusCode).toBe(400) + expect(error.message).toBe('Invalid key: bad-%EF%BF%BD-key') + }) + + it('does not throw for unpaired low surrogates', () => { + const malformedKey = 'bad-\uDC00-key' + + expect(() => ERRORS.InvalidKey(malformedKey)).not.toThrow() + + const error = ERRORS.InvalidKey(malformedKey) + expect(error.code).toBe(ErrorCode.InvalidKey) + expect(error.httpStatusCode).toBe(400) + expect(error.message).toBe('Invalid key: bad-%EF%BF%BD-key') + }) +}) From 0b61d3661cc33192747ddc436ce2fc687b8e3ddf Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 20:38:33 +0100 Subject: [PATCH 13/40] fix: sign and more coverage Signed-off-by: ferhat elmas --- src/storage/object.ts | 24 ++++++++++++------ src/test/object.test.ts | 55 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 10 deletions(-) diff --git a/src/storage/object.ts b/src/storage/object.ts index f5796fe44..b3e680ad4 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -49,6 +49,13 @@ export interface ListObjectsV2Result { nextCursorKey?: string } +function encodeObjectPathForURL(bucketId: string, objectName: string): string { + return `${encodeURIComponent(bucketId)}/${objectName + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` +} + /** * ObjectStorage * interact with remote objects and database state @@ -731,10 +738,7 @@ export class ObjectStorage { urlPath = 'render/image' } - const encodedUrlToSign = `${encodeURIComponent(this.bucketId)}/${objectName - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` + const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, objectName) // @todo parse the url properly return `/${urlPath}/sign/${encodedUrlToSign}?token=${token}` @@ -773,7 +777,8 @@ export class ObjectStorage { if (nameSet.has(path)) { const urlToSign = `${this.bucketId}/${path}` const token = await signJWT({ url: urlToSign }, urlSigningKey, expiresIn) - signedURL = `/object/sign/${urlToSign}?token=${token}` + const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, path) + signedURL = `/object/sign/${encodedUrlToSign}?token=${token}` } else { error = 'Either the object does not exist or you do not have access to it' } @@ -796,7 +801,7 @@ export class ObjectStorage { */ async signUploadObjectUrl( objectName: string, - url: string, + _url: string, expiresIn: number, owner?: string, options?: { upsert?: boolean } @@ -812,13 +817,16 @@ export class ObjectStorage { }) const { urlSigningKey } = await getJwtSecret(this.db.tenantId) + const urlToSign = `${this.bucketId}/${objectName}` const token = await signJWT( - { owner, url, upsert: Boolean(options?.upsert) }, + { owner, url: urlToSign, upsert: Boolean(options?.upsert) }, urlSigningKey, expiresIn ) - return { url: `/object/upload/sign/${url}?token=${token}`, token } + const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, objectName) + + return { url: `/object/upload/sign/${encodedUrlToSign}?token=${token}`, token } } /** diff --git a/src/test/object.test.ts b/src/test/object.test.ts index ceec1c572..fea71efba 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2255,6 +2255,54 @@ describe('testing generating signed URLs', () => { const result = JSON.parse(response.body) expect(result[0].error).toBe('Either the object does not exist or you do not have access to it') }) + + test('can generate and use batch signed URLs with Unicode and URL-reserved characters', async () => { + const objectName = `signed-batch-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + const authorization = `Bearer ${await serviceKeyAsync}` + const path = './src/test/assets/sadcat.jpg' + const { size } = fs.statSync(path) + + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + 'Content-Length': size, + 'Content-Type': 'image/jpeg', + }, + payload: fs.createReadStream(path), + }) + expect(uploadResponse.statusCode).toBe(200) + + const signResponse = await appInstance.inject({ + method: 'POST', + url: '/object/sign/bucket2', + headers: { + authorization, + }, + payload: { + expiresIn: 60, + paths: [objectName], + }, + }) + expect(signResponse.statusCode).toBe(200) + + const [signedObject] = + signResponse.json<{ error: string | null; path: string; signedURL: string | null }[]>() + expect(signedObject.error).toBeNull() + expect(signedObject.path).toBe(objectName) + expect(signedObject.signedURL).toBeTruthy() + + const signedURLParsed = new URL(signedObject.signedURL || '', 'http://localhost') + expect(signedURLParsed.searchParams.get('token')).toBeTruthy() + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `${signedURLParsed.pathname}${signedURLParsed.search}`, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + }) }) /** @@ -3367,14 +3415,17 @@ describe('Object key names with Unicode characters', () => { }) expect(signedUploadResponse.statusCode).toBe(200) - const token = signedUploadResponse.json<{ token: string }>().token + const signedUpload = signedUploadResponse.json<{ token: string; url: string }>() + const token = signedUpload.token expect(token).toBeTruthy() + const signedUploadURL = new URL(signedUpload.url, 'http://localhost') + expect(signedUploadURL.searchParams.get('token')).toBe(token) const form = new FormData() form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) const uploadResponse = await appInstance.inject({ method: 'PUT', - url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}?token=${token}`, + url: `${signedUploadURL.pathname}${signedUploadURL.search}`, headers: { ...form.getHeaders(), }, From a7e728cc94321cc74131e678ec429a2600b7c626 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 20:51:50 +0100 Subject: [PATCH 14/40] fix: more batch sign coverage Signed-off-by: ferhat elmas --- src/test/object.test.ts | 99 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index fea71efba..f5a2345d1 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2303,6 +2303,63 @@ describe('testing generating signed URLs', () => { expect(getResponse.statusCode).toBe(200) expect(getResponse.headers['etag']).toBe('abc') }) + + test('can generate and use batch signed URLs for nested Unicode and URL-reserved paths', async () => { + const authorization = `Bearer ${await serviceKeyAsync}` + const path = './src/test/assets/sadcat.jpg' + const { size } = fs.statSync(path) + const objectNames = [ + `signed-batch-${randomUUID()}-폴더?x=1/세그먼트#tag/leaf+plus;semi,.png`, + `signed-batch-${randomUUID()}-éè/中文&name=1/끝🙂.png`, + ] + + for (const objectName of objectNames) { + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + 'Content-Length': size, + 'Content-Type': 'image/jpeg', + }, + payload: fs.createReadStream(path), + }) + expect(uploadResponse.statusCode).toBe(200) + } + + const signResponse = await appInstance.inject({ + method: 'POST', + url: '/object/sign/bucket2', + headers: { + authorization, + }, + payload: { + expiresIn: 60, + paths: objectNames, + }, + }) + expect(signResponse.statusCode).toBe(200) + + const signedObjects = + signResponse.json<{ error: string | null; path: string; signedURL: string | null }[]>() + expect(signedObjects).toHaveLength(2) + + for (const signedObject of signedObjects) { + expect(signedObject.error).toBeNull() + expect(objectNames).toContain(signedObject.path) + expect(signedObject.signedURL).toBeTruthy() + + const signedURLParsed = new URL(signedObject.signedURL || '', 'http://localhost') + expect(signedURLParsed.searchParams.get('token')).toBeTruthy() + + const getResponse = await appInstance.inject({ + method: 'GET', + url: `${signedURLParsed.pathname}${signedURLParsed.search}`, + }) + expect(getResponse.statusCode).toBe(200) + expect(getResponse.headers['etag']).toBe('abc') + } + }) }) /** @@ -3445,6 +3502,48 @@ describe('Object key names with Unicode characters', () => { expect(objectResponse?.name).toBe(objectName) }) + test('can sign and upload using returned signed upload URL for nested Unicode and URL-reserved object names', async () => { + const objectName = `signed-upload-unicode-${randomUUID()}-폴더?x=1&y=%25+plus;semi:colon,/子目录#frag/파일-🙂.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const signedUploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(signedUploadResponse.statusCode).toBe(200) + + const signedUpload = signedUploadResponse.json<{ token: string; url: string }>() + expect(signedUpload.token).toBeTruthy() + const signedUploadURL = new URL(signedUpload.url, 'http://localhost') + expect(signedUploadURL.searchParams.get('token')).toBe(signedUpload.token) + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `${signedUploadURL.pathname}${signedUploadURL.search}`, + headers: { + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + + const db = await getSuperuserPostgrestClient() + const objectResponse = await db + .from('objects') + .select('*') + .where({ + name: objectName, + bucket_id: 'bucket2', + }) + .first() + expect(objectResponse?.name).toBe(objectName) + }) + test('should not generate signed upload URL for invalid key', async () => { const invalidObjectName = getInvalidObjectName() const authorization = `Bearer ${await serviceKeyAsync}` From 56e72eee8253cb099bfb6bd8d6f0e7122aaa20a9 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 21:12:53 +0100 Subject: [PATCH 15/40] fix: cleanup data after run Signed-off-by: ferhat elmas --- src/test/object.test.ts | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index f5a2345d1..4fb840634 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2257,7 +2257,7 @@ describe('testing generating signed URLs', () => { }) test('can generate and use batch signed URLs with Unicode and URL-reserved characters', async () => { - const objectName = `signed-batch-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + const objectName = `authenticated/signed-batch-${randomUUID()}-éè-中文-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` const authorization = `Bearer ${await serviceKeyAsync}` const path = './src/test/assets/sadcat.jpg' const { size } = fs.statSync(path) @@ -2302,6 +2302,15 @@ describe('testing generating signed URLs', () => { }) expect(getResponse.statusCode).toBe(200) expect(getResponse.headers['etag']).toBe('abc') + + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(deleteResponse.statusCode).toBe(200) }) test('can generate and use batch signed URLs for nested Unicode and URL-reserved paths', async () => { @@ -2309,8 +2318,8 @@ describe('testing generating signed URLs', () => { const path = './src/test/assets/sadcat.jpg' const { size } = fs.statSync(path) const objectNames = [ - `signed-batch-${randomUUID()}-폴더?x=1/세그먼트#tag/leaf+plus;semi,.png`, - `signed-batch-${randomUUID()}-éè/中文&name=1/끝🙂.png`, + `authenticated/signed-batch-${randomUUID()}-폴더?x=1/세그먼트#tag/leaf+plus;semi,.png`, + `authenticated/signed-batch-${randomUUID()}-éè/中文&name=1/끝🙂.png`, ] for (const objectName of objectNames) { @@ -2359,6 +2368,17 @@ describe('testing generating signed URLs', () => { expect(getResponse.statusCode).toBe(200) expect(getResponse.headers['etag']).toBe('abc') } + + for (const objectName of objectNames) { + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(deleteResponse.statusCode).toBe(200) + } }) }) @@ -3542,6 +3562,15 @@ describe('Object key names with Unicode characters', () => { }) .first() expect(objectResponse?.name).toBe(objectName) + + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(deleteResponse.statusCode).toBe(200) }) test('should not generate signed upload URL for invalid key', async () => { From a66ef7c515d0bbac00417e56ef0001c2a1e3c13b Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 23:09:29 +0100 Subject: [PATCH 16/40] fix: add tests for webhooks Signed-off-by: ferhat elmas --- .../events/lifecycle/webhook-filter.ts | 10 + src/storage/events/lifecycle/webhook.ts | 8 +- src/test/webhook-filter.test.ts | 35 ++++ src/test/webhooks.test.ts | 187 +++++++++++++++++- 4 files changed, 231 insertions(+), 9 deletions(-) create mode 100644 src/storage/events/lifecycle/webhook-filter.ts create mode 100644 src/test/webhook-filter.test.ts diff --git a/src/storage/events/lifecycle/webhook-filter.ts b/src/storage/events/lifecycle/webhook-filter.ts new file mode 100644 index 000000000..312995d53 --- /dev/null +++ b/src/storage/events/lifecycle/webhook-filter.ts @@ -0,0 +1,10 @@ +export function shouldDisableWebhookEvent( + disabledEvents: string[], + eventType: string, + payload: { bucketId: string; name: string } +) { + return ( + disabledEvents.includes(`Webhook:${eventType}`) || + disabledEvents.includes(`Webhook:${eventType}:${payload.bucketId}/${payload.name}`) + ) +} diff --git a/src/storage/events/lifecycle/webhook.ts b/src/storage/events/lifecycle/webhook.ts index 782e50453..24a269d42 100644 --- a/src/storage/events/lifecycle/webhook.ts +++ b/src/storage/events/lifecycle/webhook.ts @@ -5,6 +5,7 @@ import axios from 'axios' import { Job, SendOptions, WorkOptions } from 'pg-boss' import { getConfig } from '../../../config' import { BaseEvent } from '../base-event' +import { shouldDisableWebhookEvent } from './webhook-filter' const { isMultitenant, @@ -73,12 +74,7 @@ export class Webhook extends BaseEvent { // Do not send an event if disabled for this specific tenant const tenant = await getTenantConfig(payload.tenant.ref) const disabledEvents = tenant.disableEvents || [] - if ( - disabledEvents.includes(`Webhook:${payload.event.type}`) || - disabledEvents.includes( - `Webhook:${payload.event.type}:${payload.event.payload.bucketId}/${payload.event.payload.name}` - ) - ) { + if (shouldDisableWebhookEvent(disabledEvents, payload.event.type, payload.event.payload)) { return false } } diff --git a/src/test/webhook-filter.test.ts b/src/test/webhook-filter.test.ts new file mode 100644 index 000000000..576a09811 --- /dev/null +++ b/src/test/webhook-filter.test.ts @@ -0,0 +1,35 @@ +import { shouldDisableWebhookEvent } from '@storage/events/lifecycle/webhook-filter' + +describe('webhook filter', () => { + test('matches object-level disableEvents entries with Unicode and URL-reserved object names', () => { + const objectName = '폴더/子目录/파일-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' + const eventType = 'ObjectCreated:Post' + + const disabled = shouldDisableWebhookEvent(disabledEvents(eventType, objectName), eventType, { + bucketId: 'bucket6', + name: objectName, + }) + + expect(disabled).toBe(true) + }) + + test('does not match URL-encoded object-level disableEvents entries', () => { + const objectName = '폴더/子目录/파일-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' + const eventType = 'ObjectCreated:Post' + + const disabled = shouldDisableWebhookEvent( + [`Webhook:${eventType}:bucket6/${encodeURIComponent(objectName)}`], + eventType, + { + bucketId: 'bucket6', + name: objectName, + } + ) + + expect(disabled).toBe(false) + }) +}) + +function disabledEvents(eventType: string, objectName: string) { + return [`Webhook:${eventType}:bucket6/${objectName}`] +} diff --git a/src/test/webhooks.test.ts b/src/test/webhooks.test.ts index 9fcdace15..f08165c2e 100644 --- a/src/test/webhooks.test.ts +++ b/src/test/webhooks.test.ts @@ -8,7 +8,10 @@ mergeConfig({ }) import { getPostgresConnection, getServiceKeyUser } from '@internal/database' +import { StorageKnexDB } from '@storage/database' +import { TenantLocation } from '@storage/locator' import { Obj } from '@storage/schemas' +import { Storage } from '@storage/storage' import { randomUUID } from 'crypto' import { FastifyInstance } from 'fastify' import FormData from 'form-data' @@ -385,17 +388,195 @@ describe('Webhooks', () => { }) ) }) + + it('will emit Unicode and URL-reserved object names in creation webhook payloads', async () => { + const form = new FormData() + const authorization = `Bearer ${await serviceKeyAsync}` + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const headers = Object.assign({}, form.getHeaders(), { + authorization, + }) + + const objectName = `public/${randomUUID()}-폴더/子目录/파일-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + + const response = await appInstance.inject({ + method: 'POST', + url: `/object/bucket6/${encodeURIComponent(objectName)}`, + headers, + payload: form, + }) + + expect(response.statusCode).toBe(200) + expect(sendSpy).toBeCalledTimes(1) + expect(sendSpy).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + event: expect.objectContaining({ + type: 'ObjectCreated:Post', + payload: expect.objectContaining({ + bucketId: 'bucket6', + name: objectName, + }), + }), + }), + }) + ) + }) + + it('will emit a webhook with ObjectCreated:Put when uploading with upsert to an existing key', async () => { + const objectName = `upsert-${randomUUID()}-existing-key.png` + await createObject(pg, 'bucket6', objectName) + + const authorization = `Bearer ${await serviceKeyAsync}` + const response = await appInstance.inject({ + method: 'PUT', + url: `/object/bucket6/${encodeURIComponent(objectName)}`, + headers: { + authorization, + 'Content-Type': 'image/png', + }, + payload: fs.createReadStream(`./src/test/assets/sadcat.jpg`), + }) + + expect(response.statusCode).toBe(200) + + const webhookCalls = sendSpy.mock.calls + .map(([payload]) => payload) + .filter((payload) => payload?.name === 'webhooks') + + expect(webhookCalls).toHaveLength(1) + expect(webhookCalls[0]).toEqual( + expect.objectContaining({ + data: expect.objectContaining({ + event: expect.objectContaining({ + type: 'ObjectCreated:Put', + payload: expect.objectContaining({ + bucketId: 'bucket6', + name: objectName, + uploadType: 'standard', + }), + }), + }), + }) + ) + }) + + it('will emit a webhook with ObjectUpdated:Metadata when object metadata is updated', async () => { + const objectName = `metadata-${randomUUID()}-update-target.png` + const obj = await createObject(pg, 'bucket6', objectName) + + const db = new StorageKnexDB(pg, { + tenantId, + host: 'localhost', + }) + const storage = new Storage({} as any, db, new TenantLocation('bucket')) + + const metadata = { + cacheControl: 'public, max-age=120', + contentLength: 3746, + eTag: 'etag-metadata-update', + lastModified: new Date('2026-03-05T12:00:00.000Z'), + httpStatusCode: 200, + mimetype: 'image/png', + size: 3746, + xRobotsTag: 'noindex', + } + + await storage.from('bucket6').updateObjectMetadata(objectName, metadata) + + expect(sendSpy).toBeCalledTimes(1) + expect(sendSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'webhooks', + data: expect.objectContaining({ + event: expect.objectContaining({ + type: 'ObjectUpdated:Metadata', + payload: expect.objectContaining({ + bucketId: 'bucket6', + name: objectName, + metadata: expect.objectContaining({ + cacheControl: metadata.cacheControl, + mimetype: metadata.mimetype, + size: metadata.size, + xRobotsTag: metadata.xRobotsTag, + }), + }), + }), + }), + }) + ) + }) + + it('will preserve Unicode and URL-reserved object names in move webhook payloads', async () => { + const sourceKey = `source-${randomUUID()}-일이삼/子目录/파일-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png` + const destinationKey = `dest-${randomUUID()}-폴더/子目录/파일-🙂-q?x=1&y=%25+plus;semi:colon,#frag.png` + const obj = await createObject(pg, 'bucket6', sourceKey) + + const authorization = `Bearer ${await serviceKeyAsync}` + const response = await appInstance.inject({ + method: 'POST', + url: `/object/move`, + headers: { + authorization, + }, + payload: { + bucketId: 'bucket6', + sourceKey: obj.name, + destinationKey, + }, + }) + + expect(response.statusCode).toBe(200) + expect(sendSpy).toBeCalledTimes(3) + + expect(sendSpy).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + data: expect.objectContaining({ + event: expect.objectContaining({ + type: 'ObjectRemoved:Move', + payload: expect.objectContaining({ + bucketId: 'bucket6', + name: sourceKey, + }), + }), + }), + }) + ) + + expect(sendSpy).toHaveBeenNthCalledWith( + 3, + expect.objectContaining({ + data: expect.objectContaining({ + event: expect.objectContaining({ + type: 'ObjectCreated:Move', + payload: expect.objectContaining({ + bucketId: 'bucket6', + name: destinationKey, + oldObject: expect.objectContaining({ + bucketId: 'bucket6', + name: sourceKey, + }), + }), + }), + }), + }) + ) + }) }) -async function createObject(pg: TenantConnection, bucketId: string) { - const objectName = Date.now() +async function createObject( + pg: TenantConnection, + bucketId: string, + objectName = Date.now().toString() +) { const tnx = await pg.transaction() const [data] = await tnx .from('objects') .insert([ { - name: objectName.toString(), + name: objectName, bucket_id: bucketId, version: randomUUID(), metadata: { From 7a0862b0e30352993956147d0a380a5b3b8e6503 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 23:26:12 +0100 Subject: [PATCH 17/40] fix: drop dead param for sign Signed-off-by: ferhat elmas --- src/http/routes/object/getSignedUploadURL.ts | 4 +--- src/storage/object.ts | 1 - src/test/tus.test.ts | 14 ++++---------- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/http/routes/object/getSignedUploadURL.ts b/src/http/routes/object/getSignedUploadURL.ts index b84c113bf..dfb1f74dd 100644 --- a/src/http/routes/object/getSignedUploadURL.ts +++ b/src/http/routes/object/getSignedUploadURL.ts @@ -67,11 +67,9 @@ export default async function routes(fastify: FastifyInstance) { const objectName = request.params['*'] const owner = request.owner - const urlPath = `${bucketName}/${objectName}` - const signedUpload = await request.storage .from(bucketName) - .signUploadObjectUrl(objectName, urlPath as string, uploadSignedUrlExpirationTime, owner, { + .signUploadObjectUrl(objectName, uploadSignedUrlExpirationTime, owner, { upsert: request.headers['x-upsert'] === 'true', }) diff --git a/src/storage/object.ts b/src/storage/object.ts index b3e680ad4..81e36c419 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -801,7 +801,6 @@ export class ObjectStorage { */ async signUploadObjectUrl( objectName: string, - _url: string, expiresIn: number, owner?: string, options?: { upsert?: boolean } diff --git a/src/test/tus.test.ts b/src/test/tus.test.ts index f79013db9..014f62155 100644 --- a/src/test/tus.test.ts +++ b/src/test/tus.test.ts @@ -256,9 +256,7 @@ describe('Tus multipart', () => { const objectName = randomUUID() + '-cat.jpeg' - const signedUpload = await storage - .from(bucketName) - .signUploadObjectUrl(objectName, `${bucketName}/${objectName}`, 3600) + const signedUpload = await storage.from(bucketName).signUploadObjectUrl(objectName, 3600) const result = await new Promise((resolve, reject) => { const upload = new tus.Upload(oneChunkFile, { @@ -328,9 +326,7 @@ describe('Tus multipart', () => { }) const objectName = `${randomUUID()}-${getUnicodeObjectName()}` - const signedUpload = await storage - .from(bucketName) - .signUploadObjectUrl(objectName, `${bucketName}/${objectName}`, 3600) + const signedUpload = await storage.from(bucketName).signUploadObjectUrl(objectName, 3600) const result = await new Promise((resolve, reject) => { const upload = new tus.Upload(oneChunkFile, { @@ -375,7 +371,7 @@ describe('Tus multipart', () => { const signedUpload = await storage .from(bucketName) - .signUploadObjectUrl(objectName, `${bucketName}/${objectName}`, 3600, 'some-owner-id') + .signUploadObjectUrl(objectName, 3600, 'some-owner-id') const result = await new Promise((resolve, reject) => { const upload = new tus.Upload(oneChunkFile, { @@ -439,9 +435,7 @@ describe('Tus multipart', () => { const objectName = randomUUID() + '-cat.jpeg' - const signedUpload = await storage - .from(bucketName) - .signUploadObjectUrl(objectName, `${bucketName}/${objectName}`, 1) + const signedUpload = await storage.from(bucketName).signUploadObjectUrl(objectName, 1) await wait(2000) From df1b6616aef8b747cf3135c9f57a0cdf920a073b Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Thu, 5 Mar 2026 23:33:19 +0100 Subject: [PATCH 18/40] fix: s3 copy source Signed-off-by: ferhat elmas --- src/storage/backend/s3/adapter.ts | 14 +++++++-- src/test/s3-adapter.test.ts | 52 ++++++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/storage/backend/s3/adapter.ts b/src/storage/backend/s3/adapter.ts index b5607e911..9b3e50a16 100644 --- a/src/storage/backend/s3/adapter.ts +++ b/src/storage/backend/s3/adapter.ts @@ -52,6 +52,13 @@ export interface S3ClientOptions { requestTimeout?: number } +function encodeCopySource(bucket: string, key: string): string { + return `${encodeURIComponent(bucket)}/${key + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` +} + /** * S3Backend * Interacts with a s3-compatible file system with this S3Adapter @@ -255,7 +262,7 @@ export class S3Backend implements StorageBackendAdapter { try { const command = new CopyObjectCommand({ Bucket: bucket, - CopySource: encodeURIComponent(`${bucket}/${withOptionalVersion(source, version)}`), + CopySource: encodeCopySource(bucket, withOptionalVersion(source, version)), Key: withOptionalVersion(destination, destinationVersion), CopySourceIfMatch: conditions?.ifMatch, CopySourceIfNoneMatch: conditions?.ifNoneMatch, @@ -568,8 +575,9 @@ export class S3Backend implements StorageBackendAdapter { Key: withOptionalVersion(key, version), UploadId, PartNumber, - CopySource: encodeURIComponent( - `${storageS3Bucket}/${withOptionalVersion(sourceKey, sourceKeyVersion)}` + CopySource: encodeCopySource( + storageS3Bucket, + withOptionalVersion(sourceKey, sourceKeyVersion) ), CopySourceRange: bytesRange ? `bytes=${bytesRange.fromByte}-${bytesRange.toByte}` : undefined, }) diff --git a/src/test/s3-adapter.test.ts b/src/test/s3-adapter.test.ts index fd6807775..ab7900dd3 100644 --- a/src/test/s3-adapter.test.ts +++ b/src/test/s3-adapter.test.ts @@ -1,6 +1,6 @@ 'use strict' -import { S3Client, UploadPartCopyCommand } from '@aws-sdk/client-s3' +import { CopyObjectCommand, S3Client, UploadPartCopyCommand } from '@aws-sdk/client-s3' import { Readable } from 'stream' import { S3Backend } from '../storage/backend/s3/adapter' @@ -14,6 +14,12 @@ jest.mock('@aws-sdk/client-s3', () => { } }) +const encodeCopySourceByPathToken = (bucket: string, key: string) => + `${encodeURIComponent(bucket)}/${key + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` + describe('S3Backend', () => { let mockSend: jest.Mock @@ -75,8 +81,42 @@ describe('S3Backend', () => { }) }) + describe('copyObject', () => { + test('should preserve "/" and encode path tokens in CopySource', async () => { + const lastModified = new Date('2024-01-01T00:00:00.000Z') + mockSend.mockResolvedValue({ + CopyObjectResult: { + ETag: '"copy-etag"', + LastModified: lastModified, + }, + $metadata: { + httpStatusCode: 200, + }, + }) + + const backend = new S3Backend({ + region: 'us-east-1', + endpoint: 'http://localhost:9000', + }) + + const sourceKey = 'source path/일이삼-🙂?#%.jpg' + const destinationKey = 'dest/path/copied-🙂.jpg' + + await backend.copyObject('test-bucket', sourceKey, undefined, destinationKey, undefined) + + expect(mockSend).toHaveBeenCalledTimes(1) + const command = mockSend.mock.calls[0][0] as CopyObjectCommand + expect(command).toBeInstanceOf(CopyObjectCommand) + expect(command.input.CopySource).toBe( + encodeCopySourceByPathToken('test-bucket', sourceKey) + ) + expect(command.input.CopySource).toContain('test-bucket/source%20path/') + expect(command.input.CopySource).not.toContain('test-bucket%2F') + }) + }) + describe('uploadPartCopy', () => { - test('should URL-encode CopySource for unicode keys', async () => { + test('should preserve "/" and encode path tokens in CopySource', async () => { const lastModified = new Date('2024-01-01T00:00:00.000Z') mockSend.mockResolvedValue({ CopyPartResult: { @@ -90,7 +130,7 @@ describe('S3Backend', () => { endpoint: 'http://localhost:9000', }) - const sourceKey = 'source/path/일이삼-🙂.jpg' + const sourceKey = 'source path/folder/일이삼-🙂?#%.jpg' const destinationKey = 'dest/path/copied-🙂.jpg' const result = await backend.uploadPartCopy( @@ -107,7 +147,11 @@ describe('S3Backend', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as UploadPartCopyCommand expect(command).toBeInstanceOf(UploadPartCopyCommand) - expect(command.input.CopySource).toBe(encodeURIComponent(`test-bucket/${sourceKey}`)) + expect(command.input.CopySource).toBe( + encodeCopySourceByPathToken('test-bucket', sourceKey) + ) + expect(command.input.CopySource).toContain('test-bucket/source%20path/folder/') + expect(command.input.CopySource).not.toContain('test-bucket%2F') expect(command.input.CopySourceRange).toBe('bytes=0-1024') expect(result).toEqual({ eTag: '"copy-etag"', From 835d8abca0e75e20bafd454d017f4a2b2a22498d Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 00:39:35 +0100 Subject: [PATCH 19/40] fix: backward compat for list continuation Signed-off-by: ferhat elmas --- src/storage/object.ts | 43 ++++++++++++++++++++++---- src/test/object-list-v2.test.ts | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/src/storage/object.ts b/src/storage/object.ts index 81e36c419..acefd0720 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -871,9 +871,11 @@ const CONTINUATION_TOKEN_PART_MAP: Record = { c: 'sortColumn', a: 'sortColumnAfter', } +const CONTINUATION_TOKEN_VERSION = '1' +const CONTINUATION_TOKEN_VERSION_KEY = 'v' function encodeContinuationToken(tokenInfo: ContinuationToken) { - const result: string[] = [] + const result: string[] = [`${CONTINUATION_TOKEN_VERSION_KEY}:${CONTINUATION_TOKEN_VERSION}`] for (const [k, v] of Object.entries(CONTINUATION_TOKEN_PART_MAP)) { const value = tokenInfo[v] if (value) { @@ -889,18 +891,47 @@ function decodeContinuationToken(token: string): ContinuationToken { startAfter: '', sortOrder: 'asc', } + const parsedParts: { key: string; value: string }[] = [] + let version: string | undefined + for (const part of decodedParts) { const partMatch = part.match(/^(\S):(.*)/) - if (!partMatch || partMatch.length !== 3 || !(partMatch[1] in CONTINUATION_TOKEN_PART_MAP)) { + if (!partMatch || partMatch.length !== 3) { + throw new Error('Invalid continuation token') + } + const key = partMatch[1] + const value = partMatch[2] + + if (key === CONTINUATION_TOKEN_VERSION_KEY) { + if (version !== undefined) { + throw new Error('Invalid continuation token') + } + version = value + continue + } + + if (!(key in CONTINUATION_TOKEN_PART_MAP)) { throw new Error('Invalid continuation token') } - let value = partMatch[2] + parsedParts.push({ key, value }) + } + + if (version && version !== CONTINUATION_TOKEN_VERSION) { + throw new Error('Invalid continuation token') + } + + for (const part of parsedParts) { + if (!version) { + // Backward compatibility: legacy cursor values were stored unescaped. + result[CONTINUATION_TOKEN_PART_MAP[part.key]] = part.value + continue + } + try { - value = decodeURIComponent(value) + result[CONTINUATION_TOKEN_PART_MAP[part.key]] = decodeURIComponent(part.value) } catch { - // Backward compatibility: previously cursor values were stored unescaped. + throw new Error('Invalid continuation token') } - result[CONTINUATION_TOKEN_PART_MAP[partMatch[1]]] = value } return result } diff --git a/src/test/object-list-v2.test.ts b/src/test/object-list-v2.test.ts index b7d461256..9169a130e 100644 --- a/src/test/object-list-v2.test.ts +++ b/src/test/object-list-v2.test.ts @@ -795,4 +795,59 @@ describe('objects - list v2 cursor encoding', () => { const listed = [page1.objects[0]?.name, page2.objects[0]?.name].filter(Boolean).sort() expect(listed).toEqual([...keys].sort()) }) + + test('supports legacy unescaped cursor values with literal % hex sequences', async () => { + const runId = randomUUID() + const prefix = `cursor-legacy-${runId}-` + const firstKey = `${prefix}file%20name.txt` + const secondKey = `${prefix}file~name.txt` + + for (const key of [firstKey, secondKey]) { + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/${LIST_V2_CURSOR_BUCKET}/${encodeURIComponent(key)}`, + payload: createUpload('utf8.txt', 'cursor legacy test'), + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(uploadResponse.statusCode).toBe(200) + } + + const page1Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page1Response.statusCode).toBe(200) + const page1 = page1Response.json() + expect(page1.objects).toHaveLength(1) + expect(page1.objects[0]?.name).toBe(firstKey) + + const legacyCursor = Buffer.from(`l:${firstKey}\no:asc`).toString('base64') + const page2Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + cursor: legacyCursor, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page2Response.statusCode).toBe(200) + const page2 = page2Response.json() + expect(page2.objects).toHaveLength(1) + expect(page2.objects[0]?.name).toBe(secondKey) + }) }) From e72d008b28cf1ba421cf0f5d8e4ca1a560edf9fa Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 00:44:31 +0100 Subject: [PATCH 20/40] fix: backward compat for s3 continuation Signed-off-by: ferhat elmas --- src/storage/protocols/s3/s3-handler.ts | 52 +++++++++++++++++++++++--- src/test/s3-protocol.test.ts | 45 ++++++++++++++++++++++ 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index e5f2f0ed5..fec20f1a9 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -1442,19 +1442,61 @@ function parseCopySource(copySource: string): { } function encodeContinuationToken(name: string) { - return Buffer.from(`l:${name}`).toString('base64') + return Buffer.from(`v:1\nl:${encodeURIComponent(name)}`).toString('base64') +} + +function decodeLegacyContinuationToken(decoded: string) { + // Backward compatibility: preserve pre-version behavior for old in-flight tokens. + const continuationToken = decoded.split(':')[1] + if (!continuationToken) { + throw new Error('Invalid continuation token') + } + return continuationToken } function decodeContinuationToken(token: string) { const decoded = Buffer.from(token, 'base64').toString() - if (!decoded.startsWith('l:')) { + + if (decoded.startsWith('l:')) { + return decodeLegacyContinuationToken(decoded) + } + + const parts = decoded.split('\n') + let version: string | undefined + let encodedValue: string | undefined + + for (const part of parts) { + const match = part.match(/^(\S):(.*)/) + if (!match || match.length !== 3) { + throw new Error('Invalid continuation token') + } + + if (match[1] === 'v') { + if (version !== undefined) { + throw new Error('Invalid continuation token') + } + version = match[2] + continue + } + + if (match[1] === 'l') { + if (encodedValue !== undefined) { + throw new Error('Invalid continuation token') + } + encodedValue = match[2] + continue + } + throw new Error('Invalid continuation token') } - const value = decoded.slice(2) - if (!value) { + if (version !== '1' || !encodedValue) { throw new Error('Invalid continuation token') } - return value + try { + return decodeURIComponent(encodedValue) + } catch { + throw new Error('Invalid continuation token') + } } diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index 5f00e537f..d8e8a6181 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -2418,6 +2418,7 @@ describe('S3 Protocol', () => { ) expect(page1.Uploads?.length).toBe(1) expect(page1.NextKeyMarker).toBeTruthy() + expect(Buffer.from(page1.NextKeyMarker!, 'base64').toString()).toMatch(/^v:1\nl:/) const page2 = await client.send( new ListMultipartUploadsCommand({ @@ -2434,6 +2435,50 @@ describe('S3 Protocol', () => { expect([...pagedKeys].sort()).toEqual([...keys].sort()) }) + it('accepts legacy unversioned KeyMarker tokens for in-flight pagination', async () => { + const bucketName = await createBucket(client) + const runId = randomUUID() + const firstKey = `mp-legacy-${runId}:001.jpg` + const secondKey = `mp-legacy-${runId}:002.jpg` + + await Promise.all( + [firstKey, secondKey].map((key) => + client.send( + new CreateMultipartUploadCommand({ + Bucket: bucketName, + Key: key, + ContentType: 'image/jpg', + }) + ) + ) + ) + + const page1 = await client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + MaxUploads: 1, + }) + ) + + expect(page1.Uploads?.[0]?.Key).toBe(firstKey) + + const legacyToken = Buffer.from(`l:${firstKey}`).toString('base64') + const pageUsingLegacyToken = await client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + MaxUploads: 1, + KeyMarker: legacyToken, + }) + ) + + // Legacy unversioned markers keep legacy parsing semantics for rollout safety. + expect(pageUsingLegacyToken.Uploads?.[0]?.Key).toBe(firstKey) + expect(pageUsingLegacyToken.NextKeyMarker).toBeTruthy() + expect(Buffer.from(pageUsingLegacyToken.NextKeyMarker!, 'base64').toString()).toMatch( + /^v:1\nl:/ + ) + }) + it('can copy objects using Unicode keys in CopySource', async () => { const bucketName = await createBucket(client) const sourceKey = `copy-src-${randomUUID()}-일이삼-🙂.jpg` From 609e87100b39f03d9610e631368e85c1f33bbd47 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 01:00:26 +0100 Subject: [PATCH 21/40] fix: control chars in migration Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index 6f52e42e4..3193fdb52 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -17,6 +17,7 @@ BEGIN ADD CONSTRAINT objects_name_check CHECK ( name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND name !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' ) $ddl$; ELSE From 57b8e72faf3ae621bd4404eccceeecfde937aaa0 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 01:15:40 +0100 Subject: [PATCH 22/40] fix: explicit decoding Signed-off-by: ferhat elmas --- src/http/routes/object/getSignedObject.ts | 4 +- src/http/routes/render/renderSignedImage.ts | 6 +-- src/http/routes/signed-url.ts | 26 ++++++++++++ src/test/signed-url-route.test.ts | 47 +++++++++++++++++++++ 4 files changed, 77 insertions(+), 6 deletions(-) create mode 100644 src/http/routes/signed-url.ts create mode 100644 src/test/signed-url-route.test.ts diff --git a/src/http/routes/object/getSignedObject.ts b/src/http/routes/object/getSignedObject.ts index d821741ca..9f8bde316 100644 --- a/src/http/routes/object/getSignedObject.ts +++ b/src/http/routes/object/getSignedObject.ts @@ -5,6 +5,7 @@ import { SignedToken, verifyJWT } from '../../../internal/auth' import { getJwtSecret } from '../../../internal/database' import { ERRORS } from '../../../internal/errors' import { ROUTE_OPERATIONS } from '../operations' +import { doesSignedTokenMatchRequestPath } from '../signed-url' const { storageS3Bucket } = getConfig() @@ -71,9 +72,8 @@ export default async function routes(fastify: FastifyInstance) { } const { url, exp } = payload - const path = `${request.params.bucketName}/${request.params['*']}` - if (url !== path) { + if (!doesSignedTokenMatchRequestPath(request.raw.url, '/object/sign', url)) { throw ERRORS.InvalidSignature() } diff --git a/src/http/routes/render/renderSignedImage.ts b/src/http/routes/render/renderSignedImage.ts index 3bf51dc0c..29a8b21ac 100644 --- a/src/http/routes/render/renderSignedImage.ts +++ b/src/http/routes/render/renderSignedImage.ts @@ -6,6 +6,7 @@ import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' import { getConfig } from '../../../config' import { ROUTE_OPERATIONS } from '../operations' +import { doesSignedTokenMatchRequestPath } from '../signed-url' const { storageS3Bucket, isMultitenant } = getConfig() @@ -68,10 +69,7 @@ export default async function routes(fastify: FastifyInstance) { } const { url, transformations, exp } = payload - - const path = `${request.params.bucketName}/${request.params['*']}` - - if (url !== path) { + if (!doesSignedTokenMatchRequestPath(request.raw.url, '/render/image/sign', url)) { throw ERRORS.InvalidSignature() } diff --git a/src/http/routes/signed-url.ts b/src/http/routes/signed-url.ts new file mode 100644 index 000000000..2c213ceec --- /dev/null +++ b/src/http/routes/signed-url.ts @@ -0,0 +1,26 @@ +function stripQueryString(rawUrl: string): string { + const queryIdx = rawUrl.indexOf('?') + return queryIdx === -1 ? rawUrl : rawUrl.slice(0, queryIdx) +} + +function encodeObjectPathForURL(objectPath: string): string { + return objectPath + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') +} + +export function doesSignedTokenMatchRequestPath( + rawUrl: string | undefined, + routePrefix: string, + signedObjectPath: string +): boolean { + if (!rawUrl) { + return false + } + + // Verify against the raw URL path to avoid implicit dependencies on framework param decoding. + const pathname = stripQueryString(rawUrl) + const expectedPath = `${routePrefix}/${encodeObjectPathForURL(signedObjectPath)}` + return pathname === expectedPath +} diff --git a/src/test/signed-url-route.test.ts b/src/test/signed-url-route.test.ts new file mode 100644 index 000000000..6c57872c2 --- /dev/null +++ b/src/test/signed-url-route.test.ts @@ -0,0 +1,47 @@ +import { doesSignedTokenMatchRequestPath } from '../http/routes/signed-url' + +const encodeObjectPathForURL = (objectPath: string) => + objectPath + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') + +describe('signed URL route path verification', () => { + test('matches canonical encoded object path for object signed route', () => { + const signedObjectPath = 'bucket2/public/일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' + const rawPath = `/object/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + + expect(doesSignedTokenMatchRequestPath(rawPath, '/object/sign', signedObjectPath)).toBe(true) + }) + + test('matches canonical encoded object path for render signed route', () => { + const signedObjectPath = 'bucket2/authenticated/casestudy.png' + const rawPath = `/render/image/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + + expect( + doesSignedTokenMatchRequestPath(rawPath, '/render/image/sign', signedObjectPath) + ).toBe(true) + }) + + test('rejects double-encoded request paths', () => { + const signedObjectPath = 'bucket2/public/일이삼.txt' + const encodedPath = encodeObjectPathForURL(signedObjectPath) + const doubleEncodedPath = encodedPath.replaceAll('%', '%25') + const rawPath = `/object/sign/${doubleEncodedPath}?token=jwt` + + expect(doesSignedTokenMatchRequestPath(rawPath, '/object/sign', signedObjectPath)).toBe(false) + }) + + test('rejects decoded raw unicode path', () => { + const signedObjectPath = 'bucket2/public/일이삼.txt' + const rawPath = `/object/sign/${signedObjectPath}?token=jwt` + + expect(doesSignedTokenMatchRequestPath(rawPath, '/object/sign', signedObjectPath)).toBe(false) + }) + + test('returns false for missing raw url', () => { + expect( + doesSignedTokenMatchRequestPath(undefined, '/object/sign', 'bucket2/public/sadcat-upload.png') + ).toBe(false) + }) +}) From 0209a8fa79866eb5163f6a59711c59b3912f2a4c Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 10:17:40 +0100 Subject: [PATCH 23/40] fix: more test coverage Signed-off-by: ferhat elmas --- src/test/limits.test.ts | 4 +++ src/test/object.test.ts | 28 +++++++++++++++++ src/test/render-routes.test.ts | 50 +++++++++++++++++++++++++++++++ src/test/s3-adapter.test.ts | 10 ++----- src/test/signed-url-route.test.ts | 6 ++-- src/test/webhook-filter.test.ts | 20 +++++++++++++ 6 files changed, 108 insertions(+), 10 deletions(-) diff --git a/src/test/limits.test.ts b/src/test/limits.test.ts index 6204a6897..f7704118c 100644 --- a/src/test/limits.test.ts +++ b/src/test/limits.test.ts @@ -17,6 +17,10 @@ describe('isValidKey', () => { expect(isValidKey('invalid\x01name')).toBe(false) }) + test('accepts DEL (0x7F) as a valid key character', () => { + expect(isValidKey('valid\x7Fname')).toBe(true) + }) + test('rejects non-characters U+FFFE and U+FFFF', () => { expect(isValidKey(`invalid${'\uFFFE'}`)).toBe(false) expect(isValidKey(`invalid${'\uFFFF'}`)).toBe(false) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 4fb840634..59b388fcd 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -2455,6 +2455,34 @@ describe('testing retrieving signed URL', () => { expect(body.error).toBe('InvalidSignature') }) + test('rejects double-encoded signed object paths', async () => { + const objectName = `public/signed-double-${randomUUID()}-일이삼.txt` + await seedObjectForRouteTest(objectName) + + const urlToSign = `bucket2/${objectName}` + const jwtToken = await signJWT({ url: urlToSign }, jwtSecret, 100) + const encodedPath = urlToSign + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') + + const validResponse = await appInstance.inject({ + method: 'GET', + url: `/object/sign/${encodedPath}?token=${jwtToken}`, + }) + expect(validResponse.statusCode).toBe(200) + + const doubleEncodedPath = encodedPath.replaceAll('%', '%25') + const doubleEncodedResponse = await appInstance.inject({ + method: 'GET', + url: `/object/sign/${doubleEncodedPath}?token=${jwtToken}`, + }) + + expect(doubleEncodedResponse.statusCode).toBe(400) + const body = doubleEncodedResponse.json<{ error: string }>() + expect(body.error).toBe('InvalidSignature') + }) + test('get object without a token', async () => { const response = await appInstance.inject({ method: 'GET', diff --git a/src/test/render-routes.test.ts b/src/test/render-routes.test.ts index 408226b95..3a89dbe5e 100644 --- a/src/test/render-routes.test.ts +++ b/src/test/render-routes.test.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto' import { generateHS512JWK, SignedToken, signJWT, verifyJWT } from '@internal/auth' import axios from 'axios' import dotenv from 'dotenv' @@ -194,4 +195,53 @@ describe('image rendering routes', () => { const body = response.json<{ error: string }>() expect(body.error).toBe('InvalidSignature') }) + + it('will reject double-encoded signed render paths', async () => { + const objectName = `authenticated/render-double-${randomUUID()}-일이삼.png` + const encodedObjectName = objectName + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') + + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${encodedObjectName}`, + payload: Buffer.from('render double-encoded test'), + headers: { + authorization: `Bearer ${process.env.SERVICE_KEY}`, + 'content-type': 'image/png', + 'x-upsert': 'true', + }, + }) + expect(uploadResponse.statusCode).toBe(200) + + const signURLResponse = await appInstance.inject({ + method: 'POST', + url: `/object/sign/bucket2/${encodedObjectName}`, + payload: { + expiresIn: 60000, + transform: { + width: 100, + height: 100, + resize: 'contain', + }, + }, + headers: { + authorization: `Bearer ${process.env.SERVICE_KEY}`, + }, + }) + expect(signURLResponse.statusCode).toBe(200) + + const signedURL = signURLResponse.json<{ signedURL: string }>().signedURL + const signedURLParsed = new URL(signedURL, 'http://localhost') + const doubleEncodedPath = signedURLParsed.pathname.replaceAll('%', '%25') + const doubleEncodedResponse = await appInstance.inject({ + method: 'GET', + url: `${doubleEncodedPath}${signedURLParsed.search}`, + }) + + expect(doubleEncodedResponse.statusCode).toBe(400) + const body = doubleEncodedResponse.json<{ error: string }>() + expect(body.error).toBe('InvalidSignature') + }) }) diff --git a/src/test/s3-adapter.test.ts b/src/test/s3-adapter.test.ts index ab7900dd3..fc642f29d 100644 --- a/src/test/s3-adapter.test.ts +++ b/src/test/s3-adapter.test.ts @@ -107,16 +107,14 @@ describe('S3Backend', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as CopyObjectCommand expect(command).toBeInstanceOf(CopyObjectCommand) - expect(command.input.CopySource).toBe( - encodeCopySourceByPathToken('test-bucket', sourceKey) - ) + expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('test-bucket', sourceKey)) expect(command.input.CopySource).toContain('test-bucket/source%20path/') expect(command.input.CopySource).not.toContain('test-bucket%2F') }) }) describe('uploadPartCopy', () => { - test('should preserve "/" and encode path tokens in CopySource', async () => { + test('should preserve "/" and encode path tokens in CopySource for unicode source keys', async () => { const lastModified = new Date('2024-01-01T00:00:00.000Z') mockSend.mockResolvedValue({ CopyPartResult: { @@ -147,9 +145,7 @@ describe('S3Backend', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as UploadPartCopyCommand expect(command).toBeInstanceOf(UploadPartCopyCommand) - expect(command.input.CopySource).toBe( - encodeCopySourceByPathToken('test-bucket', sourceKey) - ) + expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('test-bucket', sourceKey)) expect(command.input.CopySource).toContain('test-bucket/source%20path/folder/') expect(command.input.CopySource).not.toContain('test-bucket%2F') expect(command.input.CopySourceRange).toBe('bytes=0-1024') diff --git a/src/test/signed-url-route.test.ts b/src/test/signed-url-route.test.ts index 6c57872c2..49ae3be3a 100644 --- a/src/test/signed-url-route.test.ts +++ b/src/test/signed-url-route.test.ts @@ -18,9 +18,9 @@ describe('signed URL route path verification', () => { const signedObjectPath = 'bucket2/authenticated/casestudy.png' const rawPath = `/render/image/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` - expect( - doesSignedTokenMatchRequestPath(rawPath, '/render/image/sign', signedObjectPath) - ).toBe(true) + expect(doesSignedTokenMatchRequestPath(rawPath, '/render/image/sign', signedObjectPath)).toBe( + true + ) }) test('rejects double-encoded request paths', () => { diff --git a/src/test/webhook-filter.test.ts b/src/test/webhook-filter.test.ts index 576a09811..3e84d7494 100644 --- a/src/test/webhook-filter.test.ts +++ b/src/test/webhook-filter.test.ts @@ -28,6 +28,26 @@ describe('webhook filter', () => { expect(disabled).toBe(false) }) + + test('does not match path-segment URL-encoded disableEvents entries from external config', () => { + const objectName = '폴더/子目录/파일-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' + const eventType = 'ObjectCreated:Post' + const encodedByPathSegment = objectName + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') + + const disabled = shouldDisableWebhookEvent( + [`Webhook:${eventType}:bucket6/${encodedByPathSegment}`], + eventType, + { + bucketId: 'bucket6', + name: objectName, + } + ) + + expect(disabled).toBe(false) + }) }) function disabledEvents(eventType: string, objectName: string) { From 4a563c7f8703acfe46640200aaa9564cb0091e56 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 14:52:56 +0100 Subject: [PATCH 24/40] fix: post rebase Signed-off-by: ferhat elmas --- src/http/plugins/xml.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/http/plugins/xml.ts b/src/http/plugins/xml.ts index 693fa0c26..480b037b5 100644 --- a/src/http/plugins/xml.ts +++ b/src/http/plugins/xml.ts @@ -22,15 +22,18 @@ export function decodeXmlNumericEntities(value: string): string { ) } - return value.replace(/&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, (match: string, rawValue: string) => { - const isHex = rawValue[0].toLowerCase() === 'x' - const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) - if (!isValidXmlCodePoint(codePoint)) { - return match - } + return value.replace( + /&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, + (match: string, rawValue: string) => { + const isHex = rawValue[0].toLowerCase() === 'x' + const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) + if (!isValidXmlCodePoint(codePoint)) { + return match + } - return String.fromCodePoint(codePoint) - }) + return String.fromCodePoint(codePoint) + } + ) } function forcePathAsArray(node: unknown, pathSegments: string[]): void { From 9628e546bce3ec09443e300a57fa60f331d51526 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 15:09:27 +0100 Subject: [PATCH 25/40] fix: test data dependence Signed-off-by: ferhat elmas --- src/test/object.test.ts | 54 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 59b388fcd..754be7b22 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -19,6 +19,7 @@ const { jwtSecret, serviceKeyAsync, tenantId } = getConfig() const anonKey = process.env.ANON_KEY || '' const S3Backend = backends.S3Backend const TEST_OWNER_ID = '317eadce-631a-4429-a0bb-f19a7a517b4a' +const routeTestObjectsToCleanup: Array<{ bucketId: string; name: string }> = [] let appInstance: FastifyInstance let tnx: Knex.Transaction | undefined @@ -47,6 +48,47 @@ async function seedObjectForRouteTest(name: string, bucketId = 'bucket2') { }) await seedTx.commit() tnx = undefined + routeTestObjectsToCleanup.push({ bucketId, name }) +} + +async function cleanupRouteTestObjects() { + const authorization = `Bearer ${await serviceKeyAsync}` + + for (const { bucketId, name } of routeTestObjectsToCleanup.splice(0)) { + const response = await appInstance.inject({ + method: 'DELETE', + url: `/object/${bucketId}/${encodeURIComponent(name)}`, + headers: { + authorization, + }, + }) + + if (response.statusCode !== 200 && response.statusCode !== 400) { + throw new Error(`Unexpected cleanup response ${response.statusCode} for ${bucketId}/${name}`) + } + } +} + +async function cleanupObjectsByPrefix(prefix: string, bucketId = 'bucket2') { + const cleanupTx = await getSuperuserPostgrestClient() + const objects = await cleanupTx + .from('objects') + .select('name') + .where({ bucket_id: bucketId }) + .whereLike('name', `${prefix}%`) + await cleanupTx.commit() + tnx = undefined + + const authorization = `Bearer ${await serviceKeyAsync}` + for (const object of objects) { + await appInstance.inject({ + method: 'DELETE', + url: `/object/${bucketId}/${encodeURIComponent(object.name)}`, + headers: { + authorization, + }, + }) + } } useMockObject() @@ -60,7 +102,13 @@ beforeEach(() => { afterEach(async () => { if (tnx) { await tnx.commit() + tnx = undefined + } + + if (routeTestObjectsToCleanup.length > 0) { + await cleanupRouteTestObjects() } + await appInstance.close() }) @@ -2456,7 +2504,7 @@ describe('testing retrieving signed URL', () => { }) test('rejects double-encoded signed object paths', async () => { - const objectName = `public/signed-double-${randomUUID()}-일이삼.txt` + const objectName = `authenticated/signed-double-${randomUUID()}-일이삼.txt` await seedObjectForRouteTest(objectName) const urlToSign = `bucket2/${objectName}` @@ -2768,6 +2816,10 @@ describe('testing move object', () => { }) describe('testing list objects', () => { + beforeEach(async () => { + await cleanupObjectsByPrefix('public/signed-double-') + }) + test('searching the bucket root folder', async () => { const response = await appInstance.inject({ method: 'POST', From c6a0740f1d894ee08f82379b3dc9f0e3afda30d5 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Fri, 6 Mar 2026 15:45:54 +0100 Subject: [PATCH 26/40] fix: error shape Signed-off-by: ferhat elmas --- src/storage/object.ts | 10 ++++- src/storage/protocols/s3/s3-handler.ts | 26 +++++++++--- src/test/s3-protocol.test.ts | 57 ++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/storage/object.ts b/src/storage/object.ts index acefd0720..231ed4790 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -613,7 +613,15 @@ export class ObjectStorage { const prefix = options?.prefix || '' const delimiter = options?.delimiter - const cursor = options?.cursor ? decodeContinuationToken(options.cursor) : undefined + let cursor: ContinuationToken | undefined + if (options?.cursor) { + try { + cursor = decodeContinuationToken(options.cursor) + } catch (error) { + throw ERRORS.InvalidParameter('ContinuationToken', { error: error as Error }) + } + } + let searchResult = await this.db.listObjectsV2(this.bucketId, { prefix: options?.prefix, delimiter: options?.delimiter, diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index fec20f1a9..aa096530c 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -288,17 +288,31 @@ export class S3ProtocolHandler { const bucket = command.Bucket const limit = maxKeys || 200 + let nextUploadKeyToken: string | undefined + let nextUploadToken: string | undefined + + if (keyContinuationToken) { + try { + nextUploadKeyToken = decodeContinuationToken(keyContinuationToken) + } catch (error) { + throw ERRORS.InvalidParameter('KeyMarker', { error: error as Error }) + } + } + + if (uploadContinuationToken) { + try { + nextUploadToken = decodeContinuationToken(uploadContinuationToken) + } catch (error) { + throw ERRORS.InvalidParameter('UploadIdMarker', { error: error as Error }) + } + } const multipartUploads = await this.storage.db.listMultipartUploads(bucket, { prefix, deltimeter: delimiter, maxKeys: limit + 1, - nextUploadKeyToken: keyContinuationToken - ? decodeContinuationToken(keyContinuationToken) - : undefined, - nextUploadToken: uploadContinuationToken - ? decodeContinuationToken(uploadContinuationToken) - : undefined, + nextUploadKeyToken, + nextUploadToken, }) let results: Partial[] = multipartUploads diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index d8e8a6181..ad3112cfe 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -2394,6 +2394,25 @@ describe('S3 Protocol', () => { expect([...pagedKeys].sort()).toEqual([...keys].sort()) }) + it('returns a structured 400 for an invalid ListObjectsV2 continuation token', async () => { + const bucketName = await createBucket(client) + const invalidToken = Buffer.from('v:1\nl:%E0%A4%A').toString('base64') + + await expect( + client.send( + new ListObjectsV2Command({ + Bucket: bucketName, + ContinuationToken: invalidToken, + }) + ) + ).rejects.toMatchObject({ + $metadata: { + httpStatusCode: 400, + }, + message: 'Invalid Parameter ContinuationToken', + }) + }) + it('can paginate ListMultipartUploads with Unicode and reserved characters in keys', async () => { const bucketName = await createBucket(client) const keys = [`mp-${randomUUID()}-🙂:one.jpg`, `mp-${randomUUID()}-일이삼:two.jpg`] @@ -2479,6 +2498,44 @@ describe('S3 Protocol', () => { ) }) + it('returns a structured 400 for an invalid multipart KeyMarker', async () => { + const bucketName = await createBucket(client) + const invalidToken = Buffer.from('v:1\nl:%E0%A4%A').toString('base64') + + await expect( + client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + KeyMarker: invalidToken, + }) + ) + ).rejects.toMatchObject({ + $metadata: { + httpStatusCode: 400, + }, + message: 'Invalid Parameter KeyMarker', + }) + }) + + it('returns a structured 400 for an invalid multipart UploadIdMarker', async () => { + const bucketName = await createBucket(client) + const invalidToken = Buffer.from('v:1\nl:%E0%A4%A').toString('base64') + + await expect( + client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + UploadIdMarker: invalidToken, + }) + ) + ).rejects.toMatchObject({ + $metadata: { + httpStatusCode: 400, + }, + message: 'Invalid Parameter UploadIdMarker', + }) + }) + it('can copy objects using Unicode keys in CopySource', async () => { const bucketName = await createBucket(client) const sourceKey = `copy-src-${randomUUID()}-일이삼-🙂.jpg` From ce03d4a1c863cc4476abae16507274c57811386f Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 12:47:21 +0100 Subject: [PATCH 27/40] fix: inflight pagination Signed-off-by: ferhat elmas --- src/storage/protocols/s3/s3-handler.ts | 2 +- src/test/s3-protocol.test.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index aa096530c..faab803ff 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -1461,7 +1461,7 @@ function encodeContinuationToken(name: string) { function decodeLegacyContinuationToken(decoded: string) { // Backward compatibility: preserve pre-version behavior for old in-flight tokens. - const continuationToken = decoded.split(':')[1] + const continuationToken = decoded.slice(2) if (!continuationToken) { throw new Error('Invalid continuation token') } diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index ad3112cfe..a4250b726 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -2457,8 +2457,8 @@ describe('S3 Protocol', () => { it('accepts legacy unversioned KeyMarker tokens for in-flight pagination', async () => { const bucketName = await createBucket(client) const runId = randomUUID() - const firstKey = `mp-legacy-${runId}:001.jpg` - const secondKey = `mp-legacy-${runId}:002.jpg` + const firstKey = `mp-legacy-${runId}:001%25.jpg` + const secondKey = `mp-legacy-${runId}:002%25.jpg` await Promise.all( [firstKey, secondKey].map((key) => @@ -2480,6 +2480,8 @@ describe('S3 Protocol', () => { ) expect(page1.Uploads?.[0]?.Key).toBe(firstKey) + expect(page1.NextKeyMarker).toBeTruthy() + expect(Buffer.from(page1.NextKeyMarker!, 'base64').toString()).toMatch(/^v:1\nl:/) const legacyToken = Buffer.from(`l:${firstKey}`).toString('base64') const pageUsingLegacyToken = await client.send( @@ -2491,11 +2493,9 @@ describe('S3 Protocol', () => { ) // Legacy unversioned markers keep legacy parsing semantics for rollout safety. - expect(pageUsingLegacyToken.Uploads?.[0]?.Key).toBe(firstKey) - expect(pageUsingLegacyToken.NextKeyMarker).toBeTruthy() - expect(Buffer.from(pageUsingLegacyToken.NextKeyMarker!, 'base64').toString()).toMatch( - /^v:1\nl:/ - ) + expect(pageUsingLegacyToken.Uploads?.[0]?.Key).toBe(secondKey) + expect(pageUsingLegacyToken.Uploads?.map((upload) => upload.Key)).not.toContain(firstKey) + expect(pageUsingLegacyToken.NextKeyMarker).toBeFalsy() }) it('returns a structured 400 for an invalid multipart KeyMarker', async () => { From 3f4b71118fdb46eb7c20ff553af4afb11a68a7ec Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 13:03:57 +0100 Subject: [PATCH 28/40] fix: backup compat Signed-off-by: ferhat elmas --- src/storage/backend/s3/adapter.ts | 8 +- src/storage/backend/s3/backup.ts | 5 +- src/storage/backend/s3/copy-source.ts | 6 ++ src/test/s3-backup.test.ts | 114 ++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 src/storage/backend/s3/copy-source.ts create mode 100644 src/test/s3-backup.test.ts diff --git a/src/storage/backend/s3/adapter.ts b/src/storage/backend/s3/adapter.ts index 9b3e50a16..7e75e6d65 100644 --- a/src/storage/backend/s3/adapter.ts +++ b/src/storage/backend/s3/adapter.ts @@ -32,6 +32,7 @@ import { UploadPart, withOptionalVersion, } from './../adapter' +import { encodeCopySource } from './copy-source' const { storageS3UploadQueueSize, @@ -52,13 +53,6 @@ export interface S3ClientOptions { requestTimeout?: number } -function encodeCopySource(bucket: string, key: string): string { - return `${encodeURIComponent(bucket)}/${key - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` -} - /** * S3Backend * Interacts with a s3-compatible file system with this S3Adapter diff --git a/src/storage/backend/s3/backup.ts b/src/storage/backend/s3/backup.ts index a9c65d6d6..f165ed0eb 100644 --- a/src/storage/backend/s3/backup.ts +++ b/src/storage/backend/s3/backup.ts @@ -7,6 +7,7 @@ import { S3Client, UploadPartCopyCommand, } from '@aws-sdk/client-s3' +import { encodeCopySource } from './copy-source' const FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -69,7 +70,7 @@ export class ObjectBackup { const copyParams = { Bucket: destinationBucket, Key: destinationKey, - CopySource: encodeURIComponent(`/${sourceBucket}/${sourceKey}`), + CopySource: encodeCopySource(sourceBucket, sourceKey), } const copyCommand = new CopyObjectCommand(copyParams) @@ -157,7 +158,7 @@ export class ObjectBackup { Key: destinationKey, PartNumber: partNumber, UploadId: uploadId, - CopySource: encodeURIComponent(`/${sourceBucket}/${sourceKey}`), + CopySource: encodeCopySource(sourceBucket, sourceKey), CopySourceRange: `bytes=${start}-${end}`, }) diff --git a/src/storage/backend/s3/copy-source.ts b/src/storage/backend/s3/copy-source.ts new file mode 100644 index 000000000..c6692d34f --- /dev/null +++ b/src/storage/backend/s3/copy-source.ts @@ -0,0 +1,6 @@ +export function encodeCopySource(bucket: string, key: string): string { + return `${encodeURIComponent(bucket)}/${key + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` +} diff --git a/src/test/s3-backup.test.ts b/src/test/s3-backup.test.ts new file mode 100644 index 000000000..d3d39e553 --- /dev/null +++ b/src/test/s3-backup.test.ts @@ -0,0 +1,114 @@ +'use strict' + +import { + CompleteMultipartUploadCommand, + CopyObjectCommand, + CreateMultipartUploadCommand, + S3Client, + UploadPartCopyCommand, +} from '@aws-sdk/client-s3' +import { ObjectBackup } from '../storage/backend/s3/backup' + +jest.mock('@aws-sdk/client-s3', () => { + const originalModule = jest.requireActual('@aws-sdk/client-s3') + return { + ...originalModule, + S3Client: jest.fn().mockImplementation(() => ({ + send: jest.fn(), + })), + } +}) + +const encodeCopySourceByPathToken = (bucket: string, key: string) => + `${encodeURIComponent(bucket)}/${key + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/')}` + +describe('ObjectBackup', () => { + let mockSend: jest.Mock + let client: S3Client + + beforeEach(() => { + jest.clearAllMocks() + mockSend = jest.fn() + ;(S3Client as jest.Mock).mockImplementation(() => ({ + send: mockSend, + })) + client = new S3Client({}) as unknown as S3Client + }) + + test('singleCopy preserves path separators for Unicode source keys', async () => { + mockSend.mockResolvedValue({}) + + const sourceKey = 'folder one/일이삼/子目录/🙂?#%.png' + const destinationKey = 'backup/folder/복사본.png' + + const backup = new ObjectBackup(client, { + sourceBucket: 'source-bucket', + sourceKey, + destinationBucket: 'backup-bucket', + destinationKey, + size: 1024, + }) + + await backup.backup() + + expect(mockSend).toHaveBeenCalledTimes(1) + const command = mockSend.mock.calls[0][0] as CopyObjectCommand + expect(command).toBeInstanceOf(CopyObjectCommand) + expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('source-bucket', sourceKey)) + expect(command.input.CopySource).toContain('source-bucket/folder%20one/') + expect(command.input.CopySource).not.toContain('%2Fsource-bucket%2F') + expect(command.input.CopySource).not.toContain('source-bucket%2F') + }) + + test('multipartCopy preserves path separators for Unicode source keys', async () => { + mockSend.mockImplementation((command: unknown) => { + if (command instanceof CreateMultipartUploadCommand) { + return Promise.resolve({ UploadId: 'upload-id' }) + } + + if (command instanceof UploadPartCopyCommand) { + return Promise.resolve({ + CopyPartResult: { + ETag: `"etag-${command.input.PartNumber}"`, + }, + }) + } + + if (command instanceof CompleteMultipartUploadCommand) { + return Promise.resolve({}) + } + + return Promise.resolve({}) + }) + + const sourceKey = 'folder one/일이삼/子目录/🙂?#%.png' + const destinationKey = 'backup/folder/복사본.png' + const partSize = 5 * 1024 * 1024 * 1024 + const backup = new ObjectBackup(client, { + sourceBucket: 'source-bucket', + sourceKey, + destinationBucket: 'backup-bucket', + destinationKey, + size: partSize + 1024, + }) + + await backup.backup() + + const uploadPartCommands = mockSend.mock.calls + .map(([command]) => command) + .filter( + (command): command is UploadPartCopyCommand => command instanceof UploadPartCopyCommand + ) + + expect(uploadPartCommands).toHaveLength(2) + for (const command of uploadPartCommands) { + expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('source-bucket', sourceKey)) + expect(command.input.CopySource).toContain('source-bucket/folder%20one/') + expect(command.input.CopySource).not.toContain('%2Fsource-bucket%2F') + expect(command.input.CopySource).not.toContain('source-bucket%2F') + } + }) +}) From 545465cd7a50c06654bd1322d2decbd45eba3c2c Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 14:31:31 +0100 Subject: [PATCH 29/40] fix: sign upload canonical raw validation Signed-off-by: ferhat elmas --- src/http/routes/object/uploadSignedObject.ts | 10 ++++-- src/storage/object.ts | 4 +-- src/test/object.test.ts | 32 ++++++++++++++++++++ src/test/signed-url-route.test.ts | 9 ++++++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/http/routes/object/uploadSignedObject.ts b/src/http/routes/object/uploadSignedObject.ts index d0c280ebe..fdc80f07d 100644 --- a/src/http/routes/object/uploadSignedObject.ts +++ b/src/http/routes/object/uploadSignedObject.ts @@ -1,7 +1,9 @@ import fastifyMultipart from '@fastify/multipart' +import { ERRORS } from '@internal/errors' import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' import { ROUTE_OPERATIONS } from '../operations' +import { doesSignedTokenMatchRequestPath } from '../signed-url' const uploadSignedObjectParamsSchema = { type: 'object', @@ -83,9 +85,13 @@ export default async function routes(fastify: FastifyInstance) { const { bucketName } = request.params const objectName = request.params['*'] - const { owner, upsert } = await request.storage + const { owner, upsert, url } = await request.storage .from(bucketName) - .verifyObjectSignature(token, objectName) + .verifyObjectSignature(token) + + if (!doesSignedTokenMatchRequestPath(request.raw.url, '/object/upload/sign', url)) { + throw ERRORS.InvalidSignature() + } const { objectMetadata, path } = await request.storage .asSuperUser() diff --git a/src/storage/object.ts b/src/storage/object.ts index 231ed4790..b5270bf16 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -841,7 +841,7 @@ export class ObjectStorage { * @param token * @param objectName */ - async verifyObjectSignature(token: string, objectName: string) { + async verifyObjectSignature(token: string, objectName?: string) { const { secret: jwtSecret, jwks } = await getJwtSecret(this.db.tenantId) let payload: SignedUploadToken @@ -854,7 +854,7 @@ export class ObjectStorage { const { url, exp } = payload - if (url !== `${this.bucketId}/${objectName}`) { + if (objectName && url !== `${this.bucketId}/${objectName}`) { throw ERRORS.InvalidSignature() } diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 754be7b22..9eea470f9 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -3602,6 +3602,38 @@ describe('Object key names with Unicode characters', () => { expect(objectResponse?.name).toBe(objectName) }) + test('rejects double-encoded signed upload paths', async () => { + const objectName = `signed-upload-double-${randomUUID()}-éè-中文-🙂.png` + const authorization = `Bearer ${await serviceKeyAsync}` + + const signedUploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/upload/sign/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(signedUploadResponse.statusCode).toBe(200) + + const signedUpload = signedUploadResponse.json<{ url: string }>() + const signedUploadURL = new URL(signedUpload.url, 'http://localhost') + const doubleEncodedPath = signedUploadURL.pathname.replaceAll('%', '%25') + + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadResponse = await appInstance.inject({ + method: 'PUT', + url: `${doubleEncodedPath}${signedUploadURL.search}`, + headers: { + ...form.getHeaders(), + }, + payload: form, + }) + + expect(uploadResponse.statusCode).toBe(400) + expect(uploadResponse.json<{ error: string }>().error).toBe('InvalidSignature') + }) + test('can sign and upload using returned signed upload URL for nested Unicode and URL-reserved object names', async () => { const objectName = `signed-upload-unicode-${randomUUID()}-폴더?x=1&y=%25+plus;semi:colon,/子目录#frag/파일-🙂.png` const authorization = `Bearer ${await serviceKeyAsync}` diff --git a/src/test/signed-url-route.test.ts b/src/test/signed-url-route.test.ts index 49ae3be3a..c6ddb3886 100644 --- a/src/test/signed-url-route.test.ts +++ b/src/test/signed-url-route.test.ts @@ -23,6 +23,15 @@ describe('signed URL route path verification', () => { ) }) + test('matches canonical encoded object path for upload signed route', () => { + const signedObjectPath = 'bucket2/public/일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' + const rawPath = `/object/upload/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + + expect(doesSignedTokenMatchRequestPath(rawPath, '/object/upload/sign', signedObjectPath)).toBe( + true + ) + }) + test('rejects double-encoded request paths', () => { const signedObjectPath = 'bucket2/public/일이삼.txt' const encodedPath = encodeObjectPathForURL(signedObjectPath) From ccd1c28c4621da36aa720da87783f8e0834cb5ca Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 14:53:10 +0100 Subject: [PATCH 30/40] fix: list url encoding type prefixes Signed-off-by: ferhat elmas --- src/storage/object.ts | 14 +++++++- src/storage/protocols/s3/s3-handler.ts | 12 ++++++- src/test/s3-protocol.test.ts | 45 ++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/storage/object.ts b/src/storage/object.ts index b5270bf16..a57b70989 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -56,6 +56,13 @@ function encodeObjectPathForURL(bucketId: string, objectName: string): string { .join('/')}` } +function encodePathPreservingSeparators(path: string): string { + return path + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') +} + /** * ObjectStorage * interact with remote objects and database state @@ -676,7 +683,12 @@ export class ObjectStorage { const name = obj.id === null && !obj.name.endsWith('/') ? obj.name + '/' : obj.name target.push({ ...obj, - name: options?.encodingType === 'url' ? encodeURIComponent(name) : name, + name: + options?.encodingType === 'url' + ? obj.id === null + ? encodePathPreservingSeparators(name) + : encodeURIComponent(name) + : name, }) }) diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index faab803ff..8ef6d32cd 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -333,7 +333,10 @@ export class S3ProtocolHandler { delimitedResults.push({ isFolder: true, id: object.id, - key: command.EncodingType === 'url' ? encodeURIComponent(currPrefix) : currPrefix, + key: + command.EncodingType === 'url' + ? encodePathPreservingSeparators(currPrefix) + : currPrefix, bucket_id: bucket, }) continue @@ -1414,6 +1417,13 @@ function isUSASCII(str: string): boolean { return true } +function encodePathPreservingSeparators(path: string): string { + return path + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') +} + function parseCopySource(copySource: string): { bucketName: string objectKey: string diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index a4250b726..a046d4ade 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -385,6 +385,25 @@ describe('S3 Protocol', () => { expect(resp.KeyCount).toBe(3) }) + it('preserves delimiter slashes in CommonPrefixes when EncodingType=url', async () => { + const bucket = await createBucket(client) + await Promise.all([ + uploadFile(client, bucket, '日本語/子目录/test-1.jpg', 1), + uploadFile(client, bucket, 'plain-root.jpg', 1), + ]) + + const resp = await client.send( + new ListObjectsV2Command({ + Bucket: bucket, + Delimiter: '/', + EncodingType: 'url', + }) + ) + + expect(resp.CommonPrefixes?.[0].Prefix).toBe(`${encodeURIComponent('日本語')}/`) + expect(resp.CommonPrefixes?.[0].Prefix).not.toContain('%2F') + }) + it('paginate keys and common prefixes', async () => { const bucket = await createBucket(client) const listBucketsPage1 = new ListObjectsV2Command({ @@ -1383,6 +1402,32 @@ describe('S3 Protocol', () => { expect(resp.CommonPrefixes?.[0].Prefix).toBe('nested/') }) + it('preserves delimiter slashes in multipart CommonPrefixes when EncodingType=url', async () => { + const bucketName = await createBucket(client) + const createMultiPartUpload = (key: string) => + new CreateMultipartUploadCommand({ + Bucket: bucketName, + Key: key, + ContentType: 'image/jpg', + }) + + await Promise.all([ + client.send(createMultiPartUpload('日本語/子目录/test-4.jpg')), + client.send(createMultiPartUpload('root-file.jpg')), + ]) + + const resp = await client.send( + new ListMultipartUploadsCommand({ + Bucket: bucketName, + Delimiter: '/', + EncodingType: 'url', + }) + ) + + expect(resp.CommonPrefixes?.[0].Prefix).toBe(`${encodeURIComponent('日本語')}/`) + expect(resp.CommonPrefixes?.[0].Prefix).not.toContain('%2F') + }) + it('treats % as a literal character in multipart prefix filtering with delimiter', async () => { const bucketName = await createBucket(client) const createMultiPartUpload = (key: string) => From a86d443f36146f6bea84fa6927d446d85789dfaa Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 15:00:47 +0100 Subject: [PATCH 31/40] fix: more cover for list v1 pagination Signed-off-by: ferhat elmas --- src/test/s3-protocol.test.ts | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index a046d4ade..e59466799 100644 --- a/src/test/s3-protocol.test.ts +++ b/src/test/s3-protocol.test.ts @@ -296,6 +296,54 @@ describe('S3 Protocol', () => { expect(resp2.CommonPrefixes?.length).toBe(2) expect(resp2.Contents?.length).toBe(1) }) + + it('paginates v1 listings with Unicode keys across pages', async () => { + const bucket = await createBucket(client) + const keys = [ + `v1-unicode-${randomUUID()}-éè-中文-🙂.jpg`, + `v1-unicode-${randomUUID()}-일이삼-🙂.jpg`, + `v1-unicode-${randomUUID()}-폴더-子目录-🙂.jpg`, + ].sort() + + await Promise.all(keys.map((key) => uploadFile(client, bucket, key, 1))) + + const page1 = await client.send( + new ListObjectsCommand({ + Bucket: bucket, + MaxKeys: 1, + }) + ) + expect(page1.Contents?.length).toBe(1) + expect(page1.Marker).toBeTruthy() + + const page2 = await client.send( + new ListObjectsCommand({ + Bucket: bucket, + MaxKeys: 1, + Marker: page1.Marker, + }) + ) + expect(page2.Contents?.length).toBe(1) + expect(page2.Marker).toBeTruthy() + + const page3 = await client.send( + new ListObjectsCommand({ + Bucket: bucket, + MaxKeys: 1, + Marker: page2.Marker, + }) + ) + expect(page3.Contents?.length).toBe(1) + + const pagedKeys = [ + page1.Contents?.[0]?.Key, + page2.Contents?.[0]?.Key, + page3.Contents?.[0]?.Key, + ].filter(Boolean) + expect(pagedKeys).toHaveLength(3) + expect(new Set(pagedKeys).size).toBe(3) + expect(pagedKeys).toEqual(keys) + }) }) describe('ListObjectsV2Command', () => { From f110ad9641e4a8b720cefa440bf65f6bb5bfe1da Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 15:07:36 +0100 Subject: [PATCH 32/40] fix: strict path verification for sign Signed-off-by: ferhat elmas --- src/http/routes/object/uploadSignedObject.ts | 20 +++++++++++++++++--- src/storage/object.ts | 4 ++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/http/routes/object/uploadSignedObject.ts b/src/http/routes/object/uploadSignedObject.ts index fdc80f07d..dd94cfaa4 100644 --- a/src/http/routes/object/uploadSignedObject.ts +++ b/src/http/routes/object/uploadSignedObject.ts @@ -1,4 +1,6 @@ import fastifyMultipart from '@fastify/multipart' +import { SignedUploadToken, verifyJWT } from '@internal/auth' +import { getJwtSecret } from '@internal/database' import { ERRORS } from '@internal/errors' import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' @@ -85,14 +87,26 @@ export default async function routes(fastify: FastifyInstance) { const { bucketName } = request.params const objectName = request.params['*'] - const { owner, upsert, url } = await request.storage - .from(bucketName) - .verifyObjectSignature(token) + let payload: SignedUploadToken + const { secret: jwtSecret, jwks } = await getJwtSecret(request.tenantId) + + try { + payload = (await verifyJWT(token, jwtSecret, jwks)) as SignedUploadToken + } catch (e) { + const err = e as Error + throw ERRORS.InvalidJWT(err) + } + + const { owner, upsert, url, exp } = payload if (!doesSignedTokenMatchRequestPath(request.raw.url, '/object/upload/sign', url)) { throw ERRORS.InvalidSignature() } + if (exp * 1000 < Date.now()) { + throw ERRORS.ExpiredSignature() + } + const { objectMetadata, path } = await request.storage .asSuperUser() .from(bucketName) diff --git a/src/storage/object.ts b/src/storage/object.ts index a57b70989..3a59de490 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -853,7 +853,7 @@ export class ObjectStorage { * @param token * @param objectName */ - async verifyObjectSignature(token: string, objectName?: string) { + async verifyObjectSignature(token: string, objectName: string) { const { secret: jwtSecret, jwks } = await getJwtSecret(this.db.tenantId) let payload: SignedUploadToken @@ -866,7 +866,7 @@ export class ObjectStorage { const { url, exp } = payload - if (objectName && url !== `${this.bucketId}/${objectName}`) { + if (url !== `${this.bucketId}/${objectName}`) { throw ERRORS.InvalidSignature() } From a72f6fac07e1d7dceb8d78f1380a1d26ce66485c Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 15:22:24 +0100 Subject: [PATCH 33/40] fix: encoder dedupe Signed-off-by: ferhat elmas --- src/http/routes/signed-url.ts | 7 +++---- src/storage/backend/s3/copy-source.ts | 7 +++---- src/storage/object.ts | 13 ++----------- src/storage/path-encoding.ts | 10 ++++++++++ src/storage/protocols/s3/s3-handler.ts | 8 +------- src/test/s3-adapter.test.ts | 15 +++++++-------- src/test/s3-backup.test.ts | 15 +++++++-------- src/test/signed-url-route.test.ts | 15 +++++---------- src/test/utils/path-encoding.ts | 10 ++++++++++ 9 files changed, 48 insertions(+), 52 deletions(-) create mode 100644 src/storage/path-encoding.ts create mode 100644 src/test/utils/path-encoding.ts diff --git a/src/http/routes/signed-url.ts b/src/http/routes/signed-url.ts index 2c213ceec..4f962ddab 100644 --- a/src/http/routes/signed-url.ts +++ b/src/http/routes/signed-url.ts @@ -1,13 +1,12 @@ +import { encodePathPreservingSeparators } from '../../storage/path-encoding' + function stripQueryString(rawUrl: string): string { const queryIdx = rawUrl.indexOf('?') return queryIdx === -1 ? rawUrl : rawUrl.slice(0, queryIdx) } function encodeObjectPathForURL(objectPath: string): string { - return objectPath - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/') + return encodePathPreservingSeparators(objectPath) } export function doesSignedTokenMatchRequestPath( diff --git a/src/storage/backend/s3/copy-source.ts b/src/storage/backend/s3/copy-source.ts index c6692d34f..27dce3703 100644 --- a/src/storage/backend/s3/copy-source.ts +++ b/src/storage/backend/s3/copy-source.ts @@ -1,6 +1,5 @@ +import { encodeBucketAndObjectPath } from '../../path-encoding' + export function encodeCopySource(bucket: string, key: string): string { - return `${encodeURIComponent(bucket)}/${key - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` + return encodeBucketAndObjectPath(bucket, key) } diff --git a/src/storage/object.ts b/src/storage/object.ts index 3a59de490..8a9864a9e 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -17,6 +17,7 @@ import { ObjectUpdatedMetadata, } from './events' import { mustBeValidKey } from './limits' +import { encodeBucketAndObjectPath, encodePathPreservingSeparators } from './path-encoding' import { fileUploadFromRequest, Uploader, UploadRequest } from './uploader' const { requestUrlLengthLimit } = getConfig() @@ -50,17 +51,7 @@ export interface ListObjectsV2Result { } function encodeObjectPathForURL(bucketId: string, objectName: string): string { - return `${encodeURIComponent(bucketId)}/${objectName - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` -} - -function encodePathPreservingSeparators(path: string): string { - return path - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/') + return encodeBucketAndObjectPath(bucketId, objectName) } /** diff --git a/src/storage/path-encoding.ts b/src/storage/path-encoding.ts new file mode 100644 index 000000000..8ebf89331 --- /dev/null +++ b/src/storage/path-encoding.ts @@ -0,0 +1,10 @@ +export function encodePathPreservingSeparators(path: string): string { + return path + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') +} + +export function encodeBucketAndObjectPath(bucket: string, key: string): string { + return `${encodeURIComponent(bucket)}/${encodePathPreservingSeparators(key)}` +} diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index 8ef6d32cd..3437dfd33 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -24,6 +24,7 @@ import { PassThrough, Readable } from 'stream' import stream from 'stream/promises' import { getConfig } from '../../../config' import { getFileSizeLimit, mustBeValidBucketName, mustBeValidKey } from '../../limits' +import { encodePathPreservingSeparators } from '../../path-encoding' import { S3MultipartUpload } from '../../schemas' import { Storage } from '../../storage' import { Uploader, validateMimeType } from '../../uploader' @@ -1417,13 +1418,6 @@ function isUSASCII(str: string): boolean { return true } -function encodePathPreservingSeparators(path: string): string { - return path - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/') -} - function parseCopySource(copySource: string): { bucketName: string objectKey: string diff --git a/src/test/s3-adapter.test.ts b/src/test/s3-adapter.test.ts index fc642f29d..fdb1f276f 100644 --- a/src/test/s3-adapter.test.ts +++ b/src/test/s3-adapter.test.ts @@ -3,6 +3,7 @@ import { CopyObjectCommand, S3Client, UploadPartCopyCommand } from '@aws-sdk/client-s3' import { Readable } from 'stream' import { S3Backend } from '../storage/backend/s3/adapter' +import { encodeBucketAndObjectPathForTest } from './utils/path-encoding' jest.mock('@aws-sdk/client-s3', () => { const originalModule = jest.requireActual('@aws-sdk/client-s3') @@ -14,12 +15,6 @@ jest.mock('@aws-sdk/client-s3', () => { } }) -const encodeCopySourceByPathToken = (bucket: string, key: string) => - `${encodeURIComponent(bucket)}/${key - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` - describe('S3Backend', () => { let mockSend: jest.Mock @@ -107,7 +102,9 @@ describe('S3Backend', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as CopyObjectCommand expect(command).toBeInstanceOf(CopyObjectCommand) - expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('test-bucket', sourceKey)) + expect(command.input.CopySource).toBe( + encodeBucketAndObjectPathForTest('test-bucket', sourceKey) + ) expect(command.input.CopySource).toContain('test-bucket/source%20path/') expect(command.input.CopySource).not.toContain('test-bucket%2F') }) @@ -145,7 +142,9 @@ describe('S3Backend', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as UploadPartCopyCommand expect(command).toBeInstanceOf(UploadPartCopyCommand) - expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('test-bucket', sourceKey)) + expect(command.input.CopySource).toBe( + encodeBucketAndObjectPathForTest('test-bucket', sourceKey) + ) expect(command.input.CopySource).toContain('test-bucket/source%20path/folder/') expect(command.input.CopySource).not.toContain('test-bucket%2F') expect(command.input.CopySourceRange).toBe('bytes=0-1024') diff --git a/src/test/s3-backup.test.ts b/src/test/s3-backup.test.ts index d3d39e553..24b15f169 100644 --- a/src/test/s3-backup.test.ts +++ b/src/test/s3-backup.test.ts @@ -8,6 +8,7 @@ import { UploadPartCopyCommand, } from '@aws-sdk/client-s3' import { ObjectBackup } from '../storage/backend/s3/backup' +import { encodeBucketAndObjectPathForTest } from './utils/path-encoding' jest.mock('@aws-sdk/client-s3', () => { const originalModule = jest.requireActual('@aws-sdk/client-s3') @@ -19,12 +20,6 @@ jest.mock('@aws-sdk/client-s3', () => { } }) -const encodeCopySourceByPathToken = (bucket: string, key: string) => - `${encodeURIComponent(bucket)}/${key - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/')}` - describe('ObjectBackup', () => { let mockSend: jest.Mock let client: S3Client @@ -57,7 +52,9 @@ describe('ObjectBackup', () => { expect(mockSend).toHaveBeenCalledTimes(1) const command = mockSend.mock.calls[0][0] as CopyObjectCommand expect(command).toBeInstanceOf(CopyObjectCommand) - expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('source-bucket', sourceKey)) + expect(command.input.CopySource).toBe( + encodeBucketAndObjectPathForTest('source-bucket', sourceKey) + ) expect(command.input.CopySource).toContain('source-bucket/folder%20one/') expect(command.input.CopySource).not.toContain('%2Fsource-bucket%2F') expect(command.input.CopySource).not.toContain('source-bucket%2F') @@ -105,7 +102,9 @@ describe('ObjectBackup', () => { expect(uploadPartCommands).toHaveLength(2) for (const command of uploadPartCommands) { - expect(command.input.CopySource).toBe(encodeCopySourceByPathToken('source-bucket', sourceKey)) + expect(command.input.CopySource).toBe( + encodeBucketAndObjectPathForTest('source-bucket', sourceKey) + ) expect(command.input.CopySource).toContain('source-bucket/folder%20one/') expect(command.input.CopySource).not.toContain('%2Fsource-bucket%2F') expect(command.input.CopySource).not.toContain('source-bucket%2F') diff --git a/src/test/signed-url-route.test.ts b/src/test/signed-url-route.test.ts index c6ddb3886..76a3d43ca 100644 --- a/src/test/signed-url-route.test.ts +++ b/src/test/signed-url-route.test.ts @@ -1,22 +1,17 @@ import { doesSignedTokenMatchRequestPath } from '../http/routes/signed-url' - -const encodeObjectPathForURL = (objectPath: string) => - objectPath - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/') +import { encodePathPreservingSeparatorsForTest } from './utils/path-encoding' describe('signed URL route path verification', () => { test('matches canonical encoded object path for object signed route', () => { const signedObjectPath = 'bucket2/public/일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' - const rawPath = `/object/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + const rawPath = `/object/sign/${encodePathPreservingSeparatorsForTest(signedObjectPath)}?token=jwt` expect(doesSignedTokenMatchRequestPath(rawPath, '/object/sign', signedObjectPath)).toBe(true) }) test('matches canonical encoded object path for render signed route', () => { const signedObjectPath = 'bucket2/authenticated/casestudy.png' - const rawPath = `/render/image/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + const rawPath = `/render/image/sign/${encodePathPreservingSeparatorsForTest(signedObjectPath)}?token=jwt` expect(doesSignedTokenMatchRequestPath(rawPath, '/render/image/sign', signedObjectPath)).toBe( true @@ -25,7 +20,7 @@ describe('signed URL route path verification', () => { test('matches canonical encoded object path for upload signed route', () => { const signedObjectPath = 'bucket2/public/일이삼-🙂-q?foo=1&bar=%25+plus;semi:colon,#frag.png' - const rawPath = `/object/upload/sign/${encodeObjectPathForURL(signedObjectPath)}?token=jwt` + const rawPath = `/object/upload/sign/${encodePathPreservingSeparatorsForTest(signedObjectPath)}?token=jwt` expect(doesSignedTokenMatchRequestPath(rawPath, '/object/upload/sign', signedObjectPath)).toBe( true @@ -34,7 +29,7 @@ describe('signed URL route path verification', () => { test('rejects double-encoded request paths', () => { const signedObjectPath = 'bucket2/public/일이삼.txt' - const encodedPath = encodeObjectPathForURL(signedObjectPath) + const encodedPath = encodePathPreservingSeparatorsForTest(signedObjectPath) const doubleEncodedPath = encodedPath.replaceAll('%', '%25') const rawPath = `/object/sign/${doubleEncodedPath}?token=jwt` diff --git a/src/test/utils/path-encoding.ts b/src/test/utils/path-encoding.ts new file mode 100644 index 000000000..fc5625b77 --- /dev/null +++ b/src/test/utils/path-encoding.ts @@ -0,0 +1,10 @@ +export function encodePathPreservingSeparatorsForTest(path: string): string { + return path + .split('/') + .map((pathToken) => encodeURIComponent(pathToken)) + .join('/') +} + +export function encodeBucketAndObjectPathForTest(bucket: string, key: string): string { + return `${encodeURIComponent(bucket)}/${encodePathPreservingSeparatorsForTest(key)}` +} From 81352907b32b3c5877c5da718a1c02d3ad0531b3 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 15:38:24 +0100 Subject: [PATCH 34/40] fix: more gaps --- .../protocols/s3/copy-source-parser.ts | 42 ++++++++++++++ src/storage/protocols/s3/s3-handler.ts | 42 +------------- src/test/error-codes.test.ts | 10 ++++ src/test/object-list-v2.test.ts | 55 +++++++++++++++++++ src/test/s3-copy-source-parser.test.ts | 39 +++++++++++++ 5 files changed, 147 insertions(+), 41 deletions(-) create mode 100644 src/storage/protocols/s3/copy-source-parser.ts create mode 100644 src/test/s3-copy-source-parser.test.ts diff --git a/src/storage/protocols/s3/copy-source-parser.ts b/src/storage/protocols/s3/copy-source-parser.ts new file mode 100644 index 000000000..1adee700a --- /dev/null +++ b/src/storage/protocols/s3/copy-source-parser.ts @@ -0,0 +1,42 @@ +import { ERRORS } from '@internal/errors' + +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('?') + + let decodedPath = '' + try { + decodedPath = decodeURIComponent(encodedPath) + } catch { + throw ERRORS.InvalidParameter('CopySource') + } + + const separatorIdx = decodedPath.indexOf('/') + if (separatorIdx <= 0) { + throw ERRORS.MissingParameter('CopySource') + } + + const bucketName = decodedPath.slice(0, separatorIdx) + const objectKey = decodedPath.slice(separatorIdx + 1) + if (!objectKey) { + 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, + sourceVersion, + } +} diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index 3437dfd33..83f5d6ef0 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -29,6 +29,7 @@ import { S3MultipartUpload } from '../../schemas' import { Storage } from '../../storage' import { Uploader, validateMimeType } from '../../uploader' import { ByteLimitTransformStream } from './byte-limit-stream' +import { parseCopySource } from './copy-source-parser' const { storageS3Region, storageS3Bucket } = getConfig() @@ -1418,47 +1419,6 @@ function isUSASCII(str: string): boolean { return true } -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('?') - - let decodedPath = '' - try { - decodedPath = decodeURIComponent(encodedPath) - } catch { - throw ERRORS.InvalidParameter('CopySource') - } - - const separatorIdx = decodedPath.indexOf('/') - if (separatorIdx <= 0) { - throw ERRORS.MissingParameter('CopySource') - } - - const bucketName = decodedPath.slice(0, separatorIdx) - const objectKey = decodedPath.slice(separatorIdx + 1) - if (!objectKey) { - 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, - sourceVersion, - } -} - function encodeContinuationToken(name: string) { return Buffer.from(`v:1\nl:${encodeURIComponent(name)}`).toString('base64') } diff --git a/src/test/error-codes.test.ts b/src/test/error-codes.test.ts index c62b4a5c3..b6e3e79aa 100644 --- a/src/test/error-codes.test.ts +++ b/src/test/error-codes.test.ts @@ -23,4 +23,14 @@ describe('ERRORS.InvalidKey', () => { expect(error.httpStatusCode).toBe(400) expect(error.message).toBe('Invalid key: bad-%EF%BF%BD-key') }) + + it('encodes valid Unicode and reserved characters in InvalidKey messages', () => { + const malformedKey = 'bad-일이삼/🙂?#%.png' + + const error = ERRORS.InvalidKey(malformedKey) + + expect(error.code).toBe(ErrorCode.InvalidKey) + expect(error.httpStatusCode).toBe(400) + expect(error.message).toBe(`Invalid key: ${encodeURIComponent(malformedKey)}`) + }) }) diff --git a/src/test/object-list-v2.test.ts b/src/test/object-list-v2.test.ts index 9169a130e..c89bd4195 100644 --- a/src/test/object-list-v2.test.ts +++ b/src/test/object-list-v2.test.ts @@ -850,4 +850,59 @@ describe('objects - list v2 cursor encoding', () => { expect(page2.objects).toHaveLength(1) expect(page2.objects[0]?.name).toBe(secondKey) }) + + test('supports legacy unescaped cursor values with mixed Unicode and literal % sequences', async () => { + const runId = randomUUID() + const prefix = `cursor-legacy-unicode-${runId}-` + const firstKey = `${prefix}일이삼-%20-🙂.txt` + const secondKey = `${prefix}일이삼-~.txt` + + for (const key of [firstKey, secondKey]) { + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/${LIST_V2_CURSOR_BUCKET}/${encodeURIComponent(key)}`, + payload: createUpload('utf8.txt', 'cursor legacy unicode test'), + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(uploadResponse.statusCode).toBe(200) + } + + const page1Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page1Response.statusCode).toBe(200) + const page1 = page1Response.json() + expect(page1.objects).toHaveLength(1) + expect(page1.objects[0]?.name).toBe(firstKey) + + const legacyCursor = Buffer.from(`l:${firstKey}\no:asc`).toString('base64') + const page2Response = await appInstance.inject({ + method: 'POST', + url: `/object/list-v2/${LIST_V2_CURSOR_BUCKET}`, + payload: { + with_delimiter: false, + prefix, + limit: 1, + cursor: legacyCursor, + }, + headers: { + authorization: `Bearer ${serviceKey}`, + }, + }) + expect(page2Response.statusCode).toBe(200) + const page2 = page2Response.json() + expect(page2.objects).toHaveLength(1) + expect(page2.objects[0]?.name).toBe(secondKey) + }) }) diff --git a/src/test/s3-copy-source-parser.test.ts b/src/test/s3-copy-source-parser.test.ts new file mode 100644 index 000000000..e7dfc8e45 --- /dev/null +++ b/src/test/s3-copy-source-parser.test.ts @@ -0,0 +1,39 @@ +'use strict' + +import { ErrorCode, StorageBackendError } from '@internal/errors' +import { parseCopySource } from '../storage/protocols/s3/copy-source-parser' + +describe('parseCopySource', () => { + 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' + ) + + expect(result).toEqual({ + bucketName: 'bucket', + objectKey: 'folder one/일이삼.png', + sourceVersion: 'v1?part=2', + }) + }) + + test('accepts fully URL-encoded CopySource values with versionId', () => { + const result = parseCopySource( + `${encodeURIComponent('bucket/folder one/일이삼/🙂?#%.png')}?versionId=ver-123` + ) + + expect(result).toEqual({ + bucketName: 'bucket', + objectKey: 'folder one/일이삼/🙂?#%.png', + sourceVersion: 'ver-123', + }) + }) + + test('rejects an empty versionId query value', () => { + expect(() => parseCopySource('bucket/key?versionId=')).toThrow( + expect.objectContaining>({ + code: ErrorCode.MissingParameter, + message: 'Invalid Parameter CopySource', + }) + ) + }) +}) From 3b468c6e307fb436e20dc0abacfc9d1054a25494 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 16:43:56 +0100 Subject: [PATCH 35/40] fix: extend migration for multipart tables Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 62 +++++++++++++++++++ src/storage/database/knex.ts | 10 ++- src/test/database-error-mapping.test.ts | 41 ++++++++++++ .../unicode-object-name-migration.test.ts | 25 ++++++++ 4 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 src/test/database-error-mapping.test.ts create mode 100644 src/test/unicode-object-name-migration.test.ts diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index 3193fdb52..37c29093d 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -32,5 +32,67 @@ BEGIN $ddl$; END IF; END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 's3_multipart_uploads_key_check' + AND n.nspname = 'storage' + AND t.relname = 's3_multipart_uploads' + ) THEN + IF server_encoding = 'SQL_ASCII' THEN + EXECUTE $ddl$ + ALTER TABLE "storage"."s3_multipart_uploads" + ADD CONSTRAINT s3_multipart_uploads_key_check + CHECK ( + key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND key !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' + ) + $ddl$; + ELSE + EXECUTE $ddl$ + ALTER TABLE "storage"."s3_multipart_uploads" + ADD CONSTRAINT s3_multipart_uploads_key_check + CHECK ( + key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND POSITION(U&'\FFFE' IN key) = 0 + AND POSITION(U&'\FFFF' IN key) = 0 + ) + $ddl$; + END IF; + END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 's3_multipart_uploads_parts_key_check' + AND n.nspname = 'storage' + AND t.relname = 's3_multipart_uploads_parts' + ) THEN + IF server_encoding = 'SQL_ASCII' THEN + EXECUTE $ddl$ + ALTER TABLE "storage"."s3_multipart_uploads_parts" + ADD CONSTRAINT s3_multipart_uploads_parts_key_check + CHECK ( + key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND key !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' + ) + $ddl$; + ELSE + EXECUTE $ddl$ + ALTER TABLE "storage"."s3_multipart_uploads_parts" + ADD CONSTRAINT s3_multipart_uploads_parts_key_check + CHECK ( + key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND POSITION(U&'\FFFE' IN key) = 0 + AND POSITION(U&'\FFFF' IN key) = 0 + ) + $ddl$; + END IF; + END IF; END $$; diff --git a/src/storage/database/knex.ts b/src/storage/database/knex.ts index e3e6915a9..62eeb96a5 100644 --- a/src/storage/database/knex.ts +++ b/src/storage/database/knex.ts @@ -1114,6 +1114,12 @@ export class DBError extends StorageBackendError implements RenderableError { } static fromDBError(pgError: DatabaseError, query?: string) { + const objectNameConstraintNames = new Set([ + 'objects_name_check', + 's3_multipart_uploads_key_check', + 's3_multipart_uploads_parts_key_check', + ]) + switch (pgError.code) { case '42501': return ERRORS.AccessDenied( @@ -1146,7 +1152,9 @@ export class DBError extends StorageBackendError implements RenderableError { }) default: const errorMessage = - pgError.code === '23514' && pgError.constraint === 'objects_name_check' + pgError.code === '23514' && + pgError.constraint && + objectNameConstraintNames.has(pgError.constraint) ? 'Invalid object name' : pgError.message return ERRORS.DatabaseError(errorMessage, pgError).withMetadata({ diff --git a/src/test/database-error-mapping.test.ts b/src/test/database-error-mapping.test.ts new file mode 100644 index 000000000..1af043c1f --- /dev/null +++ b/src/test/database-error-mapping.test.ts @@ -0,0 +1,41 @@ +import { ErrorCode } from '@internal/errors' +import { DatabaseError } from 'pg' +import { DBError } from '../storage/database/knex' + +function createPgError(message: string, overrides?: Partial): DatabaseError { + return Object.assign(new Error(message), overrides) as DatabaseError +} + +describe('DBError.fromDBError', () => { + test.each([ + ['objects_name_check', 'insert into storage.objects ...'], + ['s3_multipart_uploads_key_check', 'insert into storage.s3_multipart_uploads ...'], + ['s3_multipart_uploads_parts_key_check', 'insert into storage.s3_multipart_uploads_parts ...'], + ])('maps %s violations to a stable Invalid object name message', (constraint, query) => { + const pgError = createPgError(`violates check constraint "${constraint}"`, { + code: '23514', + constraint, + }) + + const err = DBError.fromDBError(pgError, query) + + expect(err.code).toBe(ErrorCode.DatabaseError) + expect(err.message).toBe('Invalid object name') + expect(err.metadata).toMatchObject({ + code: '23514', + query, + }) + }) + + it('preserves the original database message for other check constraints', () => { + const pgError = createPgError('violates check constraint "something_else"', { + code: '23514', + constraint: 'something_else', + }) + + const err = DBError.fromDBError(pgError) + + expect(err.code).toBe(ErrorCode.DatabaseError) + expect(err.message).toBe('violates check constraint "something_else"') + }) +}) diff --git a/src/test/unicode-object-name-migration.test.ts b/src/test/unicode-object-name-migration.test.ts new file mode 100644 index 000000000..98a744c2f --- /dev/null +++ b/src/test/unicode-object-name-migration.test.ts @@ -0,0 +1,25 @@ +'use strict' + +import fs from 'node:fs' +import path from 'node:path' + +describe('unicode object name migration', () => { + test('keeps both SQL_ASCII and non-SQL_ASCII constraint branches', () => { + const migrationPath = path.resolve( + __dirname, + '../../migrations/tenant/57-unicode-object-names.sql' + ) + const sql = fs.readFileSync(migrationPath, 'utf8') + + expect(sql).toContain(`server_encoding = 'SQL_ASCII'`) + expect(sql).toContain(String.raw`name !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF'`) + expect(sql).toContain(String.raw`POSITION(U&'\FFFE' IN name) = 0`) + expect(sql).toContain(String.raw`POSITION(U&'\FFFF' IN name) = 0`) + expect(sql).toContain('ADD CONSTRAINT objects_name_check') + expect(sql).toContain('ADD CONSTRAINT s3_multipart_uploads_key_check') + expect(sql).toContain('ADD CONSTRAINT s3_multipart_uploads_parts_key_check') + expect(sql).toContain(String.raw`key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]'`) + expect(sql).toContain(String.raw`POSITION(U&'\FFFE' IN key) = 0`) + expect(sql).toContain(String.raw`POSITION(U&'\FFFF' IN key) = 0`) + }) +}) From 84931e7fe995e3d59e56ea70bf7d0a27c6f1265b Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Mon, 9 Mar 2026 17:27:32 +0100 Subject: [PATCH 36/40] fix: improve migration locking and tests Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 54 ++++++++++-- src/test/object.test.ts | 44 ++++++++++ ...unicode-object-name-db-constraints.test.ts | 88 +++++++++++++++++++ .../unicode-object-name-migration.test.ts | 6 ++ 4 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 src/test/unicode-object-name-db-constraints.test.ts diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index 37c29093d..5254375e8 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -17,8 +17,9 @@ BEGIN ADD CONSTRAINT objects_name_check CHECK ( name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND name !~ E'\\xED[\\xA0-\\xBF][\\x80-\\xBF]' AND name !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' - ) + ) NOT VALID $ddl$; ELSE EXECUTE $ddl$ @@ -28,11 +29,24 @@ BEGIN name !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' AND POSITION(U&'\FFFE' IN name) = 0 AND POSITION(U&'\FFFF' IN name) = 0 - ) + ) NOT VALID $ddl$; END IF; END IF; + IF EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 'objects_name_check' + AND n.nspname = 'storage' + AND t.relname = 'objects' + AND c.convalidated = false + ) THEN + EXECUTE 'ALTER TABLE "storage"."objects" VALIDATE CONSTRAINT objects_name_check'; + END IF; + IF NOT EXISTS ( SELECT 1 FROM pg_constraint c @@ -48,8 +62,9 @@ BEGIN ADD CONSTRAINT s3_multipart_uploads_key_check CHECK ( key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND key !~ E'\\xED[\\xA0-\\xBF][\\x80-\\xBF]' AND key !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' - ) + ) NOT VALID $ddl$; ELSE EXECUTE $ddl$ @@ -59,11 +74,24 @@ BEGIN key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' AND POSITION(U&'\FFFE' IN key) = 0 AND POSITION(U&'\FFFF' IN key) = 0 - ) + ) NOT VALID $ddl$; END IF; END IF; + IF EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 's3_multipart_uploads_key_check' + AND n.nspname = 'storage' + AND t.relname = 's3_multipart_uploads' + AND c.convalidated = false + ) THEN + EXECUTE 'ALTER TABLE "storage"."s3_multipart_uploads" VALIDATE CONSTRAINT s3_multipart_uploads_key_check'; + END IF; + IF NOT EXISTS ( SELECT 1 FROM pg_constraint c @@ -79,8 +107,9 @@ BEGIN ADD CONSTRAINT s3_multipart_uploads_parts_key_check CHECK ( key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' + AND key !~ E'\\xED[\\xA0-\\xBF][\\x80-\\xBF]' AND key !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF' - ) + ) NOT VALID $ddl$; ELSE EXECUTE $ddl$ @@ -90,9 +119,22 @@ BEGIN key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]' AND POSITION(U&'\FFFE' IN key) = 0 AND POSITION(U&'\FFFF' IN key) = 0 - ) + ) NOT VALID $ddl$; END IF; END IF; + + IF EXISTS ( + SELECT 1 + FROM pg_constraint c + JOIN pg_class t ON t.oid = c.conrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE c.conname = 's3_multipart_uploads_parts_key_check' + AND n.nspname = 'storage' + AND t.relname = 's3_multipart_uploads_parts' + AND c.convalidated = false + ) THEN + EXECUTE 'ALTER TABLE "storage"."s3_multipart_uploads_parts" VALIDATE CONSTRAINT s3_multipart_uploads_parts_key_check'; + END IF; END $$; diff --git a/src/test/object.test.ts b/src/test/object.test.ts index 9eea470f9..782a3848d 100644 --- a/src/test/object.test.ts +++ b/src/test/object.test.ts @@ -3398,6 +3398,50 @@ describe('Object key names with Unicode characters', () => { expect(headResponse.headers['cache-control']).toBe('no-cache') }) + test('treats NFC and NFD object names as distinct keys', async () => { + const prefix = `normalize-${randomUUID()}` + const nfcObjectName = `${prefix}/caf\u00e9.txt` + const nfdObjectName = `${prefix}/cafe\u0301.txt` + const authorization = `Bearer ${await serviceKeyAsync}` + + for (const objectName of [nfcObjectName, nfdObjectName]) { + const form = new FormData() + form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`)) + const uploadResponse = await appInstance.inject({ + method: 'POST', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + ...form.getHeaders(), + }, + payload: form, + }) + expect(uploadResponse.statusCode).toBe(200) + } + + const db = await getSuperuserPostgrestClient() + const storedObjects = await db + .from('objects') + .select('name') + .where('bucket_id', 'bucket2') + .whereIn('name', [nfcObjectName, nfdObjectName]) + + const storedNames = storedObjects.map((entry) => entry.name) + expect(storedNames).toEqual(expect.arrayContaining([nfcObjectName, nfdObjectName])) + expect(new Set(storedNames).size).toBe(2) + + for (const objectName of [nfcObjectName, nfdObjectName]) { + const deleteResponse = await appInstance.inject({ + method: 'DELETE', + url: `/object/bucket2/${encodeURIComponent(objectName)}`, + headers: { + authorization, + }, + }) + expect(deleteResponse.statusCode).toBe(200) + } + }) + test('should not upload if the name contains invalid characters', async () => { const invalidObjectName = getInvalidObjectName() const authorization = `Bearer ${await serviceKeyAsync}` diff --git a/src/test/unicode-object-name-db-constraints.test.ts b/src/test/unicode-object-name-db-constraints.test.ts new file mode 100644 index 000000000..7f55700b7 --- /dev/null +++ b/src/test/unicode-object-name-db-constraints.test.ts @@ -0,0 +1,88 @@ +'use strict' + +import { randomUUID } from 'node:crypto' +import { DatabaseError } from 'pg' +import { useStorage } from './utils/storage' + +describe('Unicode object name database constraints', () => { + const tHelper = useStorage() + const testBucketName = `unicode-db-constraints-${Date.now()}` + + beforeAll(async () => { + await tHelper.database.createBucket({ + id: testBucketName, + name: testBucketName, + }) + }) + + const invalidKey = `invalid-\u000b-${randomUUID()}` + + it('rejects invalid object names at the storage.objects constraint', async () => { + const db = tHelper.database.connection.pool.acquire() + const tnx = await db.transaction() + + try { + await expect( + tnx.raw( + 'INSERT INTO storage.objects (bucket_id, name, owner, version) VALUES (?, ?, ?, ?)', + [testBucketName, invalidKey, null, randomUUID()] + ) + ).rejects.toMatchObject>({ + code: '23514', + constraint: 'objects_name_check', + }) + } finally { + await tnx.rollback() + } + }) + + it('rejects invalid multipart upload keys at the storage.s3_multipart_uploads constraint', async () => { + const db = tHelper.database.connection.pool.acquire() + const tnx = await db.transaction() + + try { + await expect( + tnx.raw( + `INSERT INTO storage.s3_multipart_uploads + (id, in_progress_size, upload_signature, bucket_id, key, version, owner_id) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + [randomUUID(), 0, 'sig', testBucketName, invalidKey, randomUUID(), null] + ) + ).rejects.toMatchObject>({ + code: '23514', + constraint: 's3_multipart_uploads_key_check', + }) + } finally { + await tnx.rollback() + } + }) + + it('rejects invalid multipart part keys at the storage.s3_multipart_uploads_parts constraint', async () => { + const db = tHelper.database.connection.pool.acquire() + const tnx = await db.transaction() + const uploadId = randomUUID() + + try { + await tnx.raw( + `INSERT INTO storage.s3_multipart_uploads + (id, in_progress_size, upload_signature, bucket_id, key, version, owner_id) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + [uploadId, 0, 'sig', testBucketName, `valid-${randomUUID()}.txt`, randomUUID(), null] + ) + + await expect( + tnx.raw( + `INSERT INTO storage.s3_multipart_uploads_parts + (upload_id, size, part_number, bucket_id, key, etag, owner_id, version) + VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, + [uploadId, 1, 1, testBucketName, invalidKey, 'etag', null, randomUUID()] + ) + ).rejects.toMatchObject>({ + code: '23514', + constraint: 's3_multipart_uploads_parts_key_check', + }) + } finally { + await tnx.rollback() + } + }) +}) diff --git a/src/test/unicode-object-name-migration.test.ts b/src/test/unicode-object-name-migration.test.ts index 98a744c2f..8795e72e0 100644 --- a/src/test/unicode-object-name-migration.test.ts +++ b/src/test/unicode-object-name-migration.test.ts @@ -13,13 +13,19 @@ describe('unicode object name migration', () => { expect(sql).toContain(`server_encoding = 'SQL_ASCII'`) expect(sql).toContain(String.raw`name !~ E'\\xEF\\xBF\\xBE|\\xEF\\xBF\\xBF'`) + expect(sql).toContain(String.raw`name !~ E'\\xED[\\xA0-\\xBF][\\x80-\\xBF]'`) expect(sql).toContain(String.raw`POSITION(U&'\FFFE' IN name) = 0`) expect(sql).toContain(String.raw`POSITION(U&'\FFFF' IN name) = 0`) expect(sql).toContain('ADD CONSTRAINT objects_name_check') expect(sql).toContain('ADD CONSTRAINT s3_multipart_uploads_key_check') expect(sql).toContain('ADD CONSTRAINT s3_multipart_uploads_parts_key_check') expect(sql).toContain(String.raw`key !~ E'[\\x00-\\x08\\x0B\\x0C\\x0E-\\x1F]'`) + expect(sql).toContain(String.raw`key !~ E'\\xED[\\xA0-\\xBF][\\x80-\\xBF]'`) expect(sql).toContain(String.raw`POSITION(U&'\FFFE' IN key) = 0`) expect(sql).toContain(String.raw`POSITION(U&'\FFFF' IN key) = 0`) + expect(sql).toContain('NOT VALID') + expect(sql).toContain('VALIDATE CONSTRAINT objects_name_check') + expect(sql).toContain('VALIDATE CONSTRAINT s3_multipart_uploads_key_check') + expect(sql).toContain('VALIDATE CONSTRAINT s3_multipart_uploads_parts_key_check') }) }) From bd80e2cff92d2606c85861fd3130946cd5bf1e25 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Tue, 10 Mar 2026 09:38:16 +0100 Subject: [PATCH 37/40] fix: name check constraint as 400 Signed-off-by: ferhat elmas --- src/internal/errors/codes.ts | 8 ++++++++ src/storage/database/knex.ts | 13 +++++++++---- src/test/database-error-mapping.test.ts | 2 +- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/internal/errors/codes.ts b/src/internal/errors/codes.ts index c7d05a4ec..5bab73170 100644 --- a/src/internal/errors/codes.ts +++ b/src/internal/errors/codes.ts @@ -359,6 +359,14 @@ export const ERRORS = { originalError: e, }), + InvalidObjectName: (e?: Error) => + new StorageBackendError({ + code: ErrorCode.InvalidKey, + httpStatusCode: 400, + message: 'Invalid object name', + originalError: e, + }), + InvalidKey: (key: string, e?: Error) => new StorageBackendError({ code: ErrorCode.InvalidKey, diff --git a/src/storage/database/knex.ts b/src/storage/database/knex.ts index 62eeb96a5..88ddfef88 100644 --- a/src/storage/database/knex.ts +++ b/src/storage/database/knex.ts @@ -1151,13 +1151,18 @@ export class DBError extends StorageBackendError implements RenderableError { code: pgError.code, }) default: - const errorMessage = + if ( pgError.code === '23514' && pgError.constraint && objectNameConstraintNames.has(pgError.constraint) - ? 'Invalid object name' - : pgError.message - return ERRORS.DatabaseError(errorMessage, pgError).withMetadata({ + ) { + return ERRORS.InvalidObjectName(pgError).withMetadata({ + query, + code: pgError.code, + }) + } + + return ERRORS.DatabaseError(pgError.message, pgError).withMetadata({ query, code: pgError.code, }) diff --git a/src/test/database-error-mapping.test.ts b/src/test/database-error-mapping.test.ts index 1af043c1f..bf3b57bb3 100644 --- a/src/test/database-error-mapping.test.ts +++ b/src/test/database-error-mapping.test.ts @@ -19,7 +19,7 @@ describe('DBError.fromDBError', () => { const err = DBError.fromDBError(pgError, query) - expect(err.code).toBe(ErrorCode.DatabaseError) + expect(err.code).toBe(ErrorCode.InvalidKey) expect(err.message).toBe('Invalid object name') expect(err.metadata).toMatchObject({ code: '23514', From 136aff1971a0944b214bf22f6136d04afb681a2c Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Tue, 10 Mar 2026 09:39:01 +0100 Subject: [PATCH 38/40] fix: copy source with query param Signed-off-by: ferhat elmas --- .../protocols/s3/copy-source-parser.ts | 40 +++++-- src/test/s3-copy-source-parser.test.ts | 21 ++++ src/test/s3-protocol.test.ts | 100 ++++++++++++++++++ 3 files changed, 152 insertions(+), 9 deletions(-) diff --git a/src/storage/protocols/s3/copy-source-parser.ts b/src/storage/protocols/s3/copy-source-parser.ts index 1adee700a..dd847753f 100644 --- a/src/storage/protocols/s3/copy-source-parser.ts +++ b/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, diff --git a/src/test/s3-copy-source-parser.test.ts b/src/test/s3-copy-source-parser.test.ts index e7dfc8e45..3adbea07c 100644 --- a/src/test/s3-copy-source-parser.test.ts +++ b/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>({ diff --git a/src/test/s3-protocol.test.ts b/src/test/s3-protocol.test.ts index e59466799..b1f97654c 100644 --- a/src/test/s3-protocol.test.ts +++ b/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() From 2564e3f13856e6051160bee8f1aadb575de3c6f0 Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Tue, 10 Mar 2026 10:02:59 +0100 Subject: [PATCH 39/40] fix: path traversal with relaxed key in file backend Signed-off-by: ferhat elmas --- src/storage/backend/file.ts | 115 +++++++++++++++++++--------------- src/test/file-backend.test.ts | 24 +++++++ 2 files changed, 89 insertions(+), 50 deletions(-) diff --git a/src/storage/backend/file.ts b/src/storage/backend/file.ts index f2754b1e3..b3a98fd6c 100644 --- a/src/storage/backend/file.ts +++ b/src/storage/backend/file.ts @@ -342,18 +342,17 @@ export class FileBackend implements StorageBackendAdapter { cacheControl: string ): Promise { const uploadId = randomUUID() - const multiPartFolder = this.resolveSecurePath( - path.join('multiparts', uploadId, bucketName, withOptionalVersion(key, version)) - ) - const multipartFile = this.resolveSecurePath( - path.join( - 'multiparts', - uploadId, - bucketName, - withOptionalVersion(key, version), - 'metadata.json' - ) - ) + const multiPartFolder = this.resolveSecureMultipartPath(uploadId, { + bucketName, + key, + version, + }) + const multipartFile = this.resolveSecureMultipartPath(uploadId, { + bucketName, + key, + version, + suffix: 'metadata.json', + }) await fsExtra.ensureDir(multiPartFolder) await fsExtra.writeFile(multipartFile, JSON.stringify({ contentType, cacheControl })) @@ -368,15 +367,12 @@ export class FileBackend implements StorageBackendAdapter { partNumber: number, body: stream.Readable ): Promise<{ ETag?: string }> { - const partPath = this.resolveSecurePath( - path.join( - 'multiparts', - uploadId, - bucketName, - withOptionalVersion(key, version), - `part-${partNumber}` - ) - ) + const partPath = this.resolveSecureMultipartPath(uploadId, { + bucketName, + key, + version, + suffix: `part-${partNumber}`, + }) const writeStream = fsExtra.createWriteStream(partPath) @@ -404,15 +400,12 @@ export class FileBackend implements StorageBackendAdapter { } > { const partsByEtags = parts.map(async (part) => { - const partFilePath = this.resolveSecurePath( - path.join( - 'multiparts', - uploadId, - bucketName, - withOptionalVersion(key, version), - `part-${part.PartNumber}` - ) - ) + const partFilePath = this.resolveSecureMultipartPath(uploadId, { + bucketName, + key, + version, + suffix: `part-${part.PartNumber}`, + }) const partExists = await fsExtra.pathExists(partFilePath) if (partExists) { @@ -432,15 +425,12 @@ export class FileBackend implements StorageBackendAdapter { const multipartStream = this.mergePartStreams(finalParts) const metadataContent = await fsExtra.readFile( - this.resolveSecurePath( - path.join( - 'multiparts', - uploadId, - bucketName, - withOptionalVersion(key, version), - 'metadata.json' - ) - ), + this.resolveSecureMultipartPath(uploadId, { + bucketName, + key, + version, + suffix: 'metadata.json', + }), 'utf-8' ) @@ -455,7 +445,7 @@ export class FileBackend implements StorageBackendAdapter { metadata.cacheControl ) - fsExtra.remove(this.resolveSecurePath(path.join('multiparts', uploadId))).catch(() => { + fsExtra.remove(this.resolveSecureMultipartPath(uploadId)).catch(() => { // no-op }) @@ -473,7 +463,7 @@ export class FileBackend implements StorageBackendAdapter { uploadId: string, version?: string ): Promise { - const multiPartFolder = this.resolveSecurePath(path.join('multiparts', uploadId)) + const multiPartFolder = this.resolveSecureMultipartPath(uploadId) await fsExtra.remove(multiPartFolder) @@ -495,15 +485,12 @@ export class FileBackend implements StorageBackendAdapter { sourceVersion?: string, rangeBytes?: { fromByte: number; toByte: number } ): Promise<{ eTag?: string; lastModified?: Date }> { - const partFilePath = this.resolveSecurePath( - path.join( - 'multiparts', - UploadId, - storageS3Bucket, - withOptionalVersion(key, version), - `part-${PartNumber}` - ) - ) + const partFilePath = this.resolveSecureMultipartPath(UploadId, { + bucketName: storageS3Bucket, + key, + version, + suffix: `part-${PartNumber}`, + }) const sourceFilePath = this.resolveSecurePath( `${storageS3Bucket}/${withOptionalVersion(sourceKey, sourceVersion)}` ) @@ -692,6 +679,34 @@ export class FileBackend implements StorageBackendAdapter { return normalizedPath } + private resolveSecureMultipartPath( + uploadId: string, + options?: { + bucketName?: string + key?: string + version?: string + suffix?: string + } + ): string { + // Intentionally avoid path.join for attacker-controlled segments so dot segments remain visible + // to resolveSecurePath instead of being normalized away beforehand. + let relativePath = `multiparts/${uploadId}` + + if (typeof options?.bucketName === 'string') { + relativePath += `/${options.bucketName}` + } + + if (typeof options?.key === 'string') { + relativePath += `/${withOptionalVersion(options.key, options.version)}` + } + + if (typeof options?.suffix === 'string') { + relativePath += `/${options.suffix}` + } + + return this.resolveSecurePath(relativePath) + } + private async etag(file: string, stats: fs.Stats): Promise { if (this.etagAlgorithm === 'md5') { const checksum = await this.computeMd5(file) diff --git a/src/test/file-backend.test.ts b/src/test/file-backend.test.ts index d3f7586a2..ca5ed1441 100644 --- a/src/test/file-backend.test.ts +++ b/src/test/file-backend.test.ts @@ -314,6 +314,22 @@ describe('FileBackend traversal protection', () => { }) }) + it('rejects multipart keys that only rebase within the storage root', async () => { + const traversalKey = '../multipart-rebased.txt' + + await expect( + backend.createMultiPartUpload('bucket', traversalKey, 'v1', 'text/plain', 'no-cache') + ).rejects.toMatchObject({ + code: 'InvalidKey', + }) + + await expect( + backend.uploadPart('bucket', traversalKey, 'v1', 'upload-id', 1, Readable.from('escape-part')) + ).rejects.toMatchObject({ + code: 'InvalidKey', + }) + }) + it('rejects traversal key in object operations with InvalidKey', async () => { const traversalKey = `${'../'.repeat(20)}tmp/${escapePrefix}/object-escape.txt` @@ -408,6 +424,14 @@ describe('FileBackend traversal protection', () => { code: 'InvalidKey', }) }) + + it('rejects multipart upload ids that would be normalized to sibling in-root paths', async () => { + await expect( + backend.abortMultipartUpload('bucket', 'key', '../upload-id') + ).rejects.toMatchObject({ + code: 'InvalidKey', + }) + }) }) describe('FileBackend lastModified', () => { From 3d56e91adc5097201cf52fbaf9fc2eacefd4fcfa Mon Sep 17 00:00:00 2001 From: ferhat elmas Date: Wed, 11 Mar 2026 19:50:20 +0100 Subject: [PATCH 40/40] fix: address review comments Signed-off-by: ferhat elmas --- migrations/tenant/57-unicode-object-names.sql | 3 + src/http/plugins/xml.ts | 69 ++++++----- src/http/routes/object/getSignedObject.ts | 2 +- src/http/routes/object/uploadSignedObject.ts | 8 +- src/http/routes/render/renderSignedImage.ts | 2 +- src/http/routes/signed-url.ts | 25 ---- src/internal/errors/codes.ts | 41 +------ src/internal/http/index.ts | 1 + src/internal/http/url.ts | 69 +++++++++++ src/storage/backend/s3/adapter.ts | 7 +- src/storage/backend/s3/backup.ts | 6 +- src/storage/backend/s3/copy-source.ts | 5 - src/storage/object.ts | 18 +-- src/storage/path-encoding.ts | 10 -- src/storage/protocols/s3/s3-handler.ts | 2 +- src/test/signed-url-route.test.ts | 2 +- src/test/xml-parser-plugin.test.ts | 107 ++++++++++++++++++ src/test/xml-plugin.test.ts | 31 ----- 18 files changed, 237 insertions(+), 171 deletions(-) delete mode 100644 src/http/routes/signed-url.ts create mode 100644 src/internal/http/url.ts delete mode 100644 src/storage/backend/s3/copy-source.ts delete mode 100644 src/storage/path-encoding.ts delete mode 100644 src/test/xml-plugin.test.ts diff --git a/migrations/tenant/57-unicode-object-names.sql b/migrations/tenant/57-unicode-object-names.sql index 5254375e8..3530f60fb 100644 --- a/migrations/tenant/57-unicode-object-names.sql +++ b/migrations/tenant/57-unicode-object-names.sql @@ -1,5 +1,8 @@ DO $$ DECLARE + -- SQL_ASCII databases do not have reliable Unicode code point semantics, so + -- the migration needs a byte-pattern branch instead of U&'' character checks. + -- This is the case for OrioleDB and/or --locale=C databases. server_encoding text := current_setting('server_encoding'); BEGIN IF NOT EXISTS ( diff --git a/src/http/plugins/xml.ts b/src/http/plugins/xml.ts index 480b037b5..257f2e77d 100644 --- a/src/http/plugins/xml.ts +++ b/src/http/plugins/xml.ts @@ -6,34 +6,36 @@ import xml from 'xml2js' type XmlParserOptions = { disableContentParser?: boolean; parseAsArray?: string[] } type RequestError = Error & { statusCode?: number } -export function decodeXmlNumericEntities(value: string): string { - const isValidXmlCodePoint = (codePoint: number) => { - if (!Number.isInteger(codePoint) || codePoint < 0 || codePoint > 0x10ffff) { - return false - } - - return ( - codePoint === 0x9 || - codePoint === 0xa || - codePoint === 0xd || - (codePoint >= 0x20 && codePoint <= 0xd7ff) || - (codePoint >= 0xe000 && codePoint <= 0xfffd) || - (codePoint >= 0x10000 && codePoint <= 0x10ffff) - ) +function isValidXmlCodePoint(codePoint: number): boolean { + if (!Number.isInteger(codePoint) || codePoint < 0 || codePoint > 0x10ffff) { + return false } - return value.replace( - /&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g, - (match: string, rawValue: string) => { - const isHex = rawValue[0].toLowerCase() === 'x' - const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) - if (!isValidXmlCodePoint(codePoint)) { - return match - } + return ( + codePoint === 0x9 || + codePoint === 0xa || + codePoint === 0xd || + (codePoint >= 0x20 && codePoint <= 0xd7ff) || + (codePoint >= 0xe000 && codePoint <= 0xfffd) || + (codePoint >= 0x10000 && codePoint <= 0x10ffff) + ) +} + +function getInvalidXmlNumericEntity(value: string): string | undefined { + const numericEntityPattern = /&#([xX][0-9a-fA-F]{1,6}|[0-9]{1,7});/g + + let match = numericEntityPattern.exec(value) + while (match) { + const rawValue = match[1] + const isHex = rawValue[0].toLowerCase() === 'x' + const codePoint = Number.parseInt(isHex ? rawValue.slice(1) : rawValue, isHex ? 16 : 10) - return String.fromCodePoint(codePoint) + if (!isValidXmlCodePoint(codePoint)) { + return match[0] } - ) + + match = numericEntityPattern.exec(value) + } } function forcePathAsArray(node: unknown, pathSegments: string[]): void { @@ -82,16 +84,23 @@ export const xmlParser = fastifyPlugin( return } + const xmlBody = typeof body === 'string' ? body : body.toString('utf8') + const invalidNumericEntity = getInvalidXmlNumericEntity(xmlBody) + if (invalidNumericEntity) { + const parseError: RequestError = new Error( + `Invalid XML payload: invalid numeric entity ${invalidNumericEntity}` + ) + parseError.statusCode = 400 + done(parseError) + return + } + xml.parseString( - body, + xmlBody, { explicitArray: false, trim: true, - valueProcessors: [ - decodeXmlNumericEntities, - xml.processors.parseNumbers, - xml.processors.parseBooleans, - ], + valueProcessors: [xml.processors.parseNumbers, xml.processors.parseBooleans], }, (err: Error | null, parsed: unknown) => { if (err) { diff --git a/src/http/routes/object/getSignedObject.ts b/src/http/routes/object/getSignedObject.ts index 9f8bde316..3709c41ff 100644 --- a/src/http/routes/object/getSignedObject.ts +++ b/src/http/routes/object/getSignedObject.ts @@ -4,8 +4,8 @@ import { getConfig } from '../../../config' import { SignedToken, verifyJWT } from '../../../internal/auth' import { getJwtSecret } from '../../../internal/database' import { ERRORS } from '../../../internal/errors' +import { doesSignedTokenMatchRequestPath } from '../../../internal/http' import { ROUTE_OPERATIONS } from '../operations' -import { doesSignedTokenMatchRequestPath } from '../signed-url' const { storageS3Bucket } = getConfig() diff --git a/src/http/routes/object/uploadSignedObject.ts b/src/http/routes/object/uploadSignedObject.ts index dd94cfaa4..6d77a8626 100644 --- a/src/http/routes/object/uploadSignedObject.ts +++ b/src/http/routes/object/uploadSignedObject.ts @@ -2,10 +2,10 @@ import fastifyMultipart from '@fastify/multipart' import { SignedUploadToken, verifyJWT } from '@internal/auth' import { getJwtSecret } from '@internal/database' import { ERRORS } from '@internal/errors' +import { doesSignedTokenMatchRequestPath } from '@internal/http' import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' import { ROUTE_OPERATIONS } from '../operations' -import { doesSignedTokenMatchRequestPath } from '../signed-url' const uploadSignedObjectParamsSchema = { type: 'object', @@ -97,16 +97,12 @@ export default async function routes(fastify: FastifyInstance) { throw ERRORS.InvalidJWT(err) } - const { owner, upsert, url, exp } = payload + const { owner, upsert, url } = payload if (!doesSignedTokenMatchRequestPath(request.raw.url, '/object/upload/sign', url)) { throw ERRORS.InvalidSignature() } - if (exp * 1000 < Date.now()) { - throw ERRORS.ExpiredSignature() - } - const { objectMetadata, path } = await request.storage .asSuperUser() .from(bucketName) diff --git a/src/http/routes/render/renderSignedImage.ts b/src/http/routes/render/renderSignedImage.ts index 29a8b21ac..6b19abb06 100644 --- a/src/http/routes/render/renderSignedImage.ts +++ b/src/http/routes/render/renderSignedImage.ts @@ -1,12 +1,12 @@ import { SignedToken, verifyJWT } from '@internal/auth' import { getJwtSecret, getTenantConfig } from '@internal/database' import { ERRORS } from '@internal/errors' +import { doesSignedTokenMatchRequestPath } from '@internal/http' import { ImageRenderer } from '@storage/renderer' import { FastifyInstance } from 'fastify' import { FromSchema } from 'json-schema-to-ts' import { getConfig } from '../../../config' import { ROUTE_OPERATIONS } from '../operations' -import { doesSignedTokenMatchRequestPath } from '../signed-url' const { storageS3Bucket, isMultitenant } = getConfig() diff --git a/src/http/routes/signed-url.ts b/src/http/routes/signed-url.ts deleted file mode 100644 index 4f962ddab..000000000 --- a/src/http/routes/signed-url.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { encodePathPreservingSeparators } from '../../storage/path-encoding' - -function stripQueryString(rawUrl: string): string { - const queryIdx = rawUrl.indexOf('?') - return queryIdx === -1 ? rawUrl : rawUrl.slice(0, queryIdx) -} - -function encodeObjectPathForURL(objectPath: string): string { - return encodePathPreservingSeparators(objectPath) -} - -export function doesSignedTokenMatchRequestPath( - rawUrl: string | undefined, - routePrefix: string, - signedObjectPath: string -): boolean { - if (!rawUrl) { - return false - } - - // Verify against the raw URL path to avoid implicit dependencies on framework param decoding. - const pathname = stripQueryString(rawUrl) - const expectedPath = `${routePrefix}/${encodeObjectPathForURL(signedObjectPath)}` - return pathname === expectedPath -} diff --git a/src/internal/errors/codes.ts b/src/internal/errors/codes.ts index 5bab73170..69f759d41 100644 --- a/src/internal/errors/codes.ts +++ b/src/internal/errors/codes.ts @@ -1,45 +1,6 @@ +import { safeEncodeURIComponent } from '../http' import { StorageBackendError } from './storage-error' -function toWellFormedString(value: string): string { - const maybeToWellFormed = (value as unknown as { toWellFormed?: () => string }).toWellFormed - if (typeof maybeToWellFormed === 'function') { - return maybeToWellFormed.call(value) - } - - let normalized = '' - for (let i = 0; i < value.length; i++) { - const currentCodeUnit = value.charCodeAt(i) - - if (currentCodeUnit >= 0xd800 && currentCodeUnit <= 0xdbff) { - const nextCodeUnit = value.charCodeAt(i + 1) - if (i + 1 < value.length && nextCodeUnit >= 0xdc00 && nextCodeUnit <= 0xdfff) { - normalized += value[i] + value[i + 1] - i += 1 - } else { - normalized += '\uFFFD' - } - continue - } - - if (currentCodeUnit >= 0xdc00 && currentCodeUnit <= 0xdfff) { - normalized += '\uFFFD' - continue - } - - normalized += value[i] - } - - return normalized -} - -function safeEncodeURIComponent(value: string): string { - try { - return encodeURIComponent(value) - } catch { - return encodeURIComponent(toWellFormedString(value)) - } -} - export enum ErrorCode { NoSuchBucket = 'NoSuchBucket', NoSuchKey = 'NoSuchKey', diff --git a/src/internal/http/index.ts b/src/internal/http/index.ts index c9209a598..50cde1ce9 100644 --- a/src/internal/http/index.ts +++ b/src/internal/http/index.ts @@ -1 +1,2 @@ export * from './agent' +export * from './url' diff --git a/src/internal/http/url.ts b/src/internal/http/url.ts new file mode 100644 index 000000000..1a7f0bc7b --- /dev/null +++ b/src/internal/http/url.ts @@ -0,0 +1,69 @@ +function toWellFormedString(value: string): string { + const maybeToWellFormed = (value as unknown as { toWellFormed?: () => string }).toWellFormed + if (typeof maybeToWellFormed === 'function') { + return maybeToWellFormed.call(value) + } + + let normalized = '' + for (let i = 0; i < value.length; i++) { + const currentCodeUnit = value.charCodeAt(i) + + if (currentCodeUnit >= 0xd800 && currentCodeUnit <= 0xdbff) { + const nextCodeUnit = value.charCodeAt(i + 1) + if (i + 1 < value.length && nextCodeUnit >= 0xdc00 && nextCodeUnit <= 0xdfff) { + normalized += value[i] + value[i + 1] + i += 1 + } else { + normalized += '\uFFFD' + } + continue + } + + if (currentCodeUnit >= 0xdc00 && currentCodeUnit <= 0xdfff) { + normalized += '\uFFFD' + continue + } + + normalized += value[i] + } + + return normalized +} + +export function safeEncodeURIComponent(value: string): string { + try { + return encodeURIComponent(value) + } catch { + return encodeURIComponent(toWellFormedString(value)) + } +} + +export function encodePathPreservingSeparators(path: string): string { + return path + .split('/') + .map((pathToken) => safeEncodeURIComponent(pathToken)) + .join('/') +} + +export function encodeBucketAndObjectPath(bucket: string, key: string): string { + return `${safeEncodeURIComponent(bucket)}/${encodePathPreservingSeparators(key)}` +} + +function stripQueryString(rawUrl: string): string { + const queryIdx = rawUrl.indexOf('?') + return queryIdx === -1 ? rawUrl : rawUrl.slice(0, queryIdx) +} + +export function doesSignedTokenMatchRequestPath( + rawUrl: string | undefined, + routePrefix: string, + signedObjectPath: string +): boolean { + if (!rawUrl) { + return false + } + + const pathname = stripQueryString(rawUrl) + const expectedPath = `${routePrefix}/${encodePathPreservingSeparators(signedObjectPath)}` + return pathname === expectedPath +} diff --git a/src/storage/backend/s3/adapter.ts b/src/storage/backend/s3/adapter.ts index 7e75e6d65..c85ed762f 100644 --- a/src/storage/backend/s3/adapter.ts +++ b/src/storage/backend/s3/adapter.ts @@ -19,7 +19,7 @@ import { import { Progress, Upload } from '@aws-sdk/lib-storage' import { getSignedUrl } from '@aws-sdk/s3-request-presigner' import { ERRORS, StorageBackendError } from '@internal/errors' -import { createAgent, InstrumentedAgent } from '@internal/http' +import { createAgent, encodeBucketAndObjectPath, InstrumentedAgent } from '@internal/http' import { monitorStream } from '@internal/streams' import { NodeHttpHandler } from '@smithy/node-http-handler' import { BackupObjectInfo, ObjectBackup } from '@storage/backend/s3/backup' @@ -32,7 +32,6 @@ import { UploadPart, withOptionalVersion, } from './../adapter' -import { encodeCopySource } from './copy-source' const { storageS3UploadQueueSize, @@ -256,7 +255,7 @@ export class S3Backend implements StorageBackendAdapter { try { const command = new CopyObjectCommand({ Bucket: bucket, - CopySource: encodeCopySource(bucket, withOptionalVersion(source, version)), + CopySource: encodeBucketAndObjectPath(bucket, withOptionalVersion(source, version)), Key: withOptionalVersion(destination, destinationVersion), CopySourceIfMatch: conditions?.ifMatch, CopySourceIfNoneMatch: conditions?.ifNoneMatch, @@ -569,7 +568,7 @@ export class S3Backend implements StorageBackendAdapter { Key: withOptionalVersion(key, version), UploadId, PartNumber, - CopySource: encodeCopySource( + CopySource: encodeBucketAndObjectPath( storageS3Bucket, withOptionalVersion(sourceKey, sourceKeyVersion) ), diff --git a/src/storage/backend/s3/backup.ts b/src/storage/backend/s3/backup.ts index f165ed0eb..255b14f71 100644 --- a/src/storage/backend/s3/backup.ts +++ b/src/storage/backend/s3/backup.ts @@ -7,7 +7,7 @@ import { S3Client, UploadPartCopyCommand, } from '@aws-sdk/client-s3' -import { encodeCopySource } from './copy-source' +import { encodeBucketAndObjectPath } from '@internal/http' const FIVE_GB = 5 * 1024 * 1024 * 1024 @@ -70,7 +70,7 @@ export class ObjectBackup { const copyParams = { Bucket: destinationBucket, Key: destinationKey, - CopySource: encodeCopySource(sourceBucket, sourceKey), + CopySource: encodeBucketAndObjectPath(sourceBucket, sourceKey), } const copyCommand = new CopyObjectCommand(copyParams) @@ -158,7 +158,7 @@ export class ObjectBackup { Key: destinationKey, PartNumber: partNumber, UploadId: uploadId, - CopySource: encodeCopySource(sourceBucket, sourceKey), + CopySource: encodeBucketAndObjectPath(sourceBucket, sourceKey), CopySourceRange: `bytes=${start}-${end}`, }) diff --git a/src/storage/backend/s3/copy-source.ts b/src/storage/backend/s3/copy-source.ts deleted file mode 100644 index 27dce3703..000000000 --- a/src/storage/backend/s3/copy-source.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { encodeBucketAndObjectPath } from '../../path-encoding' - -export function encodeCopySource(bucket: string, key: string): string { - return encodeBucketAndObjectPath(bucket, key) -} diff --git a/src/storage/object.ts b/src/storage/object.ts index 8a9864a9e..af26583b1 100644 --- a/src/storage/object.ts +++ b/src/storage/object.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'node:crypto' import { SignedUploadToken, signJWT, verifyJWT } from '@internal/auth' import { getJwtSecret } from '@internal/database' import { ERRORS } from '@internal/errors' +import { encodeBucketAndObjectPath, encodePathPreservingSeparators } from '@internal/http' import { StorageObjectLocator } from '@storage/locator' import { Obj } from '@storage/schemas' import { FastifyRequest } from 'fastify/types/request' @@ -17,7 +18,6 @@ import { ObjectUpdatedMetadata, } from './events' import { mustBeValidKey } from './limits' -import { encodeBucketAndObjectPath, encodePathPreservingSeparators } from './path-encoding' import { fileUploadFromRequest, Uploader, UploadRequest } from './uploader' const { requestUrlLengthLimit } = getConfig() @@ -50,10 +50,6 @@ export interface ListObjectsV2Result { nextCursorKey?: string } -function encodeObjectPathForURL(bucketId: string, objectName: string): string { - return encodeBucketAndObjectPath(bucketId, objectName) -} - /** * ObjectStorage * interact with remote objects and database state @@ -749,7 +745,7 @@ export class ObjectStorage { urlPath = 'render/image' } - const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, objectName) + const encodedUrlToSign = encodeBucketAndObjectPath(this.bucketId, objectName) // @todo parse the url properly return `/${urlPath}/sign/${encodedUrlToSign}?token=${token}` @@ -788,7 +784,7 @@ export class ObjectStorage { if (nameSet.has(path)) { const urlToSign = `${this.bucketId}/${path}` const token = await signJWT({ url: urlToSign }, urlSigningKey, expiresIn) - const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, path) + const encodedUrlToSign = encodeBucketAndObjectPath(this.bucketId, path) signedURL = `/object/sign/${encodedUrlToSign}?token=${token}` } else { error = 'Either the object does not exist or you do not have access to it' @@ -834,7 +830,7 @@ export class ObjectStorage { expiresIn ) - const encodedUrlToSign = encodeObjectPathForURL(this.bucketId, objectName) + const encodedUrlToSign = encodeBucketAndObjectPath(this.bucketId, objectName) return { url: `/object/upload/sign/${encodedUrlToSign}?token=${token}`, token } } @@ -855,16 +851,12 @@ export class ObjectStorage { throw ERRORS.InvalidJWT(err) } - const { url, exp } = payload + const { url } = payload if (url !== `${this.bucketId}/${objectName}`) { throw ERRORS.InvalidSignature() } - if (exp * 1000 < Date.now()) { - throw ERRORS.ExpiredSignature() - } - return payload } } diff --git a/src/storage/path-encoding.ts b/src/storage/path-encoding.ts deleted file mode 100644 index 8ebf89331..000000000 --- a/src/storage/path-encoding.ts +++ /dev/null @@ -1,10 +0,0 @@ -export function encodePathPreservingSeparators(path: string): string { - return path - .split('/') - .map((pathToken) => encodeURIComponent(pathToken)) - .join('/') -} - -export function encodeBucketAndObjectPath(bucket: string, key: string): string { - return `${encodeURIComponent(bucket)}/${encodePathPreservingSeparators(key)}` -} diff --git a/src/storage/protocols/s3/s3-handler.ts b/src/storage/protocols/s3/s3-handler.ts index 83f5d6ef0..37eea93bf 100644 --- a/src/storage/protocols/s3/s3-handler.ts +++ b/src/storage/protocols/s3/s3-handler.ts @@ -19,12 +19,12 @@ import { } from '@aws-sdk/client-s3' import { decrypt, encrypt } from '@internal/auth' import { ERRORS } from '@internal/errors' +import { encodePathPreservingSeparators } from '@internal/http' import { logger, logSchema } from '@internal/monitoring' import { PassThrough, Readable } from 'stream' import stream from 'stream/promises' import { getConfig } from '../../../config' import { getFileSizeLimit, mustBeValidBucketName, mustBeValidKey } from '../../limits' -import { encodePathPreservingSeparators } from '../../path-encoding' import { S3MultipartUpload } from '../../schemas' import { Storage } from '../../storage' import { Uploader, validateMimeType } from '../../uploader' diff --git a/src/test/signed-url-route.test.ts b/src/test/signed-url-route.test.ts index 76a3d43ca..aa6b9e80c 100644 --- a/src/test/signed-url-route.test.ts +++ b/src/test/signed-url-route.test.ts @@ -1,4 +1,4 @@ -import { doesSignedTokenMatchRequestPath } from '../http/routes/signed-url' +import { doesSignedTokenMatchRequestPath } from '@internal/http' import { encodePathPreservingSeparatorsForTest } from './utils/path-encoding' describe('signed URL route path verification', () => { diff --git a/src/test/xml-parser-plugin.test.ts b/src/test/xml-parser-plugin.test.ts index 6bb014dad..ee06f8e6a 100644 --- a/src/test/xml-parser-plugin.test.ts +++ b/src/test/xml-parser-plugin.test.ts @@ -50,6 +50,62 @@ describe('xmlParser plugin', () => { } }) + it('accepts valid numeric entities and still applies value processors', async () => { + const app = await buildXmlApp(['CompleteMultipartUpload.Part']) + + try { + const response = await app.inject({ + method: 'POST', + url: '/xml', + headers: { + 'content-type': 'application/xml', + accept: 'application/json', + }, + payload: + '1🙂', + }) + + expect(response.statusCode).toBe(200) + expect(response.json()).toEqual({ + body: { + CompleteMultipartUpload: { + Part: [{ PartNumber: 1, ETag: '🙂' }], + }, + }, + }) + } finally { + await app.close() + } + }) + + it('accepts decimal astral numeric entities', async () => { + const app = await buildXmlApp(['CompleteMultipartUpload.Part']) + + try { + const response = await app.inject({ + method: 'POST', + url: '/xml', + headers: { + 'content-type': 'application/xml', + accept: 'application/json', + }, + payload: + '🙂', + }) + + expect(response.statusCode).toBe(200) + expect(response.json()).toEqual({ + body: { + CompleteMultipartUpload: { + Part: [{ ETag: '🙂' }], + }, + }, + }) + } finally { + await app.close() + } + }) + it('returns 400 for malformed XML payloads', async () => { const app = await buildXmlApp() @@ -71,6 +127,57 @@ describe('xmlParser plugin', () => { } }) + it.each([ + '�', + '�', + '�', + '�', + '�', + '￿', + '￿', + ])('returns 400 for XML-forbidden numeric entity %s', async (entity) => { + const app = await buildXmlApp() + + try { + const response = await app.inject({ + method: 'POST', + url: '/xml', + headers: { + 'content-type': 'application/xml', + accept: 'application/json', + }, + payload: `${entity}`, + }) + + expect(response.statusCode).toBe(400) + expect(response.json().message).toContain('Invalid XML payload') + } finally { + await app.close() + } + }) + + it('returns 400 for out-of-range numeric entities', async () => { + const app = await buildXmlApp() + + try { + const response = await app.inject({ + method: 'POST', + url: '/xml', + headers: { + 'content-type': 'application/xml', + accept: 'application/json', + }, + payload: + '�', + }) + + expect(response.statusCode).toBe(400) + expect(response.json().message).toContain('Invalid XML payload') + } finally { + await app.close() + } + }) + it('serializes response payloads as XML when requested', async () => { const app = await buildXmlApp() diff --git a/src/test/xml-plugin.test.ts b/src/test/xml-plugin.test.ts deleted file mode 100644 index 79cd24e3e..000000000 --- a/src/test/xml-plugin.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { decodeXmlNumericEntities } from '../http/plugins/xml' - -describe('decodeXmlNumericEntities', () => { - test('decodes hexadecimal entities including astral code points', () => { - expect(decodeXmlNumericEntities('a🙂b')).toBe('a🙂b') - }) - - test('decodes decimal entities', () => { - expect(decodeXmlNumericEntities('a🙂b')).toBe('a🙂b') - }) - - test('keeps out-of-range entities unchanged', () => { - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - }) - - test('keeps NUL entities unchanged', () => { - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - }) - - test('keeps surrogate-half entities unchanged', () => { - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - expect(decodeXmlNumericEntities('a�b')).toBe('a�b') - }) - - test('keeps noncharacter entities unchanged', () => { - expect(decodeXmlNumericEntities('a￿b')).toBe('a￿b') - expect(decodeXmlNumericEntities('a￿b')).toBe('a￿b') - }) -})