From 926572b41ea363ede57189128441a42fb77f66f4 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Mon, 4 Apr 2022 12:27:45 +0100 Subject: [PATCH] Control sensitivity to invalid images via failOn Deprecates failOnError, equivalent to failOn=warning --- docs/api-composite.md | 2 +- docs/api-constructor.md | 47 +++++++++--------- docs/changelog.md | 2 + lib/composite.js | 2 +- lib/constructor.js | 7 +-- lib/input.js | 20 +++++--- src/common.cc | 8 +-- src/common.h | 4 +- src/pipeline.cc | 4 +- test/unit/{failOnError.js => failOn.js} | 66 +++++++++++++++---------- 10 files changed, 91 insertions(+), 71 deletions(-) rename test/unit/{failOnError.js => failOn.js} (52%) diff --git a/docs/api-composite.md b/docs/api-composite.md index 66df143f..93b5678e 100644 --- a/docs/api-composite.md +++ b/docs/api-composite.md @@ -42,7 +42,7 @@ and [https://www.cairographics.org/operators/][2] * `images[].raw.height` **[Number][7]?** * `images[].raw.channels` **[Number][7]?** * `images[].animated` **[boolean][9]** Set to `true` to read all frames/pages of an animated image. (optional, default `false`) - * `images[].failOnError` **[boolean][9]** @see [constructor parameters][10] (optional, default `true`) + * `images[].failOn` **[string][6]** @see [constructor parameters][10] (optional, default `'warning'`) * `images[].limitInputPixels` **([number][7] | [boolean][9])** @see [constructor parameters][10] (optional, default `268402689`) ### Examples diff --git a/docs/api-constructor.md b/docs/api-constructor.md index 12ac6391..b690c539 100644 --- a/docs/api-constructor.md +++ b/docs/api-constructor.md @@ -20,38 +20,37 @@ Implements the [stream.Duplex][1] class. JPEG, PNG, WebP, AVIF, GIF, SVG, TIFF or raw pixel image data can be streamed into the object when not present. * `options` **[Object][13]?** if present, is an Object with optional attributes. - * `options.failOnError` **[boolean][14]** by default halt processing and raise an error when loading invalid images. - Set this flag to `false` if you'd rather apply a "best effort" to decode images, even if the data is corrupt or invalid. (optional, default `true`) - * `options.limitInputPixels` **([number][15] | [boolean][14])** Do not process input images where the number of pixels + * `options.failOn` **[string][12]** level of sensitivity to invalid images, one of (in order of sensitivity): 'none' (least), 'truncated', 'error' or 'warning' (most), highers level imply lower levels. (optional, default `'warning'`) + * `options.limitInputPixels` **([number][14] | [boolean][15])** Do not process input images where the number of pixels (width x height) exceeds this limit. Assumes image dimensions contained in the input metadata can be trusted. An integral Number of pixels, zero or false to remove limit, true to use default limit of 268402689 (0x3FFF x 0x3FFF). (optional, default `268402689`) - * `options.unlimited` **[boolean][14]** Set this to `true` to remove safety features that help prevent memory exhaustion (SVG, PNG). (optional, default `false`) - * `options.sequentialRead` **[boolean][14]** Set this to `true` to use sequential rather than random access where possible. + * `options.unlimited` **[boolean][15]** Set this to `true` to remove safety features that help prevent memory exhaustion (SVG, PNG). (optional, default `false`) + * `options.sequentialRead` **[boolean][15]** Set this to `true` to use sequential rather than random access where possible. This can reduce memory usage and might improve performance on some systems. (optional, default `false`) - * `options.density` **[number][15]** number representing the DPI for vector images in the range 1 to 100000. (optional, default `72`) - * `options.pages` **[number][15]** number of pages to extract for multi-page input (GIF, WebP, AVIF, TIFF, PDF), use -1 for all pages. (optional, default `1`) - * `options.page` **[number][15]** page number to start extracting from for multi-page input (GIF, WebP, AVIF, TIFF, PDF), zero based. (optional, default `0`) - * `options.subifd` **[number][15]** subIFD (Sub Image File Directory) to extract for OME-TIFF, defaults to main image. (optional, default `-1`) - * `options.level` **[number][15]** level to extract from a multi-level input (OpenSlide), zero based. (optional, default `0`) - * `options.animated` **[boolean][14]** Set to `true` to read all frames/pages of an animated image (equivalent of setting `pages` to `-1`). (optional, default `false`) + * `options.density` **[number][14]** number representing the DPI for vector images in the range 1 to 100000. (optional, default `72`) + * `options.pages` **[number][14]** number of pages to extract for multi-page input (GIF, WebP, AVIF, TIFF, PDF), use -1 for all pages. (optional, default `1`) + * `options.page` **[number][14]** page number to start extracting from for multi-page input (GIF, WebP, AVIF, TIFF, PDF), zero based. (optional, default `0`) + * `options.subifd` **[number][14]** subIFD (Sub Image File Directory) to extract for OME-TIFF, defaults to main image. (optional, default `-1`) + * `options.level` **[number][14]** level to extract from a multi-level input (OpenSlide), zero based. (optional, default `0`) + * `options.animated` **[boolean][15]** Set to `true` to read all frames/pages of an animated image (equivalent of setting `pages` to `-1`). (optional, default `false`) * `options.raw` **[Object][13]?** describes raw pixel input image data. See `raw()` for pixel ordering. - * `options.raw.width` **[number][15]?** integral number of pixels wide. - * `options.raw.height` **[number][15]?** integral number of pixels high. - * `options.raw.channels` **[number][15]?** integral number of channels, between 1 and 4. - * `options.raw.premultiplied` **[boolean][14]?** specifies that the raw input has already been premultiplied, set to `true` + * `options.raw.width` **[number][14]?** integral number of pixels wide. + * `options.raw.height` **[number][14]?** integral number of pixels high. + * `options.raw.channels` **[number][14]?** integral number of channels, between 1 and 4. + * `options.raw.premultiplied` **[boolean][15]?** specifies that the raw input has already been premultiplied, set to `true` to avoid sharp premultiplying the image. (optional, default `false`) * `options.create` **[Object][13]?** describes a new image to be created. - * `options.create.width` **[number][15]?** integral number of pixels wide. - * `options.create.height` **[number][15]?** integral number of pixels high. - * `options.create.channels` **[number][15]?** integral number of channels, either 3 (RGB) or 4 (RGBA). + * `options.create.width` **[number][14]?** integral number of pixels wide. + * `options.create.height` **[number][14]?** integral number of pixels high. + * `options.create.channels` **[number][14]?** integral number of channels, either 3 (RGB) or 4 (RGBA). * `options.create.background` **([string][12] | [Object][13])?** parsed by the [color][16] module to extract values for red, green, blue and alpha. * `options.create.noise` **[Object][13]?** describes a noise to be created. * `options.create.noise.type` **[string][12]?** type of generated noise, currently only `gaussian` is supported. - * `options.create.noise.mean` **[number][15]?** mean of pixels in generated noise. - * `options.create.noise.sigma` **[number][15]?** standard deviation of pixels in generated noise. + * `options.create.noise.mean` **[number][14]?** mean of pixels in generated noise. + * `options.create.noise.sigma` **[number][14]?** standard deviation of pixels in generated noise. ### Examples @@ -154,9 +153,7 @@ readableStream.pipe(pipeline); // Using Promises to know when the pipeline is complete const fs = require("fs"); const got = require("got"); -const sharpStream = sharp({ - failOnError: false -}); +const sharpStream = sharp({ failOn: 'none' }); const promises = []; @@ -226,9 +223,9 @@ Returns **[Sharp][18]** [13]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object -[14]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean +[14]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number -[15]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number +[15]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean [16]: https://www.npmjs.org/package/color diff --git a/docs/changelog.md b/docs/changelog.md index ed1b3ae1..1516b9b4 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -6,6 +6,8 @@ Requires libvips v8.12.2 ### v0.30.4 - TBD +* Increase control over sensitivity to invalid images via `failOn`, deprecate `failOnError` (equivalent to `failOn: 'warning'`). + * Ensure `create` input image has correct bit depth and colour space. [#3139](https://github.com/lovell/sharp/issues/3139) diff --git a/lib/composite.js b/lib/composite.js index 9ba6b419..7b2c7588 100644 --- a/lib/composite.js +++ b/lib/composite.js @@ -105,7 +105,7 @@ const blend = { * @param {Number} [images[].raw.height] * @param {Number} [images[].raw.channels] * @param {boolean} [images[].animated=false] - Set to `true` to read all frames/pages of an animated image. - * @param {boolean} [images[].failOnError=true] - @see {@link /api-constructor#parameters|constructor parameters} + * @param {string} [images[].failOn='warning'] - @see {@link /api-constructor#parameters|constructor parameters} * @param {number|boolean} [images[].limitInputPixels=268402689] - @see {@link /api-constructor#parameters|constructor parameters} * @returns {Sharp} * @throws {Error} Invalid parameters diff --git a/lib/constructor.js b/lib/constructor.js index 503074b1..eb2c3c4a 100644 --- a/lib/constructor.js +++ b/lib/constructor.js @@ -98,8 +98,7 @@ const debuglog = util.debuglog('sharp'); * a String containing the filesystem path to an JPEG, PNG, WebP, AVIF, GIF, SVG or TIFF image file. * JPEG, PNG, WebP, AVIF, GIF, SVG, TIFF or raw pixel image data can be streamed into the object when not present. * @param {Object} [options] - if present, is an Object with optional attributes. - * @param {boolean} [options.failOnError=true] - by default halt processing and raise an error when loading invalid images. - * Set this flag to `false` if you'd rather apply a "best effort" to decode images, even if the data is corrupt or invalid. + * @param {string} [options.failOn='warning'] - level of sensitivity to invalid images, one of (in order of sensitivity): 'none' (least), 'truncated', 'error' or 'warning' (most), highers level imply lower levels. * @param {number|boolean} [options.limitInputPixels=268402689] - Do not process input images where the number of pixels * (width x height) exceeds this limit. Assumes image dimensions contained in the input metadata can be trusted. * An integral Number of pixels, zero or false to remove limit, true to use default limit of 268402689 (0x3FFF x 0x3FFF). @@ -320,9 +319,7 @@ Object.setPrototypeOf(Sharp, stream.Duplex); * // Using Promises to know when the pipeline is complete * const fs = require("fs"); * const got = require("got"); - * const sharpStream = sharp({ - * failOnError: false - * }); + * const sharpStream = sharp({ failOn: 'none' }); * * const promises = []; * diff --git a/lib/input.js b/lib/input.js index de5cea60..9fb54f0c 100644 --- a/lib/input.js +++ b/lib/input.js @@ -9,9 +9,9 @@ const sharp = require('./sharp'); * @private */ function _inputOptionsFromObject (obj) { - const { raw, density, limitInputPixels, unlimited, sequentialRead, failOnError, animated, page, pages, subifd } = obj; - return [raw, density, limitInputPixels, unlimited, sequentialRead, failOnError, animated, page, pages, subifd].some(is.defined) - ? { raw, density, limitInputPixels, unlimited, sequentialRead, failOnError, animated, page, pages, subifd } + const { raw, density, limitInputPixels, unlimited, sequentialRead, failOn, failOnError, animated, page, pages, subifd } = obj; + return [raw, density, limitInputPixels, unlimited, sequentialRead, failOn, failOnError, animated, page, pages, subifd].some(is.defined) + ? { raw, density, limitInputPixels, unlimited, sequentialRead, failOn, failOnError, animated, page, pages, subifd } : undefined; } @@ -21,7 +21,7 @@ function _inputOptionsFromObject (obj) { */ function _createInputDescriptor (input, inputOptions, containerOptions) { const inputDescriptor = { - failOnError: true, + failOn: 'warning', limitInputPixels: Math.pow(0x3FFF, 2), unlimited: false, sequentialRead: false @@ -56,14 +56,22 @@ function _createInputDescriptor (input, inputOptions, containerOptions) { }`); } if (is.object(inputOptions)) { - // Fail on error + // Deprecated: failOnError if (is.defined(inputOptions.failOnError)) { if (is.bool(inputOptions.failOnError)) { - inputDescriptor.failOnError = inputOptions.failOnError; + inputDescriptor.failOn = inputOptions.failOnError ? 'warning' : 'none'; } else { throw is.invalidParameterError('failOnError', 'boolean', inputOptions.failOnError); } } + // failOn + if (is.defined(inputOptions.failOn)) { + if (is.string(inputOptions.failOn) && is.inArray(inputOptions.failOn, ['none', 'truncated', 'error', 'warning'])) { + inputDescriptor.failOn = inputOptions.failOn; + } else { + throw is.invalidParameterError('failOn', 'one of: none, truncated, error, warning', inputOptions.failOn); + } + } // Density if (is.defined(inputOptions.density)) { if (is.inRange(inputOptions.density, 1, 100000)) { diff --git a/src/common.cc b/src/common.cc index 0ec61c93..a66ecf77 100644 --- a/src/common.cc +++ b/src/common.cc @@ -85,7 +85,9 @@ namespace sharp { descriptor->buffer = buffer.Data(); descriptor->isBuffer = TRUE; } - descriptor->failOnError = AttrAsBool(input, "failOnError"); + descriptor->failOn = static_cast( + vips_enum_from_nick(nullptr, VIPS_TYPE_FAIL_ON, + AttrAsStr(input, "failOn").data())); // Density for vector-based input if (HasAttr(input, "density")) { descriptor->density = AttrAsDouble(input, "density"); @@ -329,7 +331,7 @@ namespace sharp { try { vips::VOption *option = VImage::option() ->set("access", descriptor->access) - ->set("fail", descriptor->failOnError); + ->set("fail_on", descriptor->failOn); if (descriptor->unlimited && (imageType == ImageType::SVG || imageType == ImageType::PNG)) { option->set("unlimited", TRUE); } @@ -403,7 +405,7 @@ namespace sharp { try { vips::VOption *option = VImage::option() ->set("access", descriptor->access) - ->set("fail", descriptor->failOnError); + ->set("fail_on", descriptor->failOn); if (descriptor->unlimited && (imageType == ImageType::SVG || imageType == ImageType::PNG)) { option->set("unlimited", TRUE); } diff --git a/src/common.h b/src/common.h index 41552dbb..9065065f 100644 --- a/src/common.h +++ b/src/common.h @@ -48,7 +48,7 @@ namespace sharp { std::string name; std::string file; char *buffer; - bool failOnError; + VipsFailOn failOn; int limitInputPixels; bool unlimited; VipsAccess access; @@ -74,7 +74,7 @@ namespace sharp { InputDescriptor(): buffer(nullptr), - failOnError(TRUE), + failOn(VIPS_FAIL_ON_WARNING), limitInputPixels(0x3FFF * 0x3FFF), unlimited(FALSE), access(VIPS_ACCESS_RANDOM), diff --git a/src/pipeline.cc b/src/pipeline.cc index 88e39ec1..269d225b 100644 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -200,7 +200,7 @@ class PipelineWorker : public Napi::AsyncWorker { vips::VOption *option = VImage::option() ->set("access", baton->input->access) ->set("shrink", jpegShrinkOnLoad) - ->set("fail", baton->input->failOnError); + ->set("fail_on", baton->input->failOn); if (baton->input->buffer != nullptr) { // Reload JPEG buffer VipsBlob *blob = vips_blob_new(nullptr, baton->input->buffer, baton->input->bufferLength); @@ -214,7 +214,7 @@ class PipelineWorker : public Napi::AsyncWorker { vips::VOption *option = VImage::option() ->set("access", baton->input->access) ->set("scale", scale) - ->set("fail", baton->input->failOnError); + ->set("fail_on", baton->input->failOn); if (inputImageType == sharp::ImageType::WEBP) { option->set("n", baton->input->pages); option->set("page", baton->input->page); diff --git a/test/unit/failOnError.js b/test/unit/failOn.js similarity index 52% rename from test/unit/failOnError.js rename to test/unit/failOn.js index 54c1f0b7..21b4eb22 100644 --- a/test/unit/failOnError.js +++ b/test/unit/failOn.js @@ -3,56 +3,70 @@ const assert = require('assert'); const fs = require('fs'); -const sharp = require('../../'); +const sharp = require('../../lib'); const fixtures = require('../fixtures'); -describe('failOnError', function () { +describe('failOn', () => { it('handles truncated JPEG', function (done) { - sharp(fixtures.inputJpgTruncated, { failOnError: false }) - .resize(320, 240) + sharp(fixtures.inputJpgTruncated, { failOn: 'none' }) + .resize(32, 24) .toBuffer(function (err, data, info) { if (err) throw err; assert.strictEqual('jpeg', info.format); - assert.strictEqual(320, info.width); - assert.strictEqual(240, info.height); + assert.strictEqual(32, info.width); + assert.strictEqual(24, info.height); fixtures.assertSimilar(fixtures.expected('truncated.jpg'), data, done); }); }); it('handles truncated PNG, emits warnings', function (done) { let isWarningEmitted = false; - sharp(fixtures.inputPngTruncated, { failOnError: false }) + sharp(fixtures.inputPngTruncated, { failOn: 'none' }) .on('warning', function (warning) { assert.ok(warning.includes('not enough data') || warning.includes('end of stream')); isWarningEmitted = true; }) - .resize(320, 240) + .resize(32, 24) .toBuffer(function (err, data, info) { if (err) throw err; assert.strictEqual(true, isWarningEmitted); assert.strictEqual('png', info.format); - assert.strictEqual(320, info.width); - assert.strictEqual(240, info.height); + assert.strictEqual(32, info.width); + assert.strictEqual(24, info.height); done(); }); }); - it('rejects invalid values', function () { - assert.doesNotThrow(function () { - sharp(fixtures.inputJpg, { failOnError: true }); - }); + it('throws for invalid options', () => { + assert.throws( + () => sharp({ failOn: 'zoinks' }), + /Expected one of: none, truncated, error, warning for failOn but received zoinks of type string/ + ); + assert.throws( + () => sharp({ failOn: 1 }), + /Expected one of: none, truncated, error, warning for failOn but received 1 of type number/ + ); + }); - assert.throws(function () { - sharp(fixtures.inputJpg, { failOnError: 'zoinks' }); - }); - - assert.throws(function () { - sharp(fixtures.inputJpg, { failOnError: 1 }); - }); + it('deprecated failOnError', () => { + assert.doesNotThrow( + () => sharp({ failOnError: true }) + ); + assert.doesNotThrow( + () => sharp({ failOnError: false }) + ); + assert.throws( + () => sharp({ failOnError: 'zoinks' }), + /Expected boolean for failOnError but received zoinks of type string/ + ); + assert.throws( + () => sharp({ failOnError: 1 }), + /Expected boolean for failOnError but received 1 of type number/ + ); }); it('returns errors to callback for truncated JPEG', function (done) { - sharp(fixtures.inputJpgTruncated).toBuffer(function (err, data, info) { + sharp(fixtures.inputJpgTruncated, { failOn: 'truncated' }).toBuffer(function (err, data, info) { assert.ok(err.message.includes('VipsJpeg: Premature end of'), err); assert.strictEqual(data, undefined); assert.strictEqual(info, undefined); @@ -61,7 +75,7 @@ describe('failOnError', function () { }); it('returns errors to callback for truncated PNG', function (done) { - sharp(fixtures.inputPngTruncated).toBuffer(function (err, data, info) { + sharp(fixtures.inputPngTruncated, { failOn: 'truncated' }).toBuffer(function (err, data, info) { assert.ok(err.message.includes('read error'), err); assert.strictEqual(data, undefined); assert.strictEqual(info, undefined); @@ -70,7 +84,7 @@ describe('failOnError', function () { }); it('rejects promises for truncated JPEG', function (done) { - sharp(fixtures.inputJpgTruncated) + sharp(fixtures.inputJpgTruncated, { failOn: 'error' }) .toBuffer() .then(() => { throw new Error('Expected rejection'); @@ -80,8 +94,8 @@ describe('failOnError', function () { }); }); - it('handles stream-based input', function () { - const writable = sharp({ failOnError: false }); + it('handles stream-based input', async () => { + const writable = sharp({ failOn: 'none' }).resize(32, 24); fs.createReadStream(fixtures.inputJpgTruncated).pipe(writable); return writable.toBuffer(); });