Objects uploaded with generateKey return incorrect locations

[AdrianCurtin]

From parse-server\lib\Controllers\FilesController.js - line 32

const location = await this.adapter.getFileLocation(config, filename);
    await this.adapter.createFile(filename, data, contentType, options);
    return {
      url: location,
      name: filename
    };

Location is a const fed in from getFileLocation, but getFileLocation will always return the location before key generation occurs.

object location created by this.adpter.createFile as an output argument is discarded. Additionally, getFileLocation will return the presigned item based on the old filename rather than the new file name.

Ex:

generateKey: (filename) => {
  return ${Date.now()}_${filename}; // unique prefix for every filename
}

Uploading "file.jpeg" will result in XXX.s3.amazonaws.com/file.jpeg as a key while uploading to XXX.s3.amazonaws.com/XXXX_file.jpeg

getFileLocation will return the full (optionally presignedUrl) or "https://${this._bucket}.s3.amazonaws.com/${this._bucketPrefix}${fileName}" where fileName is file.jpeg

However createFile will instead return a location of https://${this._bucket}.s3.${this._region}.amazonaws.com/this._bucketPrefix + this._generateKey(filename)

Until this issue is fixed, generateKey should be considered to be non-functional. Either the filename should be rewritten with the generated Key prior to getFileLocation (and the createFile call should not generate a new key) or the FilesController.js should be wrapped to account for this.

[parse-github-assistant[bot]]

Thanks for opening this issue!

[mtrezza]

What version of the adapter has this issue? Did you try different versions to see where this issues started to occur?

[AdrianCurtin]

I'm using v4.0 but best I can tell from the code inspection since it appears to be related to a need to work around the parse-server integration, this issue would pre-exist the new release.

[mtrezza]

@AdrianCurtin were you able to gain any insight already as to where the issue comes from?

[AdrianCurtin]

I mean it's definitively because location returned by getFileLocation and the location used in createFile is discarded.

https://github.com/parse-community/parse-server/blob/34636be5b7f39c0ec9585a836024a414eb9474bc/src/Controllers/FilesController.js#L32

getFileLocation should return the unmodified (potentially signed) url or key, and so including generateKey in that component does not make specific sense to me. In any case it cannot know what the generated key will be necessarily)

There's one fix in parse-server that would look like this.

Original

 const location = await this.adapter.getFileLocation(config, filename);
    await this.adapter.createFile(filename, data, contentType, options);
    return {
      url: location,
      name: filename,
    }

Updated

const defaultLocation = await this.adapter.getFileLocation(config, filename);
const createResult = await this.adapter.createFile(config, filename, data, contentType, options);

return {
  url: createResult?.Location || defaultLocation,
  name: filename,
};

I don't know if there's a specific one for s3 adapter that could be made and I don't know how this would affect other file adapters

[AdrianCurtin]

I opened a PR here for s3-adapter https://github.com/parse-community/parse-server-s3-adapter/pull/242

and a PR here for parse-server https://github.com/parse-community/parse-server/pull/9557

This fix will require both of these to work, but this PR can be merged in advance of the parse-server functionality with no ill effects

[mtrezza]

I think the S3 PR should be merged first, so that the Parse Server tests with the new adapter can pass?

[AdrianCurtin]

I think it also makes sense to add a test later (which will fail on the current parse server)

basically for situations with generateKey, the filename (or url) returned from FilesController.createFile contains the new key.

Also i note that the preserve filenames for this does not need to be enabled or disabled. generateKey for instance could strip the random hex string, or prepend /uploads/ to the string etc. This basically makes it possible to use s3 directories.

@mtrezza , do you think it would make sense to give generatekey access to different attributes apart from filename?, either contentType or options for instance. That way keys could be assigned filenames (or directories) according to content type.

I also like the option of giving it the application Id via the config (if provided), then in theory the user could do something like (if they set up the generate key appropriately).

s3://prefix/app-id/images/date/upload.jpg

[AdrianCurtin]

I added it to the last commit (without the config argument) and (without giving direct access to data)

[AdrianCurtin]

@mtrezza , this may also close https://github.com/parse-community/parse-server-s3-adapter/issues/87

[mtrezza]

We're just waiting for a review of the submitted PRs.

[AdrianCurtin]

I believe it also would close #140

[elrumo]

I found this thread after encountering this same issue. While the bug is being worked on, did any of you figure out a workaround?