Replace jimp with sharp (#6774)

* add probe image size and max image size

* replace jimp and image-probe with sharp

* better name for test

* resize with just path

* resize thumb inplace

* use filename

install/package.json

@@ -51,7 +51,6 @@
         "helmet": "^3.11.0",
         "html-to-text": "^4.0.0",
         "ipaddr.js": "^1.5.4",
-        "jimp": "0.5.0",
         "jquery": "^3.2.1",
         "jsesc": "2.5.1",
         "json-2-csv": "^2.1.2",
@@ -97,6 +96,7 @@
         "sanitize-html": "^1.16.3",
         "semver": "^5.4.1",
         "serve-favicon": "^2.4.5",
+        "sharp": "0.20.8",
         "sitemap": "^1.13.0",
         "socket.io": "2.1.1",
         "socket.io-adapter-cluster": "^1.0.1",

public/language/en-GB/admin/settings/uploads.json

@@ -10,6 +10,10 @@
 	"resize-image-quality-help": "Use a lower quality setting to reduce the file size of resized images.",
 	"max-file-size": "Maximum File Size (in KiB)",
 	"max-file-size-help": "(in kibibytes, default: 2048 KiB)",
+	"reject-image-width": "Maximum Image Width (in pixels)",
+	"reject-image-width-help": "Images wider than this value will be rejected.",
+	"reject-image-height": "Maximum Image Height (in pixels)",
+	"reject-image-height-help": "Images taller than this value will be rejected.",
 	"allow-topic-thumbnails": "Allow users to upload topic thumbnails",
 	"topic-thumb-size": "Topic Thumb Size",
 	"allowed-file-extensions": "Allowed File Extensions",

public/language/en-GB/error.json

@@ -105,6 +105,7 @@
 	"invalid-image-type": "Invalid image type. Allowed types are: %1",
 	"invalid-image-extension": "Invalid image extension",
 	"invalid-file-type": "Invalid file type. Allowed types are: %1",
+	"invalid-image-dimensions": "Image dimensions are too big",
 
 	"group-name-too-short": "Group name too short",
 	"group-name-too-long": "Group name too long",

src/controllers/admin/uploads.js

@@ -5,7 +5,6 @@ var async = require('async');
 var nconf = require('nconf');
 var mime = require('mime');
 var fs = require('fs');
-var jimp = require('jimp');
 
 var meta = require('../../meta');
 var posts = require('../../posts');
@@ -177,16 +176,13 @@ uploadsController.uploadTouchIcon = function (req, res, next) {
 			}
 
 			// Resize the image into squares for use as touch icons at various DPIs
-			async.each(sizes, function (size, next) {
+			async.eachSeries(sizes, function (size, next) {
-				async.series([
+				image.resizeImage({
-					async.apply(file.saveFileToLocal, 'touchicon-' + size + '.png', 'system', uploadedFile.path),
+					path: uploadedFile.path,
-					async.apply(image.resizeImage, {
+					target: path.join(nconf.get('upload_path'), 'system', 'touchicon-' + size + '.png'),
-						path: path.join(nconf.get('upload_path'), 'system', 'touchicon-' + size + '.png'),
+					width: size,
-						extension: 'png',
+					height: size,
-						width: size,
+				}, next);
-						height: size,
-					}),
-				], next);
 			}, function (err) {
 				file.delete(uploadedFile.path);
 
@@ -291,7 +287,6 @@ function uploadImage(filename, folder, uploadedFile, req, res, next) {
 					async.apply(image.resizeImage, {
 						path: uploadedFile.path,
 						target: uploadPath,
-						extension: 'png',
 						height: 50,
 					}),
 					async.apply(meta.configs.set, 'brand:emailLogo', path.join(nconf.get('upload_url'), 'system/site-logo-x50.png')),
@@ -299,15 +294,16 @@ function uploadImage(filename, folder, uploadedFile, req, res, next) {
 					next(err, imageData);
 				});
 			} else if (path.basename(filename, path.extname(filename)) === 'og:image' && folder === 'system') {
-				jimp.read(imageData.path).then(function (image) {
+				image.size(imageData.path, function (err, size) {
+					if (err) {
+						next(err);
+					}
 					meta.configs.setMultiple({
-						'og:image:height': image.bitmap.height,
+						'og:image:width': size.width,
-						'og:image:width': image.bitmap.width,
+						'og:image:height': size.height,
 					}, function (err) {
 						next(err, imageData);
 					});
-				}).catch(function (err) {
-					next(err);
 				});
 			} else {
 				setImmediate(next, null, imageData);

src/controllers/uploads.js

@@ -25,7 +25,7 @@ uploadsController.upload = function (req, res, filesIterator) {
 		files = files[0];
 	}
 
-	async.map(files, filesIterator, function (err, images) {
+	async.mapSeries(files, filesIterator, function (err, images) {
 		deleteTempFiles(files);
 
 		if (err) {
@@ -56,6 +56,9 @@ function uploadAsImage(req, uploadedFile, callback) {
 			if (!canUpload) {
 				return next(new Error('[[error:no-privileges]]'));
 			}
+			image.checkDimensions(uploadedFile.path, next);
+		},
+		function (next) {
 			if (plugins.hasListeners('filter:uploadImage')) {
 				return plugins.fireHook('filter:uploadImage', {
 					image: uploadedFile,
@@ -113,14 +116,9 @@ function resizeImage(fileObj, callback) {
 				return callback(null, fileObj);
 			}
 
-			var dirname = path.dirname(fileObj.path);
-			var extname = path.extname(fileObj.path);
-			var basename = path.basename(fileObj.path, extname);
-
 			image.resizeImage({
 				path: fileObj.path,
-				target: path.join(dirname, basename + '-resized' + extname),
+				target: file.appendToFileName(fileObj.path, '-resized'),
-				extension: extname,
 				width: parseInt(meta.config.maximumImageWidth, 10) || 760,
 				quality: parseInt(meta.config.resizeImageQuality, 10) || 60,
 			}, next);
@@ -157,7 +155,6 @@ uploadsController.uploadThumb = function (req, res, next) {
 				var size = parseInt(meta.config.topicThumbSize, 10) || 120;
 				image.resizeImage({
 					path: uploadedFile.path,
-					extension: path.extname(uploadedFile.name),
 					width: size,
 					height: size,
 				}, next);

src/file.js

@@ -4,7 +4,7 @@ var fs = require('fs');
 var nconf = require('nconf');
 var path = require('path');
 var winston = require('winston');
-var jimp = require('jimp');
+var sharp = require('sharp');
 var mkdirp = require('mkdirp');
 var mime = require('mime');
 var graceful = require('graceful-fs');
@@ -107,12 +107,22 @@ file.isFileTypeAllowed = function (path, callback) {
 		});
 	}
 
-	// Attempt to read the file, if it passes, file type is allowed
+	sharp(path, {
-	jimp.read(path, function (err) {
+		failOnError: true,
+	}).metadata(function (err) {
 		callback(err);
 	});
 };
 
+// https://stackoverflow.com/a/31205878/583363
+file.appendToFileName = function (filename, string) {
+	var dotIndex = filename.lastIndexOf('.');
+	if (dotIndex === -1) {
+		return filename + string;
+	}
+	return filename.substring(0, dotIndex) + string + filename.substring(dotIndex);
+};
+
 file.allowedExtensions = function () {
 	var meta = require('./meta');
 	var allowedExtensions = (meta.config.allowedFileExtensions || '').trim();
@@ -163,7 +173,7 @@ file.existsSync = function (path) {
 file.delete = function (path, callback) {
 	callback = callback || function () {};
 	if (!path) {
-		return callback();
+		return setImmediate(callback);
 	}
 	fs.unlink(path, function (err) {
 		if (err) {

src/groups/cover.js

@@ -2,7 +2,6 @@
 
 var async = require('async');
 var path = require('path');
-var Jimp = require('jimp');
 var mime = require('mime');
 
 var db = require('../database');
@@ -27,7 +26,6 @@ module.exports = function (Groups) {
 		var tempPath = data.file ? data.file : '';
 		var url;
 		var type = data.file ? mime.getType(data.file) : 'image/png';
-
 		async.waterfall([
 			function (next) {
 				if (tempPath) {
@@ -49,7 +47,10 @@ module.exports = function (Groups) {
 				Groups.setGroupField(data.groupName, 'cover:url', url, next);
 			},
 			function (next) {
-				resizeCover(tempPath, next);
+				image.resizeImage({
+					path: tempPath,
+					width: 358,
+				}, next);
 			},
 			function (next) {
 				uploadsController.uploadGroupCover(uid, {
@@ -74,22 +75,6 @@ module.exports = function (Groups) {
 		});
 	};
 
-	function resizeCover(path, callback) {
-		async.waterfall([
-			function (next) {
-				new Jimp(path, next);
-			},
-			function (image, next) {
-				image.resize(358, Jimp.AUTO, next);
-			},
-			function (image, next) {
-				image.write(path, next);
-			},
-		], function (err) {
-			callback(err);
-		});
-	}
-
 	Groups.removeCover = function (data, callback) {
 		db.deleteObjectFields('group:' + data.groupName, ['cover:url', 'cover:thumb:url', 'cover:position'], callback);
 	};

src/image.js

@@ -3,9 +3,14 @@
 var os = require('os');
 var fs = require('fs');
 var path = require('path');
-var Jimp = require('jimp');
-var async = require('async');
 var crypto = require('crypto');
+var async = require('async');
+
+var sharp = require('sharp');
+if (os.platform() === 'win32') {
+	// https://github.com/lovell/sharp/issues/1259
+	sharp.cache(false);
+}
 
 var file = require('./file');
 var plugins = require('./plugins');
@@ -17,7 +22,6 @@ image.resizeImage = function (data, callback) {
 		plugins.fireHook('filter:image.resize', {
 			path: data.path,
 			target: data.target,
-			extension: data.extension,
 			width: data.width,
 			height: data.height,
 			quality: data.quality,
@@ -25,64 +29,25 @@ image.resizeImage = function (data, callback) {
 			callback(err);
 		});
 	} else {
-		new Jimp(data.path, function (err, image) {
+		async.waterfall([
-			if (err) {
+			function (next) {
-				return callback(err);
+				fs.readFile(data.path, next);
-			}
+			},
-
+			function (buffer, next) {
-			var w = image.bitmap.width;
+				var sharpImage = sharp(buffer, {
-			var h = image.bitmap.height;
+					failOnError: true,
-			var origRatio = w / h;
+				});
-			var desiredRatio = data.width && data.height ? data.width / data.height : origRatio;
+				sharpImage.rotate(); // auto-orients based on exif data
-			var x = 0;
+				sharpImage.resize(data.hasOwnProperty('width') ? data.width : null, data.hasOwnProperty('height') ? data.height : null);
-			var y = 0;
+
-			var crop;
+				if (data.quality) {
-
+					sharpImage.jpeg({ quality: data.quality });
-			if (image._exif && image._exif.tags && image._exif.tags.Orientation) {
-				image.exifRotate();
-			}
-
-			if (origRatio !== desiredRatio) {
-				if (desiredRatio > origRatio) {
-					desiredRatio = 1 / desiredRatio;
-				}
-				if (origRatio >= 1) {
-					y = 0;	// height is the smaller dimension here
-					x = Math.floor((w / 2) - (h * desiredRatio / 2));
-					crop = async.apply(image.crop.bind(image), x, y, h * desiredRatio, h);
-				} else {
-					x = 0;	// width is the smaller dimension here
-					y = Math.floor((h / 2) - (w * desiredRatio / 2));
-					crop = async.apply(image.crop.bind(image), x, y, w, w * desiredRatio);
 				}
-			} else {
+
-				// Simple resize given either width, height, or both
+				sharpImage.toFile(data.target || data.path, next);
-				crop = async.apply(setImmediate);
+			},
-			}
+		], function (err) {
-
+			callback(err);
-			async.waterfall([
-				crop,
-				function (_image, next) {
-					if (typeof _image === 'function' && !next) {
-						next = _image;
-						_image = image;
-					}
-
-					if ((data.width && data.height) || (w > data.width) || (h > data.height)) {
-						_image.resize(data.width || Jimp.AUTO, data.height || Jimp.AUTO, next);
-					} else {
-						next(null, image);
-					}
-				},
-				function (image, next) {
-					if (data.quality) {
-						image.quality(data.quality);
-					}
-					image.write(data.target || data.path, next);
-				},
-			], function (err) {
-				callback(err);
-			});
 		});
 	}
 };
@@ -91,21 +56,13 @@ image.normalise = function (path, extension, callback) {
 	if (plugins.hasListeners('filter:image.normalise')) {
 		plugins.fireHook('filter:image.normalise', {
 			path: path,
-			extension: extension,
 		}, function (err) {
 			callback(err, path + '.png');
 		});
 	} else {
-		async.waterfall([
+		sharp(path, { failOnError: true }).png().toFile(path + '.png', function (err) {
-			function (next) {
+			callback(err, path + '.png');
-				new Jimp(path, next);
+		});
-			},
-			function (image, next) {
-				image.write(path + '.png', function (err) {
-					next(err, path + '.png');
-				});
-			},
-		], callback);
 	}
 };
 
@@ -114,15 +71,32 @@ image.size = function (path, callback) {
 		plugins.fireHook('filter:image.size', {
 			path: path,
 		}, function (err, image) {
-			callback(err, image);
+			callback(err, image ? { width: image.width, height: image.height } : undefined);
 		});
 	} else {
-		new Jimp(path, function (err, data) {
+		sharp(path, { failOnError: true }).metadata(function (err, metadata) {
-			callback(err, data ? data.bitmap : null);
+			callback(err, metadata ? { width: metadata.width, height: metadata.height } : undefined);
 		});
 	}
 };
 
+image.checkDimensions = function (path, callback) {
+	const meta = require('./meta');
+	image.size(path, function (err, result) {
+		if (err) {
+			return callback(err);
+		}
+
+		const maxWidth = parseInt(meta.config.rejectImageWidth, 10) || 5000;
+		const maxHeight = parseInt(meta.config.rejectImageHeight, 10) || 5000;
+		if (result.width > maxWidth || result.height > maxHeight) {
+			return callback(new Error('[[error:invalid-image-dimensions]]'));
+		}
+
+		callback();
+	});
+};
+
 image.convertImageToBase64 = function (path, callback) {
 	fs.readFile(path, 'base64', callback);
 };

src/topics/thumb.js

@@ -49,7 +49,6 @@ module.exports = function (Topics) {
 				var size = parseInt(meta.config.topicThumbSize, 10) || 120;
 				image.resizeImage({
 					path: pathToUpload,
-					extension: path.extname(pathToUpload),
 					width: size,
 					height: size,
 				}, next);

src/upgrades/1.6.0/generate-email-logo.js

@@ -34,7 +34,6 @@ module.exports = {
 					image.resizeImage({
 						path: sourcePath,
 						target: uploadPath,
-						extension: 'png',
 						height: 50,
 					}, next);
 				});

src/user/picture.js

@@ -123,11 +123,9 @@ module.exports = function (User) {
 			},
 			function (path, next) {
 				picture.path = path;
-
 				var imageDimension = parseInt(meta.config.profileImageDimension, 10) || 200;
 				image.resizeImage({
 					path: picture.path,
-					extension: extension,
 					width: imageDimension,
 					height: imageDimension,
 				}, next);

src/views/admin/settings/uploads.tpl

@@ -21,7 +21,7 @@
 			</div>
 
 			<div class="form-group">
-				<label for="maximumImageWidth">[[admin/settings/uploads:private-extensions]]</label>
+				<label for="privateUploadsExtensions">[[admin/settings/uploads:private-extensions]]</label>
 				<input type="text" class="form-control" value="" data-field="privateUploadsExtensions" placeholder="">
 				<p class="help-block">
 					[[admin/settings/uploads:private-uploads-extensions-help]]
@@ -52,6 +52,22 @@
 				</p>
 			</div>
 
+			<div class="form-group">
+				<label for="rejectImageWidth">[[admin/settings/uploads:reject-image-width]]</label>
+				<input type="text" class="form-control" value="5000" data-field="rejectImageWidth" placeholder="5000">
+				<p class="help-block">
+					[[admin/settings/uploads:reject-image-width-help]]
+				</p>
+			</div>
+
+			<div class="form-group">
+				<label for="rejectImageHeight">[[admin/settings/uploads:reject-image-height]]</label>
+				<input type="text" class="form-control" value="5000" data-field="rejectImageHeight" placeholder="5000">
+				<p class="help-block">
+					[[admin/settings/uploads:reject-image-height-help]]
+				</p>
+			</div>
+
 			<div class="checkbox">
 				<label class="mdl-switch mdl-js-switch mdl-js-ripple-effect">
 					<input class="mdl-switch__input" type="checkbox" data-field="allowTopicsThumbnail">

test/files/notanimage.png

@@ -0,0 +1 @@
+this is totally not a png

test/uploads.js

@@ -15,6 +15,8 @@ var privileges = require('../src/privileges');
 var meta = require('../src/meta');
 var socketUser = require('../src/socket.io/user');
 var helpers = require('./helpers');
+var file = require('../src/file');
+var image = require('../src/image');
 
 describe('Upload Controllers', function () {
 	var tid;
@@ -133,6 +135,38 @@ describe('Upload Controllers', function () {
 			});
 		});
 
+		it('should fail to upload image to post if image dimensions are too big', function (done) {
+			helpers.uploadFile(nconf.get('url') + '/api/post/upload', path.join(__dirname, '../test/files/toobig.jpg'), {}, jar, csrf_token, function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 500);
+				assert(body.error, '[[error:invalid-image-dimensions]]');
+				done();
+			});
+		});
+
+		it('should fail to upload image to post if image is broken', function (done) {
+			helpers.uploadFile(nconf.get('url') + '/api/post/upload', path.join(__dirname, '../test/files/brokenimage.png'), {}, jar, csrf_token, function (err, res, body) {
+				assert.ifError(err);
+				assert.equal(res.statusCode, 500);
+				assert(body.error, 'invalid block type');
+				done();
+			});
+		});
+
+		it('should fail if file is not an image', function (done) {
+			file.isFileTypeAllowed(path.join(__dirname, '../test/files/notanimage.png'), function (err) {
+				assert.equal(err.message, 'Input file is missing or of an unsupported image format');
+				done();
+			});
+		});
+
+		it('should fail if file is not an image', function (done) {
+			image.size(path.join(__dirname, '../test/files/notanimage.png'), function (err) {
+				assert.equal(err.message, 'Input file is missing or of an unsupported image format');
+				done();
+			});
+		});
+
 		it('should fail if topic thumbs are disabled', function (done) {
 			helpers.uploadFile(nconf.get('url') + '/api/topic/thumb/upload', path.join(__dirname, '../test/files/test.png'), {}, jar, csrf_token, function (err, res, body) {
 				assert.ifError(err);