Move filename validation into adaptor for AWS "directories". Add preserveFileName Support #76

mpatnode · 2019-10-26T00:04:47Z

Sorry for mixing the two features here, but they were interdependent. The first was to implement file validation for AWS files. This validation method will be ignored until parse-community/parse-server#6157 is merged.

The end result here is that now you can specify an AWS "directory" path in your filename and it won't be rejected by a Router class that doesn't understand your filesystem. You can now supply methods for filename validation and AWS key generation, so you can decide what's a valid filename, and how you want to deal with duplicate filenames. Note the default behavior for the later needs to be disabled in parse-server:FilesController if you want to use directories in AWS.

mpatnode · 2019-10-26T00:05:57Z

index.js

@@ -2,6 +2,8 @@
 //
 // Stores Parse files in AWS S3.

+const FilesAdapter = require('parse-server/lib/Adapters/Files/FilesAdapter').FilesAdapter;
+const Parse = require('parse/lib/node/Parse').Parse;


I needed to bring in Parse to throw the right error codes.

codecov · 2019-10-26T00:06:25Z

Codecov Report

Merging #76 into master will increase coverage by 0.27%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
+ Coverage   94.26%   94.54%   +0.27%     
==========================================
  Files           2        2              
  Lines         157      165       +8     
  Branches       33       34       +1     
==========================================
+ Hits          148      156       +8     
  Misses          9        9

Impacted Files	Coverage Δ
lib/optionsFromArguments.js	`96% <100%> (+0.22%)`	⬆️
index.js	`93.33% <100%> (+0.31%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa3f3a5...bb38423. Read the comment docs.

mpatnode · 2019-10-26T00:06:28Z

index.js

@@ -11,11 +13,12 @@ const awsCredentialsDeprecationNotice = function awsCredentialsDeprecationNotice
    'See: https://github.com/parse-server-modules/parse-server-s3-adapter#aws-credentials for details');
 };

-class S3Adapter {
+class S3Adapter extends FilesAdapter {


This seemed like the right thing to do, even though the parse-server PR doesn't actually change the signature.

mpatnode · 2019-10-26T00:07:49Z

spec/test.spec.js

-      }, 100),
-      deleteObject: (params, callback) => setTimeout(() => {
-        const { Key } = params;
+  function makeS3Adaptor(options) {


This code came in handy for more createFile requests.

spec/test.spec.js

dplewis · 2019-10-26T02:38:05Z

@acinader What do you think? This is similar to #16. I think it fits the suggestions.

dplewis

@yomybaby Can you test this and provide feedback? I think this should solve your issue.

package.json

spec/test.spec.js

yomybaby · 2019-10-29T01:01:53Z

@dplewis Ok. I'll take a look at it tonight.

mpatnode · 2019-10-29T18:33:51Z

README.md

@@ -169,6 +176,8 @@ var s3Options = {
  baseUrl: process.env.SPACES_BASE_URL, 
  region: process.env.SPACES_REGION,
  directAccess: true,
+  preserveFilename: "always",
+  fileNameCheck: "safe",


FWIW: I'm only using these settings in my app. Happy to remove the others if they are at controversial.

yomybaby · 2019-10-30T13:06:47Z

Move filename validation out of the Router and into the FilesAdaptor(parse-community/parse-server#6157) is great idea.

How about make this more customizable?
There is a two options instead of preserveFilename and fileNameCheck.
There are function.

like this

new S3Adapter({
  bucket: "my_bucket",
  generateKey: (filename) => {
    return 'origin/'+filename;
  },
  validateFilename: (filename) => {
    if (filename.length > 1024) {
       return 'Filename too long.';
     }
     return null;
  }
})

mpatnode · 2019-10-30T15:02:38Z

Thought about it, but use my case was fairly simple. If it's needed in the future, the validation code could simply check the type of the validate/preserveFilename parameter, and do the right thing. I wanted to preserve the same filename check behavior from the old code, while enabling the directory capability. Happy to remove the preserveFilename feature until someone needs it. I'm not using it.

mpatnode · 2019-11-03T16:01:47Z

How about make this more customizable?
There is a two options instead of preserveFilename and fileNameCheck.
There are function.

So don't know when I'll have time to get back to this. @yomybaby Any interest in implementing that on top of my branch?

dplewis · 2019-11-20T21:58:29Z

@mpatnode Sorry for the late reply. Adding parse module (not parse-server) for Parse.Error should be ok here as we use it in the parse-server-push-adapter

https://github.com/parse-community/parse-server-push-adapter/blob/master/package.json#L39

mpatnode · 2019-11-27T17:34:12Z

@mpatnode Sorry for the late reply. Adding parse module (not parse-server) for Parse.Error should be ok here as we use it in the parse-server-push-adapter

Ignore previous comment. Will do!

mpatnode · 2019-11-27T21:17:16Z

lib/optionsFromArguments.js

@@ -90,6 +92,8 @@ const optionsFromArguments = function optionsFromArguments(args) {
  options = fromEnvironmentOrDefault(options, 'baseUrlDirect', 'S3_BASE_URL_DIRECT', false);
  options = fromEnvironmentOrDefault(options, 'signatureVersion', 'S3_SIGNATURE_VERSION', 'v4');
  options = fromEnvironmentOrDefault(options, 'globalCacheControl', 'S3_GLOBAL_CACHE_CONTROL', null);
+  options = fromOptionsDictionaryOrDefault(options, 'generateKey', null);
+  options = fromOptionsDictionaryOrDefault(options, 'validateFilename', null);


JS code in env vars didn't seem like a good idea to me...

mpatnode · 2019-11-27T21:19:10Z

README.md

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


@yomybaby This look good to you?

package.json

mpatnode · 2019-11-27T21:27:36Z

README.md

+                    validateFilename: (filename) => {
+                      if (filename.length > 1024) {
+                         return new Parse.Error(
+                            Parse.Error.INVALID_FILE_NAME,


@dplewis Hmm... this might be problematic?

No longer necessary: parse-community/parse-server#6246

You should throw new Parse.Error instead of returning

Actually ignore that last comment this is correct.

@dplewis @acinader So are we good to go here?

mpatnode · 2019-12-03T01:45:41Z

@dplewis This is happy now: parse-community/parse-server#6246

mpatnode commented Oct 26, 2019

View reviewed changes

spec/test.spec.js Show resolved Hide resolved

mpatnode mentioned this pull request Oct 26, 2019

Move filename validation out of the Router and into the FilesAdaptor parse-community/parse-server#6157

Merged

dplewis suggested changes Oct 29, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

spec/test.spec.js Outdated Show resolved Hide resolved

mpatnode commented Oct 29, 2019

View reviewed changes

Mike Patnode added 8 commits November 27, 2019 09:44

Add support for the validateFilename method

66bd154

Add AWS preserveFileName support with options for "directories"

b5affa5

Don't extend FilesAdapter (pre-emptive PR request)

63f4be4

Minor test improvements

d1b408c

Remove all dependencies. Bump version number

0d53f90

speling is hard

cb59a55

Don't encode / when returning urls. closes parse-community#75

5391b8a

Make % safe

aa6f631

mpatnode force-pushed the validate-filename branch 2 times, most recently from 43738f4 to 1ec0b10 Compare November 27, 2019 21:16

mpatnode commented Nov 27, 2019

View reviewed changes

package.json Show resolved Hide resolved

mpatnode force-pushed the validate-filename branch from 1ec0b10 to de25224 Compare November 27, 2019 21:24

Allow filename validation and key generation to be methods

e7f6cc3

mpatnode force-pushed the validate-filename branch from de25224 to e7f6cc3 Compare November 27, 2019 21:26

mpatnode commented Nov 27, 2019

View reviewed changes

mpatnode mentioned this pull request Nov 28, 2019

Allow validateFilename to return a string or Parse Error parse-community/parse-server#6246

Merged

Parse.Error return type no longer required

bb38423

acinader requested a review from dplewis December 10, 2019 22:46

dplewis approved these changes Dec 11, 2019

View reviewed changes

dplewis merged commit 041595d into parse-community:master Dec 11, 2019

dplewis mentioned this pull request Dec 11, 2019

[SUGGESTION] Organize uploaded files in different folders #16

Closed

cbaker6 mentioned this pull request Sep 17, 2022

Ability to specify a folder name in the bucket while saving Parse Fille parse-community/Parse-Swift#417

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move filename validation into adaptor for AWS "directories". Add preserveFileName Support #76

Move filename validation into adaptor for AWS "directories". Add preserveFileName Support #76

mpatnode commented Oct 26, 2019 •

edited

Loading

mpatnode Oct 26, 2019

codecov bot commented Oct 26, 2019 •

edited

Loading

mpatnode Oct 26, 2019

mpatnode Oct 26, 2019

dplewis commented Oct 26, 2019

dplewis left a comment

yomybaby commented Oct 29, 2019

mpatnode Oct 29, 2019

yomybaby commented Oct 30, 2019

mpatnode commented Oct 30, 2019

mpatnode commented Nov 3, 2019

dplewis commented Nov 20, 2019

mpatnode commented Nov 27, 2019 •

edited

Loading

mpatnode Nov 27, 2019

mpatnode Nov 27, 2019

mpatnode Nov 27, 2019

mpatnode Nov 28, 2019

dplewis Dec 2, 2019

dplewis Dec 2, 2019

mpatnode Dec 6, 2019

mpatnode commented Dec 3, 2019

Move filename validation into adaptor for AWS "directories". Add preserveFileName Support #76

Move filename validation into adaptor for AWS "directories". Add preserveFileName Support #76

Conversation

mpatnode commented Oct 26, 2019 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Oct 26, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dplewis commented Oct 26, 2019

dplewis left a comment

Choose a reason for hiding this comment

yomybaby commented Oct 29, 2019

Choose a reason for hiding this comment

yomybaby commented Oct 30, 2019

mpatnode commented Oct 30, 2019

mpatnode commented Nov 3, 2019

dplewis commented Nov 20, 2019

mpatnode commented Nov 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpatnode commented Dec 3, 2019

mpatnode commented Oct 26, 2019 •

edited

Loading

codecov bot commented Oct 26, 2019 •

edited

Loading

mpatnode commented Nov 27, 2019 •

edited

Loading