Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support other options for image uploading #248

Merged
merged 14 commits into from
Nov 27, 2016
Merged

Conversation

Yukaii
Copy link
Member

@Yukaii Yukaii commented Nov 14, 2016

TODOs

  • local filesystem
  • s3
  • enable s3 config in env
  • s3 guide: policy setting
  • image security - using sharp lib
  • update README

Screentcast

hackmd-filesystem-upload

@Yukaii
Copy link
Member Author

Yukaii commented Nov 14, 2016

Implement s3 image uploading using AWS javascript SDK.

References

var AWS = require('aws');
var config = new AWS.Config({
  accessKeyId: 'AKID', secretAccessKey: 'SECRET', region: 'us-west-2'
});

s3 = new AWS.S3(config);
var params = {
  Bucket: 's3-bucketname',
  Key: file.name,
  Body: data
};

s3.putObject(params, function (perr, pres) {
  if (perr) {
    console.log("Error uploading data: ", perr);
  } else {
    console.log("Successfully uploaded data to myBucket/myKey");
  }
});

@jackycute
Copy link
Member

@Yukaii Great PR! Thanks a lot!
I'm also considering the security issue beneath the images.
It's better to remove EXIF and process them before actually saving them.
And it's important to limit the support image formats and use some lib to help.
For example: https://github.com/lovell/sharp

😄

@jackycute
Copy link
Member

BTW, I think this should return full path with config.serverurl prefixed, or it might not able to access in some cases.

@Yukaii
Copy link
Member Author

Yukaii commented Nov 16, 2016

@jackycute

I'm not sure whether sharp will have huge impact on performance. 😓 We might need more optimization on this in the future.

@jackycute
Copy link
Member

I just looked at sharp's README.md.
And it show up that they process the image via pure lib so it will still remain non-IO-blocking.
Hope it won't impact the server performance, we'll see.

@Yukaii Yukaii changed the title [WIP] Support other options for image uploading Support other options for image uploading Nov 17, 2016
@Yukaii Yukaii force-pushed the file-upload-options branch 3 times, most recently from 7a85d11 to 4fce1b8 Compare November 17, 2016 10:27
@ccoenen
Copy link
Contributor

ccoenen commented Nov 19, 2016

I have to admit I did not look at the code before asking, but: could this extend to file upload beyond images? Sometimes this might come in handy to also store - i don't know - a .doc or .zip alongside your hackmd pad.

@Yukaii
Copy link
Member Author

Yukaii commented Nov 21, 2016

@ccoenen In this PR, I only focus on image uploading & processing. IMO text-based editing is the main goal of HackMD, similar products like Hackpad or Quip have limited support for file uploading.

To implement this feature, we'll need to open another api for handling file uploading specifically(s3 or filesystem), checking file size(maybe unlimited for self-hosting version) and file type in frontend code. You could open another issue to track this. 😸

@ccoenen
Copy link
Contributor

ccoenen commented Nov 21, 2016

No problem!

@jackycute jackycute merged commit bd3d495 into master Nov 27, 2016
@Yukaii Yukaii deleted the file-upload-options branch November 27, 2016 03:56
JJediny pushed a commit to 18F/codimd-cloudgov-template that referenced this pull request Dec 16, 2020
Update example config for gitlab authorization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants