From 2f534dc01c61d0fe8145d0ffe5d880b2d7452287 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Thu, 4 May 2017 23:20:37 +0100 Subject: [PATCH] Base maximum output dimensions on limitation of format --- docs/api-resize.md | 4 ++-- docs/changelog.md | 4 ++++ lib/composite.js | 5 +---- lib/constructor.js | 14 +------------- lib/input.js | 12 ++++++------ lib/resize.js | 12 ++++++------ src/common.cc | 19 +++++++++++++++++++ src/common.h | 5 +++++ src/pipeline.cc | 20 ++++++++++++++++---- test/unit/resize.js | 34 ++++++++++++++++++++++------------ 10 files changed, 82 insertions(+), 47 deletions(-) diff --git a/docs/api-resize.md b/docs/api-resize.md index fc7e7d72..e3237bf5 100644 --- a/docs/api-resize.md +++ b/docs/api-resize.md @@ -33,8 +33,8 @@ Possible enlargement interpolators are: **Parameters** -- `width` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** pixels wide the resultant image should be, between 1 and 16383 (0x3FFF). Use `null` or `undefined` to auto-scale the width to match the height. -- `height` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** pixels high the resultant image should be, between 1 and 16383. Use `null` or `undefined` to auto-scale the height to match the width. +- `width` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** pixels wide the resultant image should be. Use `null` or `undefined` to auto-scale the width to match the height. +- `height` **[Number](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number)?** pixels high the resultant image should be. Use `null` or `undefined` to auto-scale the height to match the width. - `options` **[Object](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object)?** - `options.kernel` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** the kernel to use for image reduction. (optional, default `'lanczos3'`) - `options.interpolator` **[String](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)?** the interpolator to use for image enlargement. (optional, default `'bicubic'`) diff --git a/docs/changelog.md b/docs/changelog.md index 9c04ca84..48446e03 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,10 @@ Requires libvips v8.5.2. #### v0.18.0 - TBD +* Ensure maximum output dimensions are based on the format to be used. + [#176](https://github.com/lovell/sharp/issues/176) + [@stephanebachelier](https://github.com/stephanebachelier) + * Avoid costly (un)premultiply when using overlayWith without alpha channel. [#573](https://github.com/lovell/sharp/issues/573) [@strarsis](https://github.com/strarsis) diff --git a/lib/composite.js b/lib/composite.js index e795ab0d..f29df7f0 100644 --- a/lib/composite.js +++ b/lib/composite.js @@ -68,10 +68,7 @@ function overlayWith (overlay, options) { } } if (is.defined(options.left) || is.defined(options.top)) { - if ( - is.integer(options.left) && is.inRange(options.left, 0, this.constructor.maximum.width) && - is.integer(options.top) && is.inRange(options.top, 0, this.constructor.maximum.height) - ) { + if (is.integer(options.left) && options.left >= 0 && is.integer(options.top) && options.top >= 0) { this.options.overlayXOffset = options.left; this.options.overlayYOffset = options.top; } else { diff --git a/lib/constructor.js b/lib/constructor.js index deb5ed4c..ca28d0f4 100644 --- a/lib/constructor.js +++ b/lib/constructor.js @@ -97,7 +97,7 @@ const Sharp = function (input, options) { this.options = { // input options sequentialRead: false, - limitInputPixels: maximum.pixels, + limitInputPixels: Math.pow(0x3FFF, 2), // ICC profiles iccProfilePath: path.join(__dirname, 'icc') + path.sep, // resize options @@ -189,18 +189,6 @@ const Sharp = function (input, options) { }; util.inherits(Sharp, stream.Duplex); -/** - * Pixel limits. - * @member - * @private - */ -const maximum = { - width: 0x3FFF, - height: 0x3FFF, - pixels: Math.pow(0x3FFF, 2) -}; -Sharp.maximum = maximum; - /** * An EventEmitter that emits a `change` event when a task is either: * - queued, waiting for _libuv_ to provide a worker thread diff --git a/lib/input.js b/lib/input.js index 9687da44..fd484a10 100644 --- a/lib/input.js +++ b/lib/input.js @@ -35,8 +35,8 @@ function _createInputDescriptor (input, inputOptions, containerOptions) { if (is.defined(inputOptions.raw)) { if ( is.object(inputOptions.raw) && - is.integer(inputOptions.raw.width) && is.inRange(inputOptions.raw.width, 1, this.constructor.maximum.width) && - is.integer(inputOptions.raw.height) && is.inRange(inputOptions.raw.height, 1, this.constructor.maximum.height) && + is.integer(inputOptions.raw.width) && inputOptions.raw.width > 0 && + is.integer(inputOptions.raw.height) && inputOptions.raw.height > 0 && is.integer(inputOptions.raw.channels) && is.inRange(inputOptions.raw.channels, 1, 4) ) { inputDescriptor.rawWidth = inputOptions.raw.width; @@ -50,8 +50,8 @@ function _createInputDescriptor (input, inputOptions, containerOptions) { if (is.defined(inputOptions.create)) { if ( is.object(inputOptions.create) && - is.integer(inputOptions.create.width) && is.inRange(inputOptions.create.width, 1, this.constructor.maximum.width) && - is.integer(inputOptions.create.height) && is.inRange(inputOptions.create.height, 1, this.constructor.maximum.height) && + is.integer(inputOptions.create.width) && inputOptions.create.width > 0 && + is.integer(inputOptions.create.height) && inputOptions.create.height > 0 && is.integer(inputOptions.create.channels) && is.inRange(inputOptions.create.channels, 3, 4) && is.defined(inputOptions.create.background) ) { @@ -239,12 +239,12 @@ function limitInputPixels (limit) { if (limit === false) { limit = 0; } else if (limit === true) { - limit = this.constructor.maximum.pixels; + limit = Math.pow(0x3FFF, 2); } if (is.integer(limit) && limit >= 0) { this.options.limitInputPixels = limit; } else { - throw new Error('Invalid pixel limit (0 to ' + this.constructor.maximum.pixels + ') ' + limit); + throw is.invalidParameterError('limitInputPixels', 'integer', limit); } return this; } diff --git a/lib/resize.js b/lib/resize.js index 3ccc6743..dcb0710c 100644 --- a/lib/resize.js +++ b/lib/resize.js @@ -91,8 +91,8 @@ const interpolator = { * // of the image data in inputBuffer * }); * - * @param {Number} [width] - pixels wide the resultant image should be, between 1 and 16383 (0x3FFF). Use `null` or `undefined` to auto-scale the width to match the height. - * @param {Number} [height] - pixels high the resultant image should be, between 1 and 16383. Use `null` or `undefined` to auto-scale the height to match the width. + * @param {Number} [width] - pixels wide the resultant image should be. Use `null` or `undefined` to auto-scale the width to match the height. + * @param {Number} [height] - pixels high the resultant image should be. Use `null` or `undefined` to auto-scale the height to match the width. * @param {Object} [options] * @param {String} [options.kernel='lanczos3'] - the kernel to use for image reduction. * @param {String} [options.interpolator='bicubic'] - the interpolator to use for image enlargement. @@ -103,19 +103,19 @@ const interpolator = { */ function resize (width, height, options) { if (is.defined(width)) { - if (is.integer(width) && is.inRange(width, 1, this.constructor.maximum.width)) { + if (is.integer(width) && width > 0) { this.options.width = width; } else { - throw is.invalidParameterError('width', `integer between 1 and ${this.constructor.maximum.width}`, width); + throw is.invalidParameterError('width', 'positive integer', width); } } else { this.options.width = -1; } if (is.defined(height)) { - if (is.integer(height) && is.inRange(height, 1, this.constructor.maximum.height)) { + if (is.integer(height) && height > 0) { this.options.height = height; } else { - throw is.invalidParameterError('height', `integer between 1 and ${this.constructor.maximum.height}`, height); + throw is.invalidParameterError('height', 'positive integer', height); } } else { this.options.height = -1; diff --git a/src/common.cc b/src/common.cc index 70d4ba8d..dff45526 100644 --- a/src/common.cc +++ b/src/common.cc @@ -347,6 +347,25 @@ namespace sharp { image.set(VIPS_META_RESOLUTION_UNIT, "in"); } + /* + Check the proposed format supports the current dimensions. + */ + void AssertImageTypeDimensions(VImage image, ImageType const imageType) { + if (imageType == ImageType::JPEG) { + if (image.width() > 65535 || image.height() > 65535) { + throw vips::VError("Processed image is too large for the JPEG format"); + } + } else if (imageType == ImageType::PNG) { + if (image.width() > 2147483647 || image.height() > 2147483647) { + throw vips::VError("Processed image is too large for the PNG format"); + } + } else if (imageType == ImageType::WEBP) { + if (image.width() > 16383 || image.height() > 16383) { + throw vips::VError("Processed image is too large for the WebP format"); + } + } + } + /* Called when a Buffer undergoes GC, required to support mixed runtime libraries in Windows */ diff --git a/src/common.h b/src/common.h index a6cf922a..7779d69c 100644 --- a/src/common.h +++ b/src/common.h @@ -184,6 +184,11 @@ namespace sharp { */ void SetDensity(VImage image, const int density); + /* + Check the proposed format supports the current dimensions. + */ + void AssertImageTypeDimensions(VImage image, ImageType const imageType); + /* Called when a Buffer undergoes GC, required to support mixed runtime libraries in Windows */ diff --git a/src/pipeline.cc b/src/pipeline.cc index 6a54e1a2..f31540cb 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -738,6 +738,7 @@ class PipelineWorker : public Nan::AsyncWorker { // Buffer output if (baton->formatOut == "jpeg" || (baton->formatOut == "input" && inputImageType == ImageType::JPEG)) { // Write JPEG to buffer + sharp::AssertImageTypeDimensions(image, ImageType::JPEG); VipsArea *area = VIPS_AREA(image.jpegsave_buffer(VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->jpegQuality) @@ -759,11 +760,12 @@ class PipelineWorker : public Nan::AsyncWorker { } } else if (baton->formatOut == "png" || (baton->formatOut == "input" && (inputImageType == ImageType::PNG || inputImageType == ImageType::GIF || inputImageType == ImageType::SVG))) { + // Write PNG to buffer + sharp::AssertImageTypeDimensions(image, ImageType::PNG); // Strip profile if (!baton->withMetadata) { vips_image_remove(image.get_image(), VIPS_META_ICC_NAME); } - // Write PNG to buffer VipsArea *area = VIPS_AREA(image.pngsave_buffer(VImage::option() ->set("interlace", baton->pngProgressive) ->set("compression", baton->pngCompressionLevel) @@ -775,6 +777,7 @@ class PipelineWorker : public Nan::AsyncWorker { baton->formatOut = "png"; } else if (baton->formatOut == "webp" || (baton->formatOut == "input" && inputImageType == ImageType::WEBP)) { // Write WEBP to buffer + sharp::AssertImageTypeDimensions(image, ImageType::WEBP); VipsArea *area = VIPS_AREA(image.webpsave_buffer(VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->webpQuality) @@ -787,11 +790,14 @@ class PipelineWorker : public Nan::AsyncWorker { vips_area_unref(area); baton->formatOut = "webp"; } else if (baton->formatOut == "tiff" || (baton->formatOut == "input" && inputImageType == ImageType::TIFF)) { + // Write TIFF to buffer + if (baton->tiffCompression == VIPS_FOREIGN_TIFF_COMPRESSION_JPEG) { + sharp::AssertImageTypeDimensions(image, ImageType::JPEG); + } // Cast pixel values to float, if required if (baton->tiffPredictor == VIPS_FOREIGN_TIFF_PREDICTOR_FLOAT) { image = image.cast(VIPS_FORMAT_FLOAT); } - // Write TIFF to buffer VipsArea *area = VIPS_AREA(image.tiffsave_buffer(VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->tiffQuality) @@ -844,6 +850,7 @@ class PipelineWorker : public Nan::AsyncWorker { !(isJpeg || isPng || isWebp || isTiff || isDz || isDzZip || isV); if (baton->formatOut == "jpeg" || isJpeg || (matchInput && inputImageType == ImageType::JPEG)) { // Write JPEG to file + sharp::AssertImageTypeDimensions(image, ImageType::JPEG); image.jpegsave(const_cast(baton->fileOut.data()), VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->jpegQuality) @@ -857,11 +864,12 @@ class PipelineWorker : public Nan::AsyncWorker { baton->channels = std::min(baton->channels, 3); } else if (baton->formatOut == "png" || isPng || (matchInput && (inputImageType == ImageType::PNG || inputImageType == ImageType::GIF || inputImageType == ImageType::SVG))) { + // Write PNG to file + sharp::AssertImageTypeDimensions(image, ImageType::PNG); // Strip profile if (!baton->withMetadata) { vips_image_remove(image.get_image(), VIPS_META_ICC_NAME); } - // Write PNG to file image.pngsave(const_cast(baton->fileOut.data()), VImage::option() ->set("interlace", baton->pngProgressive) ->set("compression", baton->pngCompressionLevel) @@ -869,6 +877,7 @@ class PipelineWorker : public Nan::AsyncWorker { baton->formatOut = "png"; } else if (baton->formatOut == "webp" || isWebp || (matchInput && inputImageType == ImageType::WEBP)) { // Write WEBP to file + AssertImageTypeDimensions(image, ImageType::WEBP); image.webpsave(const_cast(baton->fileOut.data()), VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->webpQuality) @@ -877,11 +886,14 @@ class PipelineWorker : public Nan::AsyncWorker { ->set("alpha_q", baton->webpAlphaQuality)); baton->formatOut = "webp"; } else if (baton->formatOut == "tiff" || isTiff || (matchInput && inputImageType == ImageType::TIFF)) { + // Write TIFF to file + if (baton->tiffCompression == VIPS_FOREIGN_TIFF_COMPRESSION_JPEG) { + sharp::AssertImageTypeDimensions(image, ImageType::JPEG); + } // Cast pixel values to float, if required if (baton->tiffPredictor == VIPS_FOREIGN_TIFF_PREDICTOR_FLOAT) { image = image.cast(VIPS_FORMAT_FLOAT); } - // Write TIFF to file image.tiffsave(const_cast(baton->fileOut.data()), VImage::option() ->set("strip", !baton->withMetadata) ->set("Q", baton->tiffQuality) diff --git a/test/unit/resize.js b/test/unit/resize.js index 1a79a1a7..58aafd47 100644 --- a/test/unit/resize.js +++ b/test/unit/resize.js @@ -66,37 +66,47 @@ describe('Resize dimensions', function () { it('Invalid width - NaN', function () { assert.throws(function () { sharp().resize('spoons', 240); - }, /Expected integer between 1 and 16383 for width but received spoons of type string/); + }, /Expected positive integer for width but received spoons of type string/); }); it('Invalid height - NaN', function () { assert.throws(function () { sharp().resize(320, 'spoons'); - }, /Expected integer between 1 and 16383 for height but received spoons of type string/); + }, /Expected positive integer for height but received spoons of type string/); }); it('Invalid width - float', function () { assert.throws(function () { sharp().resize(1.5, 240); - }, /Expected integer between 1 and 16383 for width but received 1.5 of type number/); + }, /Expected positive integer for width but received 1.5 of type number/); }); it('Invalid height - float', function () { assert.throws(function () { sharp().resize(320, 1.5); - }, /Expected integer between 1 and 16383 for height but received 1.5 of type number/); + }, /Expected positive integer for height but received 1.5 of type number/); }); - it('Invalid width - too large', function () { - assert.throws(function () { - sharp().resize(0x4000, 240); - }, /Expected integer between 1 and 16383 for width but received 16384 of type number/); + it('Invalid width - too large', function (done) { + sharp(fixtures.inputJpg) + .resize(0x4000, 1) + .webp() + .toBuffer(function (err) { + assert.strictEqual(true, err instanceof Error); + assert.strictEqual('Processed image is too large for the WebP format', err.message); + done(); + }); }); - it('Invalid height - too large', function () { - assert.throws(function () { - sharp().resize(320, 0x4000); - }, /Expected integer between 1 and 16383 for height but received 16384 of type number/); + it('Invalid height - too large', function (done) { + sharp(fixtures.inputJpg) + .resize(1, 0x4000) + .webp() + .toBuffer(function (err) { + assert.strictEqual(true, err instanceof Error); + assert.strictEqual('Processed image is too large for the WebP format', err.message); + done(); + }); }); it('WebP shrink-on-load rounds to zero, ensure recalculation is correct', function (done) {