From 8cd2b0e778e360df881122bc816af105768f4e26 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 4 May 2012 18:29:56 -0700 Subject: [PATCH 1/7] test: No need for weak in 'make test' --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d433474875..acc957dda7 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ install: uninstall: @$(WAF) uninstall -test: all node_modules/weak +test: all $(PYTHON) tools/test.py --mode=release simple message test-http1: all From e02af94947bf3702648a9eb04d073090833d4e43 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 7 May 2012 22:49:10 +0200 Subject: [PATCH 2/7] test: add failing HTTP client timeout test See #3231. --- test/simple/test-http-client-timeout.js | 50 +++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 test/simple/test-http-client-timeout.js diff --git a/test/simple/test-http-client-timeout.js b/test/simple/test-http-client-timeout.js new file mode 100644 index 0000000000..98eb5539eb --- /dev/null +++ b/test/simple/test-http-client-timeout.js @@ -0,0 +1,50 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// 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 assert = require('assert'); +var http = require('http'); + +var options = { + method: 'GET', + port: common.PORT, + host: '127.0.0.1', + path: '/' +}; + +var server = http.createServer(function(req, res) { + // this space intentionally left blank +}); + +server.listen(options.port, options.host, function() { + var req = http.request(options, function(res) { + // this space intentionally left blank + }); + req.on('close', function() { + server.close(); + }); + function destroy() { + req.destroy(); + } + req.setTimeout(1, destroy); + req.on('error', destroy); + req.end(); +}); From b4fbf6d27590e0fae42cc39adca9694793efdce1 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 7 May 2012 14:17:17 -0700 Subject: [PATCH 3/7] Fix #3231. Don't try to emit error on a null'ed req object --- lib/http.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/http.js b/lib/http.js index ef42584322..eb050ad423 100644 --- a/lib/http.js +++ b/lib/http.js @@ -1158,18 +1158,21 @@ ClientRequest.prototype.onSocket = function(socket) { // Setup "drain" propogation. httpSocketSetup(socket); - var errorListener = function(err) { + function errorListener(err) { debug('HTTP SOCKET ERROR: ' + err.message + '\n' + err.stack); - req.emit('error', err); - // For Safety. Some additional errors might fire later on - // and we need to make sure we don't double-fire the error event. - req._hadError = true; + if (req) { + req.emit('error', err); + // For Safety. Some additional errors might fire later on + // and we need to make sure we don't double-fire the error event. + req._hadError = true; + } if (parser) { parser.finish(); freeParser(parser, req); } socket.destroy(); } + socket.on('error', errorListener); socket.ondata = function(d, start, end) { From 8c758e127c9d81791cb55043abb418ce624dfcaf Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 7 May 2012 14:19:16 -0700 Subject: [PATCH 4/7] Don't destroy on timeout --- lib/http.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/http.js b/lib/http.js index eb050ad423..070942ef24 100644 --- a/lib/http.js +++ b/lib/http.js @@ -1346,7 +1346,6 @@ ClientRequest.prototype.setTimeout = function(msecs, callback) { var self = this; function emitTimeout() { self.emit('timeout'); - self.destroy(new Error('timeout')); } if (this.socket && this.socket.writable) { From ee437c0557c9f3a6a1960d3a8d64e09166d7048d Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 7 May 2012 17:04:56 +0200 Subject: [PATCH 5/7] 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; From 814033365b146afb095fad6c2c05d0da0237615f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisendo=CC=88rfer?= Date: Tue, 8 May 2012 16:07:14 +0200 Subject: [PATCH 6/7] Fix process.nextTick throw call sites This patch now reports the proper throw call site for exceptions triggered within process.nextTick. So instead of this: node.js:201 throw e; // process.nextTick error, or 'error' event on first tick ^ You will now see: mydir/myscript.js:15 throw new Error('My Error'); ^ From my testing this patch causes no performance regressions, but does greatly simplify processing the nextTickQueue. --- src/node.cc | 12 ++++++---- src/node.js | 24 +++++++------------ test/message/stack_overflow.out | 6 ++--- test/message/throw_custom_error.out | 6 ++--- test/message/throw_non_error.out | 6 ++--- .../undefined_reference_in_new_context.out | 6 ++--- 6 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/node.cc b/src/node.cc index e86303f5d8..31594bb5a5 100644 --- a/src/node.cc +++ b/src/node.cc @@ -255,9 +255,7 @@ static void Spin(uv_idle_t* handle, int status) { Tick(); } - -static Handle NeedTickCallback(const Arguments& args) { - HandleScope scope; +static void StartTickSpinner() { need_tick_cb = true; // TODO: this tick_spinner shouldn't be necessary. An ev_prepare should be // sufficent, the problem is only in the case of the very last "tick" - @@ -268,9 +266,12 @@ static Handle NeedTickCallback(const Arguments& args) { uv_idle_start(&tick_spinner, Spin); uv_ref(uv_default_loop()); } - return Undefined(); } +static Handle NeedTickCallback(const Arguments& args) { + StartTickSpinner(); + return Undefined(); +} static void PrepareTick(uv_prepare_t* handle, int status) { assert(handle == &prepare_tick_watcher); @@ -1694,6 +1695,9 @@ void FatalException(TryCatch &try_catch) { emit->Call(process, 2, event_argv); // Decrement so we know if the next exception is a recursion or not uncaught_exception_counter--; + + // This makes sure uncaught exceptions don't interfere with process.nextTick + StartTickSpinner(); } diff --git a/src/node.js b/src/node.js index 239a1abc91..9179d8146d 100644 --- a/src/node.js +++ b/src/node.js @@ -180,26 +180,18 @@ startup.processNextTick = function() { var nextTickQueue = []; + var nextTickIndex = 0; process._tickCallback = function() { - var l = nextTickQueue.length; - if (l === 0) return; + var nextTickLength = nextTickQueue.length; + if (nextTickLength === 0) return; - var q = nextTickQueue; - nextTickQueue = []; - - try { - for (var i = 0; i < l; i++) q[i](); - } - catch (e) { - if (i + 1 < l) { - nextTickQueue = q.slice(i + 1).concat(nextTickQueue); - } - if (nextTickQueue.length) { - process._needTickCallback(); - } - throw e; // process.nextTick error, or 'error' event on first tick + while (nextTickIndex < nextTickLength) { + nextTickQueue[nextTickIndex++](); } + + nextTickQueue.splice(0, nextTickIndex); + nextTickIndex = 0; }; process.nextTick = function(callback) { diff --git a/test/message/stack_overflow.out b/test/message/stack_overflow.out index f09f10bb5a..1b6971da32 100644 --- a/test/message/stack_overflow.out +++ b/test/message/stack_overflow.out @@ -1,6 +1,6 @@ before -node.js:* - throw e; // process.nextTick error, or 'error' event on first tick - ^ +module.js:311 + throw err; + ^ RangeError: Maximum call stack size exceeded diff --git a/test/message/throw_custom_error.out b/test/message/throw_custom_error.out index 357afdb76b..b5fd92b425 100644 --- a/test/message/throw_custom_error.out +++ b/test/message/throw_custom_error.out @@ -1,6 +1,6 @@ before -node.js:* - throw e; // process.nextTick error, or 'error' event on first tick - ^ +module.js:311 + throw err; + ^ MyCustomError: This is a custom message diff --git a/test/message/throw_non_error.out b/test/message/throw_non_error.out index 477b13cd8a..255e5e4d54 100644 --- a/test/message/throw_non_error.out +++ b/test/message/throw_non_error.out @@ -1,6 +1,6 @@ before -node.js:* - throw e; // process.nextTick error, or 'error' event on first tick - ^ +module.js:311 + throw err; + ^ [object Object] diff --git a/test/message/undefined_reference_in_new_context.out b/test/message/undefined_reference_in_new_context.out index d0e6e2f692..1e6a2d31d1 100644 --- a/test/message/undefined_reference_in_new_context.out +++ b/test/message/undefined_reference_in_new_context.out @@ -1,8 +1,8 @@ before -node.js:* - throw e; // process.nextTick error, or 'error' event on first tick - ^ +module.js:311 + throw err; + ^ ReferenceError: foo is not defined at evalmachine.:* at Object. (*test*message*undefined_reference_in_new_context.js:*) From bf9d8e9214e2098bacf18416564dc3a91bcdf5d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Geisendo=CC=88rfer?= Date: Tue, 8 May 2012 22:02:28 +0200 Subject: [PATCH 7/7] Fix exception output for module load exceptions So instead of: node.js:201 throw e; // process.nextTick error, or 'error' event on first tick ^ You will now see: path/to/foo.js:1 throw new Error('bar'); ^ This is a sub-set of isaacs patch here: https://github.com/joyent/node/issues/3235 The difference is that this patch purely adresses the exception output, but does not try to make any behavior changes / improvements. --- lib/module.js | 11 ++++++++--- test/message/stack_overflow.out | 6 +++--- test/message/throw_custom_error.out | 6 +++--- test/message/throw_non_error.out | 6 +++--- test/message/undefined_reference_in_new_context.out | 6 +++--- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/module.js b/lib/module.js index bf90c9692e..c4196ed120 100644 --- a/lib/module.js +++ b/lib/module.js @@ -304,11 +304,16 @@ Module._load = function(request, parent, isMain) { } Module._cache[filename] = module; + + var hadException = true; + try { module.load(filename); - } catch (err) { - delete Module._cache[filename]; - throw err; + hadException = false; + } finally { + if (hadException) { + delete Module._cache[filename]; + } } return module.exports; diff --git a/test/message/stack_overflow.out b/test/message/stack_overflow.out index 1b6971da32..72dc019ff1 100644 --- a/test/message/stack_overflow.out +++ b/test/message/stack_overflow.out @@ -1,6 +1,6 @@ before -module.js:311 - throw err; - ^ +*test*message*stack_overflow.js:31 +function stackOverflow() { + ^ RangeError: Maximum call stack size exceeded diff --git a/test/message/throw_custom_error.out b/test/message/throw_custom_error.out index b5fd92b425..1406226d7d 100644 --- a/test/message/throw_custom_error.out +++ b/test/message/throw_custom_error.out @@ -1,6 +1,6 @@ before -module.js:311 - throw err; - ^ +*test*message*throw_custom_error.js:31 +throw { name: 'MyCustomError', message: 'This is a custom message' }; +^ MyCustomError: This is a custom message diff --git a/test/message/throw_non_error.out b/test/message/throw_non_error.out index 255e5e4d54..f0717c63c2 100644 --- a/test/message/throw_non_error.out +++ b/test/message/throw_non_error.out @@ -1,6 +1,6 @@ before -module.js:311 - throw err; - ^ +*/test/message/throw_non_error.js:31 +throw { foo: 'bar' }; +^ [object Object] diff --git a/test/message/undefined_reference_in_new_context.out b/test/message/undefined_reference_in_new_context.out index 1e6a2d31d1..d6e9011554 100644 --- a/test/message/undefined_reference_in_new_context.out +++ b/test/message/undefined_reference_in_new_context.out @@ -1,8 +1,8 @@ before -module.js:311 - throw err; - ^ +*test*message*undefined_reference_in_new_context.js:34 +script.runInNewContext(); + ^ ReferenceError: foo is not defined at evalmachine.:* at Object. (*test*message*undefined_reference_in_new_context.js:*)