From ee437c0557c9f3a6a1960d3a8d64e09166d7048d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 7 May 2012 17:04:56 +0200 Subject: [PATCH] zlib: fix error reporting This commit is a back-port of the changes on the master branch. Fixes #3230. --- lib/zlib.js | 70 +++++- src/node_zlib.cc | 257 ++++++++++++++++----- test/simple/test-zlib-invalid-input.js | 38 ++- test/simple/test-zlib-random-byte-pipes.js | 1 + 4 files changed, 297 insertions(+), 69 deletions(-) diff --git a/lib/zlib.js b/lib/zlib.js index 155924f4eb..e1e903986a 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -50,6 +50,22 @@ Object.keys(binding).forEach(function(k) { if (k.match(/^Z/)) exports[k] = binding[k]; }); +// translation table for return codes. +exports.codes = { + Z_OK: binding.Z_OK, + Z_STREAM_END: binding.Z_STREAM_END, + Z_NEED_DICT: binding.Z_NEED_DICT, + Z_ERRNO: binding.Z_ERRNO, + Z_STREAM_ERROR: binding.Z_STREAM_ERROR, + Z_DATA_ERROR: binding.Z_DATA_ERROR, + Z_MEM_ERROR: binding.Z_MEM_ERROR, + Z_BUF_ERROR: binding.Z_BUF_ERROR, + Z_VERSION_ERROR: binding.Z_VERSION_ERROR +}; + +Object.keys(exports.codes).forEach(function(k) { + exports.codes[exports.codes[k]] = k; +}); exports.Deflate = Deflate; exports.Inflate = Inflate; @@ -135,7 +151,7 @@ function zlibBuffer(engine, buffer, callback) { function onEnd() { var buffer; - switch(buffers.length) { + switch (buffers.length) { case 0: buffer = new Buffer(0); break; @@ -169,12 +185,12 @@ function zlibBuffer(engine, buffer, callback) { // minimal 2-byte header function Deflate(opts) { if (!(this instanceof Deflate)) return new Deflate(opts); - Zlib.call(this, opts, binding.Deflate); + Zlib.call(this, opts, binding.DEFLATE); } function Inflate(opts) { if (!(this instanceof Inflate)) return new Inflate(opts); - Zlib.call(this, opts, binding.Inflate); + Zlib.call(this, opts, binding.INFLATE); } @@ -182,12 +198,12 @@ function Inflate(opts) { // gzip - bigger header, same deflate compression function Gzip(opts) { if (!(this instanceof Gzip)) return new Gzip(opts); - Zlib.call(this, opts, binding.Gzip); + Zlib.call(this, opts, binding.GZIP); } function Gunzip(opts) { if (!(this instanceof Gunzip)) return new Gunzip(opts); - Zlib.call(this, opts, binding.Gunzip); + Zlib.call(this, opts, binding.GUNZIP); } @@ -195,19 +211,19 @@ function Gunzip(opts) { // raw - no header function DeflateRaw(opts) { if (!(this instanceof DeflateRaw)) return new DeflateRaw(opts); - Zlib.call(this, opts, binding.DeflateRaw); + Zlib.call(this, opts, binding.DEFLATERAW); } function InflateRaw(opts) { if (!(this instanceof InflateRaw)) return new InflateRaw(opts); - Zlib.call(this, opts, binding.InflateRaw); + Zlib.call(this, opts, binding.INFLATERAW); } // auto-detect header. function Unzip(opts) { if (!(this instanceof Unzip)) return new Unzip(opts); - Zlib.call(this, opts, binding.Unzip); + Zlib.call(this, opts, binding.UNZIP); } @@ -216,7 +232,7 @@ function Unzip(opts) { // true or false if there is anything in the queue when // you call the .write() method. -function Zlib(opts, Binding) { +function Zlib(opts, mode) { this._opts = opts = opts || {}; this._queue = []; this._processing = false; @@ -263,7 +279,23 @@ function Zlib(opts, Binding) { } } - this._binding = new Binding(); + this._binding = new binding.Zlib(mode); + + var self = this; + this._binding.onerror = function(message, errno) { + // there is no way to cleanly recover. + // continuing only obscures problems. + self._binding = null; + self._hadError = true; + self._queue.length = 0; + self._processing = false; + + var error = new Error(message); + error.errno = errno; + error.code = exports.codes[errno]; + self.emit('error', error); + }; + this._binding.init(opts.windowBits || exports.Z_DEFAULT_WINDOWBITS, opts.level || exports.Z_DEFAULT_COMPRESSION, opts.memLevel || exports.Z_DEFAULT_MEMLEVEL, @@ -278,6 +310,8 @@ function Zlib(opts, Binding) { util.inherits(Zlib, stream.Stream); Zlib.prototype.write = function write(chunk, cb) { + if (this._hadError) return true; + if (this._ended) { return this.emit('error', new Error('Cannot write after end')); } @@ -306,12 +340,18 @@ Zlib.prototype.write = function write(chunk, cb) { return empty; }; +Zlib.prototype.reset = function reset() { + return this._binding.reset(); +}; + Zlib.prototype.flush = function flush(cb) { this._flush = binding.Z_SYNC_FLUSH; return this.write(cb); }; Zlib.prototype.end = function end(chunk, cb) { + if (this._hadError) return true; + var self = this; this._ending = true; var ret = this.write(chunk, function() { @@ -323,6 +363,8 @@ Zlib.prototype.end = function end(chunk, cb) { }; Zlib.prototype._process = function() { + if (this._hadError) return; + if (this._processing || this._paused) return; if (this._queue.length === 0) { @@ -360,6 +402,8 @@ Zlib.prototype._process = function() { this._processing = req; function callback(availInAfter, availOutAfter, buffer) { + if (self._hadError) return; + var have = availOutBefore - availOutAfter; assert(have >= 0, 'have should not go down'); @@ -416,6 +460,12 @@ Zlib.prototype.resume = function() { this._process(); }; +Zlib.prototype.destroy = function() { + this.readable = false; + this.writable = false; + this._ended = true; +}; + util.inherits(Deflate, Zlib); util.inherits(Inflate, Zlib); util.inherits(Gzip, Zlib); diff --git a/src/node_zlib.cc b/src/node_zlib.cc index fe7be7412f..e34c5ce1a0 100644 --- a/src/node_zlib.cc +++ b/src/node_zlib.cc @@ -20,15 +20,15 @@ // USE OR OTHER DEALINGS IN THE SOFTWARE. -#include +#include "v8.h" #include #include #include #include -#include -#include -#include +#include "zlib.h" +#include "node.h" +#include "node_buffer.h" namespace node { @@ -36,6 +36,7 @@ using namespace v8; static Persistent callback_sym; +static Persistent onerror_sym; enum node_zlib_mode { DEFLATE = 1, @@ -47,8 +48,6 @@ enum node_zlib_mode { UNZIP }; -template class ZCtx; - void InitZlib(v8::Handle target); @@ -56,18 +55,19 @@ void InitZlib(v8::Handle target); /** * Deflate/Inflate */ -template class ZCtx : public ObjectWrap { +class ZCtx : public ObjectWrap { public: - ZCtx() : ObjectWrap() { - } + ZCtx(node_zlib_mode mode) : ObjectWrap(), dictionary_(NULL), mode_(mode) {} ~ZCtx() { - if (mode == DEFLATE || mode == GZIP || mode == DEFLATERAW) { + if (mode_ == DEFLATE || mode_ == GZIP || mode_ == DEFLATERAW) { (void)deflateEnd(&strm_); - } else if (mode == INFLATE || mode == GUNZIP || mode == INFLATERAW) { + } else if (mode_ == INFLATE || mode_ == GUNZIP || mode_ == INFLATERAW) { (void)inflateEnd(&strm_); } + + if (dictionary_ != NULL) delete[] dictionary_; } // write(flush, in, in_off, in_len, out, out_off, out_len) @@ -75,7 +75,7 @@ template class ZCtx : public ObjectWrap { HandleScope scope; assert(args.Length() == 7); - ZCtx *ctx = ObjectWrap::Unwrap< ZCtx >(args.This()); + ZCtx *ctx = ObjectWrap::Unwrap(args.This()); assert(ctx->init_done_ && "write before init"); assert(!ctx->write_in_progress_ && "write already in progress"); @@ -124,8 +124,8 @@ template class ZCtx : public ObjectWrap { uv_queue_work(uv_default_loop(), work_req, - ZCtx::Process, - ZCtx::After); + ZCtx::Process, + ZCtx::After); ctx->Ref(); @@ -138,28 +138,46 @@ template class ZCtx : public ObjectWrap { // for a single write() call, until all of the input bytes have // been consumed. static void Process(uv_work_t* work_req) { - ZCtx *ctx = container_of(work_req, ZCtx, work_req_); + ZCtx *ctx = container_of(work_req, ZCtx, work_req_); // If the avail_out is left at 0, then it means that it ran out // of room. If there was avail_out left over, then it means // that all of the input was consumed. - int err; - switch (mode) { + switch (ctx->mode_) { case DEFLATE: case GZIP: case DEFLATERAW: - err = deflate(&ctx->strm_, ctx->flush_); + ctx->err_ = deflate(&ctx->strm_, ctx->flush_); break; case UNZIP: case INFLATE: case GUNZIP: case INFLATERAW: - err = inflate(&ctx->strm_, ctx->flush_); + ctx->err_ = inflate(&ctx->strm_, ctx->flush_); + + // If data was encoded with dictionary + if (ctx->err_ == Z_NEED_DICT) { + assert(ctx->dictionary_ != NULL && "Stream has no dictionary"); + if (ctx->dictionary_ != NULL) { + + // Load it + ctx->err_ = inflateSetDictionary(&ctx->strm_, + ctx->dictionary_, + ctx->dictionary_len_); + assert(ctx->err_ == Z_OK && "Failed to set dictionary"); + if (ctx->err_ == Z_OK) { + + // And try to decode again + ctx->err_ = inflate(&ctx->strm_, ctx->flush_); + } + } + } break; default: assert(0 && "wtf?"); } - assert(err != Z_STREAM_ERROR); + + // pass any errors back to the main thread to deal with. // now After will emit the output, and // either schedule another call to Process, @@ -169,7 +187,20 @@ template class ZCtx : public ObjectWrap { // v8 land! static void After(uv_work_t* work_req) { HandleScope scope; - ZCtx *ctx = container_of(work_req, ZCtx, work_req_); + ZCtx *ctx = container_of(work_req, ZCtx, work_req_); + + // Acceptable error states depend on the type of zlib stream. + switch (ctx->err_) { + case Z_OK: + case Z_STREAM_END: + case Z_BUF_ERROR: + // normal statuses, not fatal + break; + default: + // something else. + ZCtx::Error(ctx, "Zlib error"); + return; + } Local avail_out = Integer::New(ctx->strm_.avail_out); Local avail_in = Integer::New(ctx->strm_.avail_in); @@ -185,9 +216,37 @@ template class ZCtx : public ObjectWrap { ctx->Unref(); } + static void Error(ZCtx *ctx, const char *msg_) { + const char *msg; + if (ctx->strm_.msg != NULL) { + msg = ctx->strm_.msg; + } else { + msg = msg_; + } + + assert(ctx->handle_->Get(onerror_sym)->IsFunction() && + "Invalid error handler"); + HandleScope scope; + Local args[2] = { String::New(msg), + Local::New(Number::New(ctx->err_)) }; + MakeCallback(ctx->handle_, "onerror", ARRAY_SIZE(args), args); + + // no hope of rescue. + ctx->Unref(); + } + static Handle New(const Arguments& args) { HandleScope scope; - ZCtx *ctx = new ZCtx(); + if (args.Length() < 1 || !args[0]->IsInt32()) { + return ThrowException(Exception::TypeError(String::New("Bad argument"))); + } + node_zlib_mode mode = (node_zlib_mode) args[0]->Int32Value(); + + if (mode < DEFLATE || mode > UNZIP) { + return ThrowException(Exception::TypeError(String::New("Bad argument"))); + } + + ZCtx *ctx = new ZCtx(mode); ctx->Wrap(args.This()); return args.This(); } @@ -196,10 +255,10 @@ template class ZCtx : public ObjectWrap { static Handle Init(const Arguments& args) { HandleScope scope; - assert(args.Length() == 4 && - "init(windowBits, level, memLevel, strategy)"); + assert((args.Length() == 4 || args.Length() == 5) && + "init(windowBits, level, memLevel, strategy, [dictionary])"); - ZCtx *ctx = ObjectWrap::Unwrap< ZCtx >(args.This()); + ZCtx *ctx = ObjectWrap::Unwrap(args.This()); int windowBits = args[0]->Uint32Value(); assert((windowBits >= 8 && windowBits <= 15) && "invalid windowBits"); @@ -217,12 +276,35 @@ template class ZCtx : public ObjectWrap { strategy == Z_FIXED || strategy == Z_DEFAULT_STRATEGY) && "invalid strategy"); - Init(ctx, level, windowBits, memLevel, strategy); + char* dictionary = NULL; + size_t dictionary_len = 0; + if (args.Length() >= 5 && Buffer::HasInstance(args[4])) { + Local dictionary_ = args[4]->ToObject(); + + dictionary_len = Buffer::Length(dictionary_); + dictionary = new char[dictionary_len]; + + memcpy(dictionary, Buffer::Data(dictionary_), dictionary_len); + } + + Init(ctx, level, windowBits, memLevel, strategy, + dictionary, dictionary_len); + SetDictionary(ctx); + return Undefined(); + } + + static Handle Reset(const Arguments &args) { + HandleScope scope; + + ZCtx *ctx = ObjectWrap::Unwrap(args.This()); + + Reset(ctx); + SetDictionary(ctx); return Undefined(); } static void Init(ZCtx *ctx, int level, int windowBits, int memLevel, - int strategy) { + int strategy, char* dictionary, size_t dictionary_len) { ctx->level_ = level; ctx->windowBits_ = windowBits; ctx->memLevel_ = memLevel; @@ -234,43 +316,93 @@ template class ZCtx : public ObjectWrap { ctx->flush_ = Z_NO_FLUSH; - if (mode == GZIP || mode == GUNZIP) { + ctx->err_ = Z_OK; + + if (ctx->mode_ == GZIP || ctx->mode_ == GUNZIP) { ctx->windowBits_ += 16; } - if (mode == UNZIP) { + if (ctx->mode_ == UNZIP) { ctx->windowBits_ += 32; } - if (mode == DEFLATERAW || mode == INFLATERAW) { + if (ctx->mode_ == DEFLATERAW || ctx->mode_ == INFLATERAW) { ctx->windowBits_ *= -1; } - int err; - switch (mode) { + switch (ctx->mode_) { case DEFLATE: case GZIP: case DEFLATERAW: - err = deflateInit2(&(ctx->strm_), - ctx->level_, - Z_DEFLATED, - ctx->windowBits_, - ctx->memLevel_, - ctx->strategy_); + ctx->err_ = deflateInit2(&ctx->strm_, + ctx->level_, + Z_DEFLATED, + ctx->windowBits_, + ctx->memLevel_, + ctx->strategy_); break; case INFLATE: case GUNZIP: case INFLATERAW: case UNZIP: - err = inflateInit2(&(ctx->strm_), ctx->windowBits_); + ctx->err_ = inflateInit2(&ctx->strm_, ctx->windowBits_); break; default: assert(0 && "wtf?"); } + if (ctx->err_ != Z_OK) { + ZCtx::Error(ctx, "Init error"); + } + + + ctx->dictionary_ = reinterpret_cast(dictionary); + ctx->dictionary_len_ = dictionary_len; + ctx->write_in_progress_ = false; ctx->init_done_ = true; - assert(err == Z_OK); + } + + static void SetDictionary(ZCtx* ctx) { + if (ctx->dictionary_ == NULL) return; + + ctx->err_ = Z_OK; + + switch (ctx->mode_) { + case DEFLATE: + case DEFLATERAW: + ctx->err_ = deflateSetDictionary(&ctx->strm_, + ctx->dictionary_, + ctx->dictionary_len_); + break; + default: + break; + } + + if (ctx->err_ != Z_OK) { + ZCtx::Error(ctx, "Failed to set dictionary"); + } + } + + static void Reset(ZCtx* ctx) { + ctx->err_ = Z_OK; + + switch (ctx->mode_) { + case DEFLATE: + case DEFLATERAW: + ctx->err_ = deflateReset(&ctx->strm_); + break; + case INFLATE: + case INFLATERAW: + ctx->err_ = inflateReset(&ctx->strm_); + break; + default: + break; + } + + if (ctx->err_ != Z_OK) { + ZCtx::Error(ctx, "Failed to reset stream"); + } } private: @@ -283,6 +415,11 @@ template class ZCtx : public ObjectWrap { int memLevel_; int strategy_; + int err_; + + Bytef* dictionary_; + size_t dictionary_len_; + int flush_; int chunk_size_; @@ -290,31 +427,26 @@ template class ZCtx : public ObjectWrap { bool write_in_progress_; uv_work_t work_req_; + node_zlib_mode mode_; }; -#define NODE_ZLIB_CLASS(mode, name) \ - { \ - Local z = FunctionTemplate::New(ZCtx::New); \ - z->InstanceTemplate()->SetInternalFieldCount(1); \ - NODE_SET_PROTOTYPE_METHOD(z, "write", ZCtx::Write); \ - NODE_SET_PROTOTYPE_METHOD(z, "init", ZCtx::Init); \ - z->SetClassName(String::NewSymbol(name)); \ - target->Set(String::NewSymbol(name), z->GetFunction()); \ - } - void InitZlib(Handle target) { HandleScope scope; - NODE_ZLIB_CLASS(INFLATE, "Inflate") - NODE_ZLIB_CLASS(DEFLATE, "Deflate") - NODE_ZLIB_CLASS(INFLATERAW, "InflateRaw") - NODE_ZLIB_CLASS(DEFLATERAW, "DeflateRaw") - NODE_ZLIB_CLASS(GZIP, "Gzip") - NODE_ZLIB_CLASS(GUNZIP, "Gunzip") - NODE_ZLIB_CLASS(UNZIP, "Unzip") + Local z = FunctionTemplate::New(ZCtx::New); + + z->InstanceTemplate()->SetInternalFieldCount(1); + + NODE_SET_PROTOTYPE_METHOD(z, "write", ZCtx::Write); + NODE_SET_PROTOTYPE_METHOD(z, "init", ZCtx::Init); + NODE_SET_PROTOTYPE_METHOD(z, "reset", ZCtx::Reset); + + z->SetClassName(String::NewSymbol("Zlib")); + target->Set(String::NewSymbol("Zlib"), z->GetFunction()); callback_sym = NODE_PSYMBOL("callback"); + onerror_sym = NODE_PSYMBOL("onerror"); NODE_DEFINE_CONSTANT(target, Z_NO_FLUSH); NODE_DEFINE_CONSTANT(target, Z_PARTIAL_FLUSH); @@ -322,6 +454,8 @@ void InitZlib(Handle target) { NODE_DEFINE_CONSTANT(target, Z_FULL_FLUSH); NODE_DEFINE_CONSTANT(target, Z_FINISH); NODE_DEFINE_CONSTANT(target, Z_BLOCK); + + // return/error codes NODE_DEFINE_CONSTANT(target, Z_OK); NODE_DEFINE_CONSTANT(target, Z_STREAM_END); NODE_DEFINE_CONSTANT(target, Z_NEED_DICT); @@ -331,6 +465,7 @@ void InitZlib(Handle target) { NODE_DEFINE_CONSTANT(target, Z_MEM_ERROR); NODE_DEFINE_CONSTANT(target, Z_BUF_ERROR); NODE_DEFINE_CONSTANT(target, Z_VERSION_ERROR); + NODE_DEFINE_CONSTANT(target, Z_NO_COMPRESSION); NODE_DEFINE_CONSTANT(target, Z_BEST_SPEED); NODE_DEFINE_CONSTANT(target, Z_BEST_COMPRESSION); @@ -342,6 +477,14 @@ void InitZlib(Handle target) { NODE_DEFINE_CONSTANT(target, Z_DEFAULT_STRATEGY); NODE_DEFINE_CONSTANT(target, ZLIB_VERNUM); + NODE_DEFINE_CONSTANT(target, DEFLATE); + NODE_DEFINE_CONSTANT(target, INFLATE); + NODE_DEFINE_CONSTANT(target, GZIP); + NODE_DEFINE_CONSTANT(target, GUNZIP); + NODE_DEFINE_CONSTANT(target, DEFLATERAW); + NODE_DEFINE_CONSTANT(target, INFLATERAW); + NODE_DEFINE_CONSTANT(target, UNZIP); + target->Set(String::NewSymbol("ZLIB_VERSION"), String::New(ZLIB_VERSION)); } diff --git a/test/simple/test-zlib-invalid-input.js b/test/simple/test-zlib-invalid-input.js index 4c581e1c33..f97c5831ad 100644 --- a/test/simple/test-zlib-invalid-input.js +++ b/test/simple/test-zlib-invalid-input.js @@ -27,12 +27,46 @@ var common = require('../common.js'), var nonStringInputs = [1, true, {a: 1}, ['a']]; +console.error('Doing the non-strings'); nonStringInputs.forEach(function(input) { // zlib.gunzip should not throw an error when called with bad input. - assert.doesNotThrow(function () { - zlib.gunzip(input, function (err, buffer) { + assert.doesNotThrow(function() { + zlib.gunzip(input, function(err, buffer) { // zlib.gunzip should pass the error to the callback. assert.ok(err); }); }); }); + +console.error('Doing the unzips'); +// zlib.Unzip classes need to get valid data, or else they'll throw. +var unzips = [ zlib.Unzip(), + zlib.Gunzip(), + zlib.Inflate(), + zlib.InflateRaw() ]; +var hadError = []; +unzips.forEach(function (uz, i) { + console.error('Error for '+uz.constructor.name); + uz.on('error', function(er) { + console.error('Error event', er); + hadError[i] = true; + + // to be friendly to the Stream API, zlib objects just return true and + // ignore data on the floor after an error. It's up to the user to + // catch the 'error' event and do something intelligent. They do not + // emit any more data, however. + assert.equal(uz.write('also invalid'), true); + assert.equal(uz.end(), true); + }); + + uz.on('end', function(er) { + throw new Error('end event should not be emitted '+uz.constructor.name); + }); + + // this will trigger error event + uz.write('this is not valid compressed data.'); +}); + +process.on('exit', function() { + assert.deepEqual(hadError, [true, true, true, true], 'expect 4 errors'); +}); diff --git a/test/simple/test-zlib-random-byte-pipes.js b/test/simple/test-zlib-random-byte-pipes.js index 88838c68ca..f9723cc40d 100644 --- a/test/simple/test-zlib-random-byte-pipes.js +++ b/test/simple/test-zlib-random-byte-pipes.js @@ -19,6 +19,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. +var common = require('../common'); var crypto = require('crypto'); var stream = require('stream'); var Stream = stream.Stream;