Permalink
Browse files

Fix #3052 Handle errors properly in zlib

  • Loading branch information...
1 parent cc15299 commit 01d46f3a204edd56465b970e1c6ca79bcc15de2e @isaacs isaacs committed Apr 1, 2012
Showing with 157 additions and 32 deletions.
  1. +40 −0 lib/zlib.js
  2. +83 −32 src/node_zlib.cc
  3. +34 −0 test/simple/test-zlib-invalid-input.js
View
@@ -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;
@@ -270,6 +286,22 @@ function Zlib(opts, mode) {
}
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,
@@ -285,6 +317,8 @@ function Zlib(opts, mode) {
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'));
}
@@ -323,6 +357,8 @@ Zlib.prototype.flush = function flush(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() {
@@ -334,6 +370,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) {
@@ -371,6 +409,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');
View
@@ -36,6 +36,7 @@ using namespace v8;
static Persistent<String> callback_sym;
+static Persistent<String> onerror_sym;
enum node_zlib_mode {
DEFLATE = 1,
@@ -142,37 +143,41 @@ class ZCtx : public ObjectWrap {
// 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 = Z_STREAM_ERROR;
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 (err == Z_NEED_DICT) {
+ if (ctx->err_ == Z_NEED_DICT) {
assert(ctx->dictionary_ != NULL && "Stream has no dictionary");
-
- // Load it
- err = inflateSetDictionary(&ctx->strm_,
- ctx->dictionary_,
- ctx->dictionary_len_);
- assert(err == Z_OK && "Failed to set dictionary");
-
- // And try to decode again
- err = inflate(&ctx->strm_, ctx->flush_);
+ 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,
@@ -184,6 +189,19 @@ class ZCtx : public ObjectWrap {
HandleScope scope;
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<Integer> avail_out = Integer::New(ctx->strm_.avail_out);
Local<Integer> avail_in = Integer::New(ctx->strm_.avail_in);
@@ -198,6 +216,25 @@ 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<Value> args[2] = { String::New(msg),
+ Local<Value>::New(Number::New(ctx->err_)) };
+ MakeCallback(ctx->handle_, "onerror", ARRAY_SIZE(args), args);
+
+ // no hope of rescue.
+ ctx->Unref();
+ }
+
static Handle<Value> New(const Arguments& args) {
HandleScope scope;
if (args.Length() < 1 || !args[0]->IsInt32()) {
@@ -279,6 +316,8 @@ class ZCtx : public ObjectWrap {
ctx->flush_ = Z_NO_FLUSH;
+ ctx->err_ = Z_OK;
+
if (ctx->mode_ == GZIP || ctx->mode_ == GUNZIP) {
ctx->windowBits_ += 16;
}
@@ -291,29 +330,31 @@ class ZCtx : public ObjectWrap {
ctx->windowBits_ *= -1;
}
- int err;
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?");
}
- assert(err == Z_OK);
+ if (ctx->err_ != Z_OK) {
+ ZCtx::Error(ctx, "Init error");
+ }
+
ctx->dictionary_ = reinterpret_cast<Bytef *>(dictionary);
ctx->dictionary_len_ = dictionary_len;
@@ -325,39 +366,43 @@ class ZCtx : public ObjectWrap {
static void SetDictionary(ZCtx* ctx) {
if (ctx->dictionary_ == NULL) return;
- int err = Z_OK;
+ ctx->err_ = Z_OK;
switch (ctx->mode_) {
case DEFLATE:
case DEFLATERAW:
- err = deflateSetDictionary(&ctx->strm_,
- ctx->dictionary_,
- ctx->dictionary_len_);
+ ctx->err_ = deflateSetDictionary(&ctx->strm_,
+ ctx->dictionary_,
+ ctx->dictionary_len_);
break;
default:
break;
}
- assert(err == Z_OK && "Failed to set dictionary");
+ if (ctx->err_ != Z_OK) {
+ ZCtx::Error(ctx, "Failed to set dictionary");
+ }
}
static void Reset(ZCtx* ctx) {
- int err = Z_OK;
+ ctx->err_ = Z_OK;
switch (ctx->mode_) {
case DEFLATE:
case DEFLATERAW:
- err = deflateReset(&ctx->strm_);
+ ctx->err_ = deflateReset(&ctx->strm_);
break;
case INFLATE:
case INFLATERAW:
- err = inflateReset(&ctx->strm_);
+ ctx->err_ = inflateReset(&ctx->strm_);
break;
default:
break;
}
- assert(err == Z_OK && "Failed to reset stream");
+ if (ctx->err_ != Z_OK) {
+ ZCtx::Error(ctx, "Failed to reset stream");
+ }
}
private:
@@ -370,6 +415,8 @@ class ZCtx : public ObjectWrap {
int memLevel_;
int strategy_;
+ int err_;
+
Bytef* dictionary_;
size_t dictionary_len_;
@@ -399,13 +446,16 @@ void InitZlib(Handle<Object> target) {
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);
NODE_DEFINE_CONSTANT(target, Z_SYNC_FLUSH);
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);
@@ -415,6 +465,7 @@ void InitZlib(Handle<Object> 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);
@@ -27,6 +27,7 @@ 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() {
@@ -36,3 +37,36 @@ nonStringInputs.forEach(function(input) {
});
});
});
+
+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');
+});

0 comments on commit 01d46f3

Please sign in to comment.