UTF-8 Filenames break EditUpload component

aus commented 2 months ago

Link to reproduction

No response

Payload Version

3.0.0-beta.47

Node Version

v20.12.2

Next.js Version

15.0.0-rc.0

Describe the Bug

When a file with a UTF-8 filename is selected for upload in the Upload component, the upload fails (at least for S3 storage providers). An ASCII string is passed to EditUpload filename prop.

A common example is the MacOS default screenshot file names. Example:

$ ls | grep Screen | xxd
00000000: 5363 7265 656e 7368 6f74 2032 3032 342d  Screenshot 2024-
00000010: 3036 2d31 3420 6174 2031 322e 3539 2e35  06-14 at 12.59.5
00000020: 36e2 80af 504d 2e70 6e67 0a              6...PM.png.

Reproduction Steps

Setup a upload-enabled collection (I used Supabase as backend)
Take a screenshot with the native MacOS screenshot utility.
Upload the screenshot (or any file with a UTF-8 filename)

Adapters and Plugins

No response

PatrikKozak commented 1 month ago

Hey @aus - I have been unable to reproduce the issue above with uploading a screenshot with the MacOS screenshot utility.

Can you please send over your Uploads config, as well as, an example image file that you are having difficulties uploading?

And can you confirm with me if you are still experiencing this issue on the latest beta version - I.e 3.0.0-beta.60

aus commented 1 month ago

Yes, this happens with 3.0.0-beta.60. Screenshot was produced with Sonoma 14.5 with APFS disk.

You can reproduce the filename via:

cp someimage.png $(echo U2NyZWVuc2hvdCAyMDI0LTA3LTExIGF0IDEuMDguNDXigK9QTS5wbmcK | base64 -d)

Speficially, I think this is the U+202F character breaking things.

s3Storage plugin settings:

plugins: [
    s3Storage({
      collections: {
        media: {
          prefix: 'media',
        }
      },
      bucket: process.env.S3_BUCKET || '',
      config: {
        forcePathStyle: true,
        credentials: {
          accessKeyId: process.env.S3_ACCESS_KEY_ID || '',
          secretAccessKey: process.env.S3_SECRET_ACCESS_KEY || '',
        },
        region: process.env.S3_REGION,
        endpoint: process.env.S3_ENDPOINT,
      },
    }),
  ],

CollectionConfig:

export const Media: CollectionConfig = {
  slug: 'media',
  admin: {
    group: 'Manage',
  },
  access: {
    read: () => true,
  },
  fields: [
    {
      name: 'alt',
      type: 'text',
      required: true,
    },
  ],
  upload: true,
}

Here's the full stack trace:

[13:12:24] ERROR: InvalidKey: Invalid key: media/Screenshot 2024-07-11 at 1.08.45â¯PM.png
    at throwDefaultError (/Users/user/Desktop/my-payload/node_modules/@smithy/smithy-client/dist-cjs/index.js:839:20)
    at /Users/user/Desktop/my-payload/node_modules/@smithy/smithy-client/dist-cjs/index.js:848:5
    at de_CommandError (/Users/user/Desktop/my-payload/node_modules/@aws-sdk/client-s3/dist-cjs/index.js:4741:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /Users/user/Desktop/my-payload/node_modules/@smithy/middleware-serde/dist-cjs/index.js:35:20
    at async /Users/user/Desktop/my-payload/node_modules/@aws-sdk/middleware-signing/dist-cjs/index.js:226:18
    at async /Users/user/Desktop/my-payload/node_modules/@smithy/middleware-retry/dist-cjs/index.js:320:38
    at async /Users/user/Desktop/my-payload/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/index.js:174:18
    at async /Users/user/Desktop/my-payload/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:98:20
    at async /Users/user/Desktop/my-payload/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/index.js:121:14
    at async /Users/user/Desktop/my-payload/node_modules/@aws-sdk/middleware-logger/dist-cjs/index.js:34:22
    at async Object.eval [as handleUpload] (webpack-internal:///(rsc)/./node_modules/@payloadcms/storage-s3/dist/handleUpload.js:18:13)
    at async eval (webpack-internal:///(rsc)/./node_modules/@payloadcms/plugin-cloud-storage/dist/hooks/beforeChange.js:38:21)
    at async Promise.all (index 0)
    at async eval (webpack-internal:///(rsc)/./node_modules/@payloadcms/plugin-cloud-storage/dist/hooks/beforeChange.js:45:17)
    at async eval (webpack-internal:///(rsc)/./node_modules/payload/dist/collections/operations/create.js:121:20)
    at async createOperation (webpack-internal:///(rsc)/./node_modules/payload/dist/collections/operations/create.js:119:9)
    at async Object.create (webpack-internal:///(rsc)/./node_modules/@payloadcms/next/dist/routes/rest/collections/create.js:20:17)
    at async eval (webpack-internal:///(rsc)/./node_modules/@payloadcms/next/dist/routes/rest/index.js:451:35)
    at async /Users/user/Desktop/my-payload/node_modules/next/dist/compiled/next-server/app-route.runtime.dev.js:6:65921
    at async eN.execute (/Users/user/Desktop/my-payload/node_modules/next/dist/compiled/next-server/app-route.runtime.dev.js:6:56289)
    at async eN.handle (/Users/user/Desktop/my-payload/node_modules/next/dist/compiled/next-server/app-route.runtime.dev.js:6:67222)
    at async doRender (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:1426:42)
    at async responseGenerator (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:1672:40)
    at async DevServer.renderToResponseWithComponentsImpl (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:1711:28)
    at async DevServer.renderPageComponent (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:2045:24)
    at async DevServer.renderToResponseImpl (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:2083:32)
    at async DevServer.pipeImpl (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:937:25)
    at async NextNodeServer.handleCatchallRenderRequest (/Users/user/Desktop/my-payload/node_modules/next/dist/server/next-server.js:283:17)
    at async DevServer.handleRequestImpl (/Users/user/Desktop/my-payload/node_modules/next/dist/server/base-server.js:830:17)
    at async /Users/user/Desktop/my-payload/node_modules/next/dist/server/dev/next-dev-server.js:338:20
    at async Span.traceAsyncFn (/Users/user/Desktop/my-payload/node_modules/next/dist/trace/trace.js:157:20)
    at async DevServer.handleRequest (/Users/user/Desktop/my-payload/node_modules/next/dist/server/dev/next-dev-server.js:335:24)
    at async invokeRender (/Users/user/Desktop/my-payload/node_modules/next/dist/server/lib/router-server.js:175:21)
    at async handleRequest (/Users/user/Desktop/my-payload/node_modules/next/dist/server/lib/router-server.js:352:24)
    at async requestHandlerImpl (/Users/user/Desktop/my-payload/node_modules/next/dist/server/lib/router-server.js:376:13)
    at async Server.requestListener (/Users/user/Desktop/my-payload/node_modules/next/dist/server/lib/start-server.js:142:13)
 POST /api/media?depth=0&fallback-locale=null?depth=0&fallback-locale=null 500 in 593ms

aus commented 1 month ago

This maybe unique to supabase and there's a related issue here: https://github.com/supabase/storage/issues/133

However, as an S3 client, should @payloadcms/storage-s3 enforce best practices for S3 keys? Alternatively, how can I change the key before upload?

PatrikKozak commented 1 week ago

Hey @aus - just spoke to the team about this - one way you could get around this is using a beforeChange hook to sanitize the name accordingly.

We will most likely add in our own sanitization in the cloud-storage / s3 plugins etc in the near future so I am going to convert this to a discussion for now.

Let me know if there is anything else!

payloadcms / payload