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

Feature/sharp resizer #24

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 22 additions & 32 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ var AWS = require("aws-sdk"),
fs = require("fs"),
request = require("request"),
path = require("path"),
sharp = require("sharp"),
winston = module.parent.require("winston"),
nconf = module.parent.require('nconf'),
gm = require("gm"),
im = gm.subClass({imageMagick: true}),
nconf = module.parent.require("nconf"),
meta = module.parent.require("./meta"),
db = module.parent.require("./database");

var plugin = {}
var plugin = {};

"use strict";

Expand Down Expand Up @@ -120,14 +119,13 @@ function makeError(err) {
}

plugin.activate = function (data) {
if (data.id === 'nodebb-plugin-s3-uploads') {
if (data.id === "nodebb-plugin-s3-uploads") {
fetchSettings();
}

};

plugin.deactivate = function (data) {
if (data.id === 'nodebb-plugin-s3-uploads') {
if (data.id === "nodebb-plugin-s3-uploads") {
S3Conn = null;
}
};
Expand All @@ -153,7 +151,7 @@ function renderAdmin(req, res) {
// Regenerate csrf token
var token = req.csrfToken();

var forumPath = nconf.get('url');
var forumPath = nconf.get("url");
if(forumPath.split("").reverse()[0] != "/" ){
forumPath = forumPath + "/";
}
Expand Down Expand Up @@ -219,7 +217,7 @@ plugin.uploadImage = function (data, callback) {
}

var type = image.url ? "url" : "file";
var allowedMimeTypes = ['image/png', 'image/jpeg', 'image/gif'];
var allowedMimeTypes = ["image/png", "image/jpeg", "image/gif"];

if (type === "file") {
if (!image.path) {
Expand All @@ -231,35 +229,27 @@ plugin.uploadImage = function (data, callback) {
}

fs.readFile(image.path, function (err, buffer) {
uploadToS3(image.name, err, buffer, callback);
var maxWidth = parseInt(meta.config.maximumImageWidth);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is correct. I looks like it will try to resize files? that are not images.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "normal" files and images are processed in different hooks and therefore in different handler methods.

Normal files are processed by plugin.uploadFile

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It kind of is there are cases when both hooks are called or when a file can be passed to the image hook so its best to just let that through. I've not used sharp and don't know what errors it gives but a try catch around the resize and just uploading if it fails would be best I think.

sharp(buffer)
.resize(maxWidth)
.withoutEnlargement()
.toBuffer(function(err, outputBuffer) {
uploadToS3(image.name, err, outputBuffer, callback);
});
});
}
else {
if (allowedMimeTypes.indexOf(mime.lookup(image.url)) === -1) {
return callback(new Error("invalid mime type"));
}
var filename = image.url.split("/").pop();
// We're uploading a new profile picture from an external URL

var imageDimension = parseInt(meta.config.profileImageDimension, 10) || 128;

// Resize image.
im(request(image.url), filename)
.resize(imageDimension + "^", imageDimension + "^")
.stream(function (err, stdout, stderr) {
if (err) {
return callback(makeError(err));
}

// This is sort of a hack - We"re going to stream the gm output to a buffer and then upload.
// See https://github.com/aws/aws-sdk-js/issues/94
var buf = new Buffer(0);
stdout.on("data", function (d) {
buf = Buffer.concat([buf, d]);
});
stdout.on("end", function () {
uploadToS3(filename, null, buf, callback);
// Get the image and buffer it
request({url:image.url, encoding: null}, function(error, response, body) {
sharp(body)
.resize(imageDimension)
.toBuffer(function(err, outputBuffer) {
uploadToS3(image.name, err, outputBuffer, callback);
});
});
});
}
};

Expand Down
12 changes: 8 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"contributors": [
"Barış Soner Uşaklı <[email protected]>",
"Joseph Chen <[email protected]>",
"louise McMahon <[email protected]>"
"louise McMahon <[email protected]>",
"Carlos Escura <[email protected]>"
],
"license": "MIT",
"bugs": {
Expand All @@ -32,9 +33,12 @@
},
"dependencies": {
"aws-sdk": "^2.0.23",
"uuid": "~1.4.1",
"mime": "~1.2.11",
"request": "~2.69.0",
"gm": "~1.21.1"
"request": "~2.81.0",
"sharp": "^0.17.3",
"uuid": "~3.0.1"
},
"devDependencies": {
"eslint": "^3.19.0"
}
}