keystonejs / keystone-storage-adapter-s3

⚠️ Archived - Legacy S3 Storage Adapter for KeystoneJS
MIT License
17 stars 55 forks source link

failure to prefix path with `/` results in leaking local directory info into s3 url #25

Closed weisjohn closed 6 years ago

weisjohn commented 7 years ago

When specifying the [s3 config].path property without a / prefix, the resulting filename at s3 includes the current running directory of the application. I believe this is less than desirable, and possibly a bug.

I believe this is due to the _resolveFilename step here: https://github.com/keystonejs/keystone-storage-adapter-s3/blob/4fb4ce18f1f59606099fd703105571afb56b761d/index.js#L107 which in turn calls: https://github.com/keystonejs/keystone-storage-adapter-s3/blob/4fb4ce18f1f59606099fd703105571afb56b761d/index.js#L86-L94

Code to reproduce:

const keystone = require('keystone');

const s3 = {
  bucket: 'foobar',
  key: '...',
  secret: '...',
  path: 'dev',
  headers: { 'x-amz-acl': 'public-read' }
}
const storage = new keystone.Storage({
  adapter: require('keystone-storage-adapter-s3'),
  s3,
  schema: {
    bucket: true, // optional; store the bucket the file was uploaded to in your db
    etag: true, // optional; store the etag for the resource
    path: true, // optional; store the path of the file in your db
    url: true, // optional; generate & store a public URL
  },
});

const Types = keystone.Field.Types;
const Client = new keystone.List('Client', {
  autokey: { from: 'name', path: 'key', unique: true }
});

Client.add({
  name: { type: String, required: true },
  image: { type: Types.File, storage },
});
Client.register();

When uploading the image to the Client through the admin, the resulting URL is https://foobar.s3.amazonaws.com/Users/john/mysrc/[redacted]/[redcated]/app/dev/a5735c91051859368da65af294f11892.png when it probably should be: http://foobar.s3.amazonaws.com/dev/a5735c91051859368da65af294f11892.png

Even though the docs specify a / as the prefix, I myself failed to enter it and it took quite a while to understand what was going on.

grishamo commented 7 years ago

+1

mikehazell commented 6 years ago

fixed in v2.0.0 #35