From 6ac47c1ef8acf1b2d153866b5651ad21a214dff7 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Sun, 28 Jun 2015 23:35:40 +0100 Subject: [PATCH] Add raw EXIF data to metadata response Copy metadata input buffer to match pipeline Prevents possible metadata segfault under load --- README.md | 1 + index.js | 8 ++++---- package.json | 11 ++++++----- src/metadata.cc | 43 ++++++++++++++++++++++++++++++++++++++----- src/pipeline.cc | 2 +- test/unit/metadata.js | 8 ++++++++ test/unit/overlay.js | 2 -- 7 files changed, 58 insertions(+), 17 deletions(-) mode change 100644 => 100755 test/unit/overlay.js diff --git a/README.md b/README.md index 86b98d1b..ac0376b1 100755 --- a/README.md +++ b/README.md @@ -343,6 +343,7 @@ Fast access to image metadata without decoding any compressed image data. * `hasProfile`: Boolean indicating the presence of an embedded ICC profile * `hasAlpha`: Boolean indicating the presence of an alpha transparency channel * `orientation`: Number value of the EXIF Orientation header, if present +* `exif`: Buffer containing raw EXIF data, if present A Promises/A+ promise is returned when `callback` is not provided. diff --git a/index.js b/index.js index e7532eec..8b1be523 100755 --- a/index.js +++ b/index.js @@ -737,22 +737,22 @@ Sharp.prototype.metadata = function(callback) { if (this.options.streamIn) { return new BluebirdPromise(function(resolve, reject) { that.on('finish', function() { - sharp.metadata(that.options, function(err, data) { + sharp.metadata(that.options, function(err, metadata) { if (err) { reject(err); } else { - resolve(data); + resolve(metadata); } }); }); }); } else { return new BluebirdPromise(function(resolve, reject) { - sharp.metadata(that.options, function(err, data) { + sharp.metadata(that.options, function(err, metadata) { if (err) { reject(err); } else { - resolve(data); + resolve(metadata); } }); }); diff --git a/package.json b/package.json index 8b549524..b75cd70a 100755 --- a/package.json +++ b/package.json @@ -45,19 +45,20 @@ "vips" ], "dependencies": { - "bluebird": "^2.9.27", - "color": "^0.8.0", + "bluebird": "^2.9.30", + "color": "^0.9.0", "nan": "^1.8.4", "semver": "^4.3.6" }, "devDependencies": { - "async": "^1.1.0", + "async": "^1.2.1", "coveralls": "^2.11.2", - "istanbul": "^0.3.14", + "exif-reader": "1.0.0", + "istanbul": "^0.3.17", "mocha": "^2.2.5", "mocha-jshint": "^2.2.3", "node-cpplint": "^0.4.0", - "rimraf": "^2.3.4" + "rimraf": "^2.4.0" }, "license": "Apache-2.0", "engines": { diff --git a/src/metadata.cc b/src/metadata.cc index a8ceab42..e86df506 100755 --- a/src/metadata.cc +++ b/src/metadata.cc @@ -27,7 +27,7 @@ using sharp::counterQueue; struct MetadataBaton { // Input std::string fileIn; - void* bufferIn; + char *bufferIn; size_t bufferInLength; // Output std::string format; @@ -38,13 +38,26 @@ struct MetadataBaton { bool hasProfile; bool hasAlpha; int orientation; + char *exif; + size_t exifLength; std::string err; MetadataBaton(): bufferInLength(0), - orientation(0) {} + orientation(0), + exifLength(0) {} }; +/* + Delete input char[] buffer and notify V8 of memory deallocation + Used as the callback function for the "postclose" signal +*/ +static void DeleteBuffer(VipsObject *object, char *buffer) { + if (buffer != NULL) { + delete[] buffer; + } +} + class MetadataWorker : public NanAsyncWorker { public: @@ -57,17 +70,22 @@ class MetadataWorker : public NanAsyncWorker { ImageType imageType = ImageType::UNKNOWN; VipsImage *image = NULL; - if (baton->bufferInLength > 1) { + if (baton->bufferInLength > 0) { // From buffer imageType = DetermineImageType(baton->bufferIn, baton->bufferInLength); if (imageType != ImageType::UNKNOWN) { image = InitImage(baton->bufferIn, baton->bufferInLength, VIPS_ACCESS_RANDOM); - if (image == NULL) { + if (image != NULL) { + // Listen for "postclose" signal to delete input buffer + g_signal_connect(image, "postclose", G_CALLBACK(DeleteBuffer), baton->bufferIn); + } else { (baton->err).append("Input buffer has corrupt header"); imageType = ImageType::UNKNOWN; + DeleteBuffer(NULL, baton->bufferIn); } } else { (baton->err).append("Input buffer contains unsupported image format"); + DeleteBuffer(NULL, baton->bufferIn); } } else { // From file @@ -102,6 +120,16 @@ class MetadataWorker : public NanAsyncWorker { // Derived attributes baton->hasAlpha = HasAlpha(image); baton->orientation = ExifOrientation(image); + // EXIF + if (vips_image_get_typeof(image, VIPS_META_EXIF_NAME) == VIPS_TYPE_BLOB) { + void* exif; + size_t exifLength; + if (!vips_image_get_blob(image, VIPS_META_EXIF_NAME, &exif, &exifLength)) { + baton->exifLength = exifLength; + baton->exif = new char[exifLength]; + memcpy(baton->exif, exif, exifLength); + } + } // Drop image reference g_object_unref(image); } @@ -130,6 +158,9 @@ class MetadataWorker : public NanAsyncWorker { if (baton->orientation > 0) { info->Set(NanNew("orientation"), NanNew(baton->orientation)); } + if (baton->exifLength > 0) { + info->Set(NanNew("exif"), NanBufferUse(baton->exif, baton->exifLength)); + } argv[1] = info; } delete baton; @@ -157,8 +188,10 @@ NAN_METHOD(metadata) { // Input Buffer object if (options->Get(NanNew("bufferIn"))->IsObject()) { Local buffer = options->Get(NanNew("bufferIn"))->ToObject(); + // Take a copy of the input Buffer to avoid problems with V8 heap compaction baton->bufferInLength = node::Buffer::Length(buffer); - baton->bufferIn = node::Buffer::Data(buffer); + baton->bufferIn = new char[baton->bufferInLength]; + memcpy(baton->bufferIn, node::Buffer::Data(buffer), baton->bufferInLength); } // Join queue for worker thread diff --git a/src/pipeline.cc b/src/pipeline.cc index 99940a6f..b313355e 100755 --- a/src/pipeline.cc +++ b/src/pipeline.cc @@ -184,7 +184,7 @@ class PipelineWorker : public NanAsyncWorker { // Input ImageType inputImageType = ImageType::UNKNOWN; VipsImage *image = NULL; - if (baton->bufferInLength > 1) { + if (baton->bufferInLength > 0) { // From buffer inputImageType = DetermineImageType(baton->bufferIn, baton->bufferInLength); if (inputImageType != ImageType::UNKNOWN) { diff --git a/test/unit/metadata.js b/test/unit/metadata.js index 0481bf23..48d7099e 100755 --- a/test/unit/metadata.js +++ b/test/unit/metadata.js @@ -2,6 +2,7 @@ var fs = require('fs'); var assert = require('assert'); +var exifReader = require('exif-reader'); var sharp = require('../../index'); var fixtures = require('../fixtures'); @@ -21,6 +22,7 @@ describe('Image metadata', function() { assert.strictEqual(false, metadata.hasProfile); assert.strictEqual(false, metadata.hasAlpha); assert.strictEqual('undefined', typeof metadata.orientation); + assert.strictEqual('undefined', typeof metadata.exif); done(); }); }); @@ -36,6 +38,12 @@ describe('Image metadata', function() { assert.strictEqual(true, metadata.hasProfile); assert.strictEqual(false, metadata.hasAlpha); assert.strictEqual(8, metadata.orientation); + assert.strictEqual('object', typeof metadata.exif); + assert.strictEqual(true, metadata.exif instanceof Buffer); + var exif = exifReader(metadata.exif); + assert.strictEqual('object', typeof exif); + assert.strictEqual('object', typeof exif.image); + assert.strictEqual('number', typeof exif.image.XResolution); done(); }); }); diff --git a/test/unit/overlay.js b/test/unit/overlay.js old mode 100644 new mode 100755 index 8a3e4057..82641e91 --- a/test/unit/overlay.js +++ b/test/unit/overlay.js @@ -147,7 +147,6 @@ describe('Overlays', function() { sharp(fixtures.inputJpg) .overlayWith(fixtures.inputPngWithGreyAlpha) .toBuffer(function(error) { - console.dir(error); assert.strictEqual(true, error instanceof Error); done(); }); @@ -157,7 +156,6 @@ describe('Overlays', function() { sharp(fixtures.inputPngOverlayLayer1) .overlayWith(fixtures.inputJpg) .toBuffer(function(error) { - console.dir(error); assert.strictEqual(true, error instanceof Error); done(); });