alireza12prom / multer-minio

A simple storage engine for Multer to upload files on Minio storage.
MIT License
4 stars 2 forks source link

Unexpected behaviour of file.path on files uploaded from Windows #1

Closed mickeymond closed 7 months ago

mickeymond commented 7 months ago

Windows seems to use forward slashes leading to path not creating folders in the bucket like one would expect them to.

Upload Config Below

const upload = multer({
    storage: new MinioStorageEngine(minioClient, 'recipe-api', {
        path: 'images'
    }),
    preservePath: true
});

Console Log of req.file

{
  fieldname: 'image',
  originalname: 'GrowWebDevDiagrams-express_local_upload_using_multer.jpg',
  encoding: '7bit',
  mimetype: 'image/jpeg',
  filename: '9e1cb7b9-b3e9-4cd3-b86b-af2e7a796eaa',
  path: 'images\\9e1cb7b9-b3e9-4cd3-b86b-af2e7a796eaa',
  bucket: 'recipe-api'
}
mickeymond commented 7 months ago

@alireza12prom After forking the code, it was clear after my testing that joining the this.options.path and the objectName on Windows (where the API code is running) is the one creating the mess. I changed the code to below and it works on Windows and hopefully on Unix/Linux

private _getObjectPath(objectName: string) {
    const { path } = this.options;
    // return join(path || '/', objectName);
    return `${path}/${objectName}`;
}

I have tried searching for a way to get path.join to work on Windows like it does on Unix/Linux but to no avail so please advice and I am more than happy to raise a PR.

alireza12prom commented 7 months ago

Hi @mickeymond I reviewed your issue and you're right. I've searched and found something that can help to normalize unix-style paths (whether the OS is Windows or Linux). Also notice that the path is undefined if the client doesn't specify it.

private _getObjectPath(objectName: string) {
    const { path } = this.options;
    return posix.join(`${path || '/'}/${objectName}`);
  }

Please fix it and then make a PR. Thanks for contributing!

mickeymond commented 7 months ago

@alireza12prom Thank you very much for sharing. After looking at the posix available in nodejs I am more compelled to just change only the join to posix.join in your existing implementation like below.

private _getObjectPath(objectName: string) {
    const { path } = this.options;
    // return join(path || '/', objectName); // Your implementation
    return posix.join(path || '/', objectName); // Replacing **join** with **posix.join**
}

I just want to know if using the string interpolation like so return posix.join(`${path || '/'}/${objectName}`); with the posix.join has any extra benefits. PS: Your earlier implementation actually checks if the path is undefined and defaults to '/'.