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

Author: mlatief

migrations/tenant/0026-unicode-object-names.sql

@@ -0,0 +1,4 @@
+ALTER TABLE "storage"."objects"
+    ADD CONSTRAINT objects_name_check
+    CHECK (name SIMILAR TO '[\x09\x0A\x0D\x20-\xD7FF\xE000-\xFFFD\x00010000-\x0010ffff]+');
+

src/http/plugins/xml.ts

@@ -21,6 +21,10 @@ export const xmlParser = fastifyPlugin(
         isArray: (_: string, jpath: string) => {
           return opts.parseAsArray?.includes(jpath)
         },
+        tagValueProcessor: (name: string, value: string) =>
+          value.replace(/&#x([0-9a-fA-F]{1,6});/g, (_, str: string) =>
+            String.fromCharCode(Number.parseInt(str, 16))
+          ),
       })
     }
 

src/internal/database/migrations/types.ts

@@ -24,4 +24,5 @@ export const DBMigration = {
   'optimize-search-function': 22,
   'operation-function': 23,
   'custom-metadata': 24,
+  'unicode-object-names': 25,
 } as const

src/internal/errors/codes.ts

@@ -265,7 +265,7 @@ export const ERRORS = {
       code: ErrorCode.InvalidKey,
       resource: key,
       httpStatusCode: 400,
-      message: `Invalid key: ${key}`,
+      message: `Invalid key: ${encodeURIComponent(key)}`,
       originalError: e,
     }),
 

src/storage/database/knex.ts

@@ -877,7 +877,11 @@ export class DBError extends StorageBackendError implements RenderableError {
           code: pgError.code,
         })
       default:
-        return ERRORS.DatabaseError(pgError.message, 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,
         })

src/storage/limits.ts

