vendure-ecommerce / vendure

The commerce platform with customization in its DNA.
https://www.vendure.io
Other
5.79k stars 1.03k forks source link

Uploads with the S3 asset storage plugin have mime type of application/octet-stream in S3. #3184

Open jawngee opened 2 weeks ago

jawngee commented 2 weeks ago

Describe the bug Using the S3 asset storage plugin all uploads have a mime type of application/octet-stream in S3. This happens with minio, supabase storage and aws.

The asset table in the database has the correct mime type however.

I'm guessing whatever is doing the uploading to S3 isn't setting the Content-Type of the request correctly.

To Reproduce Steps to reproduce the behavior:

  1. Fresh install of vendure
  2. Configure the asset storage plugin to use S3
  3. Upload an asset
  4. View upload in S3's file browser

Expected behavior Expect the files in S3 to have the correct mime-type

Environment (please complete the following information):

jawngee commented 2 weeks ago

This is my config for vendure:

AssetServerPlugin.init({
            route: 'assets',
            assetUploadDir: path.join(__dirname, '../static/assets'),
            namingStrategy: new DefaultAssetNamingStrategy(),
            storageStrategyFactory: configureS3AssetStorage({
                bucket: process.env.S3_BUCKET!,
                credentials: {
                    accessKeyId: process.env.S3_ACCESS_KEY!,
                    secretAccessKey: process.env.S3_SECRET_KEY!,
                }, // or any other credential provider
                nativeS3Configuration: {
                    region: process.env.S3_REGION!,
                    endpoint: process.env.S3_URL!,
                    forcePathStyle: true,
                },
            }),
            // // For local dev, the correct value for assetUrlPrefix should
            // // be guessed correctly, but for production it will usually need
            // // to be set manually to match your production url.
            // assetUrlPrefix: IS_DEV ? undefined : 'https://www.my-shop.com/assets/',
        }),

and then for minio in docker:

version: '3.4'
services:
  minio:
    image: minio/minio
    container_name: minio
    ports:
      - "9000:9000"
      - "9001:9001"
    environment:
      MINIO_ROOT_USER: admin
      MINIO_ROOT_PASSWORD: password
    volumes:
      - ./minio/data:/data
    command: server /data --console-address ":9001"
jawngee commented 2 weeks ago

In s3-asset-storage-strategy.ts for Minio and Supabase you need to specify the ContentType in the params object. See lines 246-254.

https://github.com/vendure-ecommerce/vendure/blob/478989e70c4c79fda174419387840f7fbc2e3607/packages/asset-server-plugin/src/s3-asset-storage-strategy.ts#L246-L254

The issue is that there doesn't appear to be any way to find the mime type for the file being passed to the writeFile() function as it's only given a filename key and a blob of data to upload.

You could use a package like mime but that package infers the mime type based on file extension. To be more secure you'd need to use a magic-mime based package which would introduce a native dependency (libmagic) as it infers mime type based on the contents of the file. I don't know how big of an issue this actually is.

For the time being, for our own app, we are using the mime package:

npm install mime@3 @types/mime@3

and using a modified version of the writeFile function:

    private async writeFile(fileName: string, data: PutObjectRequest['Body'] | string | Uint8Array | Buffer) {
        const { Upload } = this.libStorage;

        const upload = new Upload({
            client: this.s3Client,
            params: {
                ...this.s3Config.nativeS3UploadConfiguration,
                Bucket: this.s3Config.bucket,
                Key: fileName,
                Body: data,
                ContentType: mime.getType(fileName) ?? 'application/octet-stream',
            },
        });

        return upload.done().then(result => {
            if (!('Key' in result) || !result.Key) {
                Logger.error(`Got undefined Key for ${fileName}`, loggerCtx);
                throw new Error(`Got undefined Key for ${fileName}`);
            }

            return result.Key;
        });
    }

We are now seeing the correct mime types show up which is no longer confusing our processing pipelines!

Thanks!