@@ -47,9 +47,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)
 }
 
 /**

src/storage/protocols/s3/s3-handler.ts

@@ -506,6 +506,9 @@ export class S3ProtocolHandler {
       throw ERRORS.InvalidUploadId()
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     await uploader.canUpload({
       bucketId: Bucket as string,
       objectName: Key as string,
@@ -601,6 +604,9 @@ export class S3ProtocolHandler {
       throw ERRORS.MissingContentLength()
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     const bucket = await this.storage.asSuperUser().findBucket(Bucket, 'file_size_limit')
     const maxFileSize = await getFileSizeLimit(this.storage.db.tenantId, bucket?.file_size_limit)
 
@@ -755,6 +761,9 @@ export class S3ProtocolHandler {
       throw ERRORS.MissingParameter('Key')
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     const multipart = await this.storage.db
       .asSuperUser()
       .findMultipartUpload(UploadId, 'id,version')
@@ -797,6 +806,9 @@ export class S3ProtocolHandler {
       throw ERRORS.MissingParameter('Bucket')
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     const object = await this.storage
       .from(Bucket)
       .findObject(Key, 'metadata,user_metadata,created_at,updated_at')
@@ -836,6 +848,9 @@ export class S3ProtocolHandler {
       throw ERRORS.MissingParameter('Key')
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     const object = await this.storage.from(Bucket).findObject(Key, 'id')
 
     if (!object) {
@@ -864,6 +879,9 @@ export class S3ProtocolHandler {
     const bucket = command.Bucket as string
     const key = command.Key as string
 
+    mustBeValidBucketName(bucket)
+    mustBeValidKey(key)
+
     const object = await this.storage.from(bucket).findObject(key, 'version,user_metadata')
     const response = await this.storage.backend.getObject(
       storageS3Bucket,
@@ -916,6 +934,9 @@ export class S3ProtocolHandler {
       throw ERRORS.MissingParameter('Key')
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+
     await this.storage.from(Bucket).deleteObject(Key)
 
     return {}
@@ -947,6 +968,9 @@ export class S3ProtocolHandler {
       return {}
     }
 
+    mustBeValidBucketName(Bucket)
+    Delete.Objects.forEach((o) => mustBeValidKey(o.Key))
+
     const deletedResult = await this.storage
       .from(Bucket)
       .deleteObjects(Delete.Objects.map((o) => o.Key || ''))
@@ -1017,6 +1041,11 @@ export class S3ProtocolHandler {
       command.MetadataDirective = 'COPY'
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+    mustBeValidBucketName(sourceBucket)
+    mustBeValidKey(sourceKey)
+
     const copyResult = await this.storage.from(sourceBucket).copyObject({
       sourceKey,
       destinationBucket: Bucket,
@@ -1147,6 +1176,11 @@ export class S3ProtocolHandler {
       throw ERRORS.NoSuchKey('')
     }
 
+    mustBeValidBucketName(Bucket)
+    mustBeValidKey(Key)
+    mustBeValidBucketName(sourceBucketName)
+    mustBeValidKey(sourceKey)
+
     // Check if copy source exists
     const copySource = await this.storage.db.findObject(
       sourceBucketName,

src/test/common.ts

@@ -8,6 +8,42 @@ export const adminApp = app({})
 
 const ENV = process.env
 
+/**
+ * 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(() => {

src/test/object.test.ts

@@ -6,10 +6,11 @@ import app from '../app'
 import { getConfig, mergeConfig } from '../config'
 import { signJWT } from '@internal/auth'
 import { Obj, backends } from '../storage'
-import { useMockObject, useMockQueue } from './common'
+import { getInvalidObjectName, getUnicodeObjectName, useMockObject, useMockQueue } from './common'
 import { getServiceKeyUser, getPostgresConnection } from '@internal/database'
 import { Knex } from 'knex'
 import { ErrorCode, StorageBackendError } from '@internal/errors'
+import { randomUUID } from 'crypto'
 
 const { jwtSecret, serviceKey, tenantId } = getConfig()
 const anonKey = process.env.ANON_KEY || ''
@@ -2456,3 +2457,90 @@ describe('testing list objects', () => {
     expect(responseJSON[1].name).toBe('sadcat-upload23.png')
   })
 })
+
+describe('Object key with Unicode characters', () => {
+  test('can upload, get and list', async () => {
+    const prefix = `test-utf8-${randomUUID()}`
+    const objectName = getUnicodeObjectName()
+
+    const form = new FormData()
+    form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
+    const headers = Object.assign({}, form.getHeaders(), {
+      authorization: `Bearer ${serviceKey}`,
+      'x-upsert': 'true',
+    })
+
+    const uploadResponse = await app().inject({
+      method: 'POST',
+      url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`,
+      headers: {
+        ...headers,
+        ...form.getHeaders(),
+      },
+      payload: form,
+    })
+    expect(uploadResponse.statusCode).toBe(200)
+
+    const getResponse = await app().inject({
+      method: 'GET',
+      url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`,
+      headers: {
+        authorization: `Bearer ${serviceKey}`,
+      },
+    })
+    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 app().inject({
+      method: 'POST',
+      url: `/object/list/bucket2`,
+      headers: {
+        authorization: `Bearer ${serviceKey}`,
+      },
+      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 app().inject({
+      method: 'DELETE',
+      url: `/object/bucket2/${prefix}/${encodeURIComponent(objectName)}`,
+      headers: {
+        authorization: `Bearer ${serviceKey}`,
+      },
+    })
+    expect(deleteResponse.statusCode).toBe(200)
+  })
+
+  test('should not be allowed to upload objects with invalid names', async () => {
+    const invalidObjectName = getInvalidObjectName()
+    const form = new FormData()
+    form.append('file', fs.createReadStream(`./src/test/assets/sadcat.jpg`))
+    const headers = Object.assign({}, form.getHeaders(), {
+      authorization: `Bearer ${serviceKey}`,
+      'x-upsert': 'true',
+    })
+    const uploadResponse = await app().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)}`,
+      })
+    )
+  })
+})