From 90802d628df9ca516036c287c37610029a2e8dc7 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 20 Apr 2011 15:44:34 -0700 Subject: [PATCH 01/27] Close #954 URL parsing/formatting corrections 1. Allow single-quotes in urls, but escape them. 2. Add comments about which RFCs we're following for guidance. 3. Handle any invalid character in the hostname portion. 4. lcase protocol and hostname portions, since they are case-insensitive. --- lib/url.js | 72 +++++++++++++++++++++--- test/simple/test-url.js | 118 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 171 insertions(+), 19 deletions(-) diff --git a/lib/url.js b/lib/url.js index cbea2f0fc3a..99a0e67422b 100644 --- a/lib/url.js +++ b/lib/url.js @@ -24,25 +24,40 @@ exports.resolve = urlResolve; exports.resolveObject = urlResolveObject; exports.format = urlFormat; +// Reference: RFC 3986, RFC 1808, RFC 2396 + // define these here so at least they only have to be // compiled once on the first module load. -var protocolPattern = /^([a-z0-9]+:)/, +var protocolPattern = /^([a-z0-9]+:)/i, portPattern = /:[0-9]+$/, - delims = ['<', '>', '"', '\'', '`', /\s/], + // RFC 2396: characters reserved for delimiting URLs. + delims = ['<', '>', '"', '`', ' ', '\r', '\n', '\t'], + // RFC 2396: characters not allowed for various reasons. unwise = ['{', '}', '|', '\\', '^', '~', '[', ']', '`'].concat(delims), - nonHostChars = ['/', '?', ';', '#'].concat(unwise), + // Allowed by RFCs, but cause of XSS attacks. Always escape these. + autoEscape = ['\''], + // Characters that are never ever allowed in a hostname. + // Note that any invalid chars are also handled, but these + // are the ones that are *expected* to be seen, so we fast-path + // them. + nonHostChars = ['%', '/', '?', ';', '#'] + .concat(unwise).concat(autoEscape), hostnameMaxLen = 255, - hostnamePartPattern = /^[a-z0-9][a-z0-9A-Z-]{0,62}$/, + hostnamePartPattern = /^[a-zA-Z0-9][a-z0-9A-Z-]{0,62}$/, + hostnamePartStart = /^([a-zA-Z0-9][a-z0-9A-Z-]{0,62})(.*)$/, + // protocols that can allow "unsafe" and "unwise" chars. unsafeProtocol = { 'javascript': true, 'javascript:': true }, + // protocols that never have a hostname. hostlessProtocol = { 'javascript': true, 'javascript:': true, 'file': true, 'file:': true }, + // protocols that always have a path component. pathedProtocol = { 'http': true, 'https': true, @@ -54,6 +69,7 @@ var protocolPattern = /^([a-z0-9]+:)/, 'gopher:': true, 'file:': true }, + // protocols that always contain a // bit. slashedProtocol = { 'http': true, 'https': true, @@ -74,10 +90,19 @@ function urlParse(url, parseQueryString, slashesDenoteHost) { var out = {}, rest = url; + // cut off any delimiters. + // This is to support parse stuff like "" + for (var i = 0, l = rest.length; i < l; i++) { + if (delims.indexOf(rest.charAt(i)) === -1) break; + } + if (i !== 0) rest = rest.substr(i); + + var proto = protocolPattern.exec(rest); if (proto) { proto = proto[0]; - out.protocol = proto; + var lowerProto = proto.toLowerCase(); + out.protocol = lowerProto; rest = rest.substr(proto.length); } @@ -119,6 +144,7 @@ function urlParse(url, parseQueryString, slashesDenoteHost) { var key = keys[i]; out[key] = p[key]; } + // we've indicated that there is a hostname, // so even if it's empty, it has to be present. out.hostname = out.hostname || ''; @@ -130,17 +156,49 @@ function urlParse(url, parseQueryString, slashesDenoteHost) { var hostparts = out.hostname.split(/\./); for (var i = 0, l = hostparts.length; i < l; i++) { var part = hostparts[i]; + if (!part) continue; if (!part.match(hostnamePartPattern)) { - out.hostname = ''; + var validParts = hostparts.slice(0, i); + var notHost = hostparts.slice(i + 1); + var bit = part.match(hostnamePartStart); + if (bit) { + validParts.push(bit[1]); + notHost.unshift(bit[2]); + } + if (notHost.length) { + rest = '/' + notHost.join('.') + rest + } + out.hostname = validParts.join('.'); break; } } } + // hostnames are always lower case. + out.hostname = out.hostname.toLowerCase(); + + out.host = ((out.auth) ? out.auth + '@' : '') + + (out.hostname || '') + + ((out.port) ? ':' + out.port : ''); + out.href += out.host; } // now rest is set to the post-host stuff. // chop off any delim chars. - if (!unsafeProtocol[proto]) { + if (!unsafeProtocol[lowerProto]) { + + // First, make 100% sure that any "autoEscape" chars get + // escaped, even if encodeURIComponent doesn't think they + // need to be. + for (var i = 0, l = autoEscape.length; i < l; i++) { + var ae = autoEscape[i]; + var esc = encodeURIComponent(ae); + if (esc === ae) { + esc = escape(ae); + } + rest = rest.split(ae).join(esc); + } + + // Now make sure that delims never appear in a url. var chop = rest.length; for (var i = 0, l = delims.length; i < l; i++) { var c = rest.indexOf(delims[i]); diff --git a/test/simple/test-url.js b/test/simple/test-url.js index 4f3d139ca9a..e52dacd8bd0 100644 --- a/test/simple/test-url.js +++ b/test/simple/test-url.js @@ -32,6 +32,99 @@ var parseTests = { 'href': '//some_path', 'pathname': '//some_path' }, + 'HTTP://www.example.com/' : { + 'href': 'http://www.example.com/', + 'protocol': 'http:', + 'host': 'www.example.com', + 'hostname': 'www.example.com', + 'pathname': '/' + }, + 'http://www.ExAmPlE.com/' : { + 'href': 'http://www.example.com/', + 'protocol': 'http:', + 'host': 'www.example.com', + 'hostname': 'www.example.com', + 'pathname': '/' + + }, + 'http://user:pw@www.ExAmPlE.com/' : { + 'href': 'http://user:pw@www.example.com/', + 'protocol': 'http:', + 'auth': 'user:pw', + 'host': 'user:pw@www.example.com', + 'hostname': 'www.example.com', + 'pathname': '/' + + }, + 'http://USER:PW@www.ExAmPlE.com/' : { + 'href': 'http://USER:PW@www.example.com/', + 'protocol': 'http:', + 'auth': 'USER:PW', + 'host': 'USER:PW@www.example.com', + 'hostname': 'www.example.com', + 'pathname': '/' + }, + 'http://x.com/path?that\'s#all, folks' : { + 'href': 'http://x.com/path?that%27s#all,', + 'protocol': 'http:', + 'host': 'x.com', + 'hostname': 'x.com', + 'search': '?that%27s', + 'query': 'that%27s', + 'pathname': '/path', + 'hash': '#all,' + }, + 'HTTP://X.COM/Y' : { + 'href': 'http://x.com/Y', + 'protocol': 'http:', + 'host': 'x.com', + 'hostname': 'x.com', + 'pathname': '/Y', + }, + // an unexpected invalid char in the hostname. + 'HtTp://x.y.cOm*a/b/c?d=e#f gi' : { + 'href': 'http://x.y.com/*a/b/c?d=e#f', + 'protocol': 'http:', + 'host': 'x.y.com', + 'hostname': 'x.y.com', + 'pathname': '/*a/b/c', + 'search': '?d=e', + 'query': 'd=e', + 'hash': '#f' + }, + // make sure that we don't accidentally lcast the path parts. + 'HtTp://x.y.cOm*A/b/c?d=e#f gi' : { + 'href': 'http://x.y.com/*A/b/c?d=e#f', + 'protocol': 'http:', + 'host': 'x.y.com', + 'hostname': 'x.y.com', + 'pathname': '/*A/b/c', + 'search': '?d=e', + 'query': 'd=e', + 'hash': '#f' + }, + 'http://x...y...#p': { + 'href': 'http://x...y.../#p', + 'protocol': 'http:', + 'host': 'x...y...', + 'hostname': 'x...y...', + 'hash': '#p', + 'pathname': '/' + }, + 'http://x/p/"quoted"': { + 'href': 'http://x/p/', + 'protocol':'http:', + 'host': 'x', + 'hostname': 'x', + 'pathname': '/p/' + }, + ' Is a URL!': { + 'href': 'http://goo.corn/bread', + 'protocol': 'http:', + 'host': 'goo.corn', + 'hostname': 'goo.corn', + 'pathname': '/bread' + }, 'http://www.narwhaljs.org/blog/categories?id=news' : { 'href': 'http://www.narwhaljs.org/blog/categories?id=news', 'protocol': 'http:', @@ -58,17 +151,18 @@ var parseTests = { 'query': '??&hl=en&src=api&x=2&y=2&z=3&s=', 'pathname': '/vt/lyrs=m@114' }, - 'http://user:pass@mt0.google.com/vt/lyrs=m@114???&hl=en&src=api&x=2&y=2&z=3&s=' : { - 'href': 'http://user:pass@mt0.google.com/vt/lyrs=m@114???' + - '&hl=en&src=api&x=2&y=2&z=3&s=', - 'protocol': 'http:', - 'host': 'user:pass@mt0.google.com', - 'auth': 'user:pass', - 'hostname': 'mt0.google.com', - 'search': '???&hl=en&src=api&x=2&y=2&z=3&s=', - 'query': '??&hl=en&src=api&x=2&y=2&z=3&s=', - 'pathname': '/vt/lyrs=m@114' - }, + 'http://user:pass@mt0.google.com/vt/lyrs=m@114???&hl=en&src=api&x=2&y=2&z=3&s=': + { + 'href': 'http://user:pass@mt0.google.com/vt/lyrs=m@114???' + + '&hl=en&src=api&x=2&y=2&z=3&s=', + 'protocol': 'http:', + 'host': 'user:pass@mt0.google.com', + 'auth': 'user:pass', + 'hostname': 'mt0.google.com', + 'search': '???&hl=en&src=api&x=2&y=2&z=3&s=', + 'query': '??&hl=en&src=api&x=2&y=2&z=3&s=', + 'pathname': '/vt/lyrs=m@114' + }, 'file:///etc/passwd' : { 'href': 'file:///etc/passwd', 'protocol': 'file:', @@ -154,7 +248,7 @@ for (var u in parseTests) { 'parse(' + u + ').' + i + ' == ' + e + '\nactual: ' + a); } - var expected = u, + var expected = parseTests[u].href, actual = url.format(parseTests[u]); assert.equal(expected, actual, From d00739ce56b07ab438eceffb0d701e90dc8b3fdc Mon Sep 17 00:00:00 2001 From: Wade Simmons Date: Wed, 13 Apr 2011 17:54:50 -0600 Subject: [PATCH 02/27] make it possible to do repl.start('', stream) --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index 75d09d70314..434ff66baeb 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -93,7 +93,7 @@ function REPLServer(prompt, stream) { process.stdin.resume(); } - self.prompt = prompt || '> '; + self.prompt = (prompt != undefined ? prompt : '> '); function complete(text) { return self.complete(text); From 6c7c4aeab6918425e27f1280842e39151cf270ae Mon Sep 17 00:00:00 2001 From: Tim Baumann Date: Wed, 30 Mar 2011 21:05:56 +0200 Subject: [PATCH 03/27] Don't overwrite an user-specified repl.writer --- lib/repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/repl.js b/lib/repl.js index 434ff66baeb..33794776727 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -105,7 +105,7 @@ function REPLServer(prompt, stream) { this.commands = {}; defineDefaultCommands(this); - if (rli.enabled && !disableColors) { + if (rli.enabled && !disableColors && exports.writer === util.inspect) { // Turn on ANSI coloring. exports.writer = function(obj, showHidden, depth) { return util.inspect(obj, showHidden, depth, true); From ed657e9d462e5d3c8dda82569209ec3508d39401 Mon Sep 17 00:00:00 2001 From: Robert Mustacchi Date: Thu, 21 Apr 2011 19:39:16 -0700 Subject: [PATCH 04/27] Add loadavg for SunOS --- src/platform_sunos.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/platform_sunos.cc b/src/platform_sunos.cc index 002b373a5a7..99488f1f0e4 100644 --- a/src/platform_sunos.cc +++ b/src/platform_sunos.cc @@ -31,6 +31,7 @@ #include #include #include +#include #if (!defined(_LP64)) && (_FILE_OFFSET_BITS - 0 == 64) #define PROCFS_FILE_OFFSET_BITS_HACK 1 @@ -249,6 +250,14 @@ double Platform::GetUptime() { } int Platform::GetLoadAvg(Local *loads) { + HandleScope scope; + double loadavg[3]; + + (void) getloadavg(loadavg, 3); + (*loads)->Set(0, Number::New(loadavg[LOADAVG_1MIN])); + (*loads)->Set(1, Number::New(loadavg[LOADAVG_5MIN])); + (*loads)->Set(2, Number::New(loadavg[LOADAVG_15MIN])); + return 0; } From 3ce5e6fe5e646c6f768097028eac239574ea1e53 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 16:01:34 -0700 Subject: [PATCH 05/27] Use better ports on the home page --- doc/index.html | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/index.html b/doc/index.html index 5770786a929..d3c6b250c5e 100644 --- a/doc/index.html +++ b/doc/index.html @@ -55,8 +55,8 @@ var http = require('http'); http.createServer(function (req, res) { res.writeHead(200, {'Content-Type': 'text/plain'}); res.end('Hello World\n'); -}).listen(8124, "127.0.0.1"); -console.log('Server running at http://127.0.0.1:8124/'); +}).listen(1337, "127.0.0.1"); +console.log('Server running at http://127.0.0.1:1337/');

@@ -66,10 +66,10 @@ console.log('Server running at http://127.0.0.1:8124/'); 

 % node example.js
-Server running at http://127.0.0.1:8124/
+Server running at http://127.0.0.1:1337/

- Here is an example of a simple TCP server which listens on port 8124 + Here is an example of a simple TCP server which listens on port 1337 and echoes whatever you send it:

@@ -79,9 +79,9 @@ var net = require('net'); var server = net.createServer(function (socket) { socket.write("Echo server\r\n"); socket.pipe(socket); -}) +}); -server.listen(8124, "127.0.0.1"); +server.listen(1337, "127.0.0.1");

From c8e447ee63da9eb1d19141d8772f202edd4afe26 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 16:38:27 -0700 Subject: [PATCH 06/27] Fix timeouts with floating point numbers bug fixes #897. --- src/node.h | 2 +- test/simple/test-regress-GH-897.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/simple/test-regress-GH-897.js diff --git a/src/node.h b/src/node.h index 06135d66fb6..14d726de263 100644 --- a/src/node.h +++ b/src/node.h @@ -43,7 +43,7 @@ int Start (int argc, char *argv[]); /* Converts a unixtime to V8 Date */ #define NODE_UNIXTIME_V8(t) v8::Date::New(1000*static_cast(t)) -#define NODE_V8_UNIXTIME(v) (static_cast((v)->IntegerValue())/1000.0); +#define NODE_V8_UNIXTIME(v) (static_cast((v)->NumberValue())/1000.0); #define NODE_DEFINE_CONSTANT(target, constant) \ (target)->Set(v8::String::NewSymbol(#constant), \ diff --git a/test/simple/test-regress-GH-897.js b/test/simple/test-regress-GH-897.js new file mode 100644 index 00000000000..7103a724713 --- /dev/null +++ b/test/simple/test-regress-GH-897.js @@ -0,0 +1,14 @@ +var common = require('../common'); +var assert = require('assert'); + +var t = Date.now(); +var diff; +setTimeout(function () { + diff = Date.now() - t; + console.error(diff); +}, 0.1); + + +process.on('exit', function() { + assert.ok(diff < 10); +}); From c85455a954411b38232e79752d4abb61bb75031b Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 17:06:02 -0700 Subject: [PATCH 07/27] bump version to v0.4.7 --- AUTHORS | 2 ++ ChangeLog | 18 ++++++++++++++++++ doc/index.html | 8 ++++---- src/node_version.h | 2 +- wscript | 2 +- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/AUTHORS b/AUTHORS index cf29f0189e4..6378303907b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -170,4 +170,6 @@ Arnout Kazemier George Stagas Scott McWhirter Jakub Lekstan +Tim Baumann +Robert Mustacchi diff --git a/ChangeLog b/ChangeLog index bba5127ea3c..a0d54171cc0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,21 @@ +2011.04.22, Version 0.4.7 (stable) + +* Don't emit error on ECONNRESET from read() #670 + +* Fix: Multiple pipes to the same stream were broken #929 + (Felix Geisendörfer) + +* URL parsing/formatting corrections #954 (isaacs) + +* make it possible to do repl.start('', stream) (Wade Simmons) + +* Add os.loadavg for SunOS (Robert Mustacchi) + +* Fix timeouts with floating point numbers #897 + +* Improve docs. + + 2011.04.13, Version 0.4.6 (stable) * Don't error on ENOTCONN from shutdown() #670 diff --git a/doc/index.html b/doc/index.html index d3c6b250c5e..1cf7e0e4b61 100644 --- a/doc/index.html +++ b/doc/index.html @@ -26,7 +26,7 @@

  • Download
  • ChangeLog
  • About
    • -
  • v0.4.6 docs
    • +
  • v0.4.7 docs

  • Wiki
  • Blog
    • @@ -107,9 +107,9 @@ server.listen(1337, "127.0.0.1");

    - 2011.04.13 - node-v0.4.6.tar.gz - (Documentation) + 2011.04.22 + node-v0.4.7.tar.gz + (Documentation)

    Historical: versions, docs

    diff --git a/src/node_version.h b/src/node_version.h index fdd27cb3de4..d5c81d21e34 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -28,7 +28,7 @@ #define NODE_MAJOR_VERSION 0 #define NODE_MINOR_VERSION 4 #define NODE_PATCH_VERSION 7 -#define NODE_VERSION_IS_RELEASE 0 +#define NODE_VERSION_IS_RELEASE 1 #ifndef NODE_STRINGIFY #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n) diff --git a/wscript b/wscript index f8357c470db..a07fceeeab9 100644 --- a/wscript +++ b/wscript @@ -872,7 +872,7 @@ def build(bld): , 'CPPFLAGS' : " ".join(program.env["CPPFLAGS"]).replace('"', '\\"') , 'LIBFLAGS' : " ".join(program.env["LIBFLAGS"]).replace('"', '\\"') , 'PREFIX' : safe_path(program.env["PREFIX"]) - , 'VERSION' : '0.4.6' # FIXME should not be hard-coded, see NODE_VERSION_STRING in src/node_version. + , 'VERSION' : '0.4.7' # FIXME should not be hard-coded, see NODE_VERSION_STRING in src/node_version. } return x From 1f470b68d6bf90b47339175fbd63bd7bd8fad06c Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 17:49:56 -0700 Subject: [PATCH 08/27] Now working on v0.4.8 --- src/node_version.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_version.h b/src/node_version.h index d5c81d21e34..3eeeb739343 100644 --- a/src/node_version.h +++ b/src/node_version.h @@ -27,8 +27,8 @@ #define NODE_MAJOR_VERSION 0 #define NODE_MINOR_VERSION 4 -#define NODE_PATCH_VERSION 7 -#define NODE_VERSION_IS_RELEASE 1 +#define NODE_PATCH_VERSION 8 +#define NODE_VERSION_IS_RELEASE 0 #ifndef NODE_STRINGIFY #define NODE_STRINGIFY(n) NODE_STRINGIFY_HELPER(n) From 621b024b6ed5e3af24867e2824dc4a9a77ca77ef Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 17:50:40 -0700 Subject: [PATCH 09/27] Bump bounds on #897 test - was too small for slow machines --- test/simple/test-regress-GH-897.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/simple/test-regress-GH-897.js b/test/simple/test-regress-GH-897.js index 7103a724713..937226d42b4 100644 --- a/test/simple/test-regress-GH-897.js +++ b/test/simple/test-regress-GH-897.js @@ -10,5 +10,5 @@ setTimeout(function () { process.on('exit', function() { - assert.ok(diff < 10); + assert.ok(diff < 100); }); From 0325a21ff329ea7044ea6e6ae13085dcd34cbf4f Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 22 Apr 2011 19:04:43 -0700 Subject: [PATCH 10/27] Correct attribution --- ChangeLog | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index a0d54171cc0..e1f8c4c3c8f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -11,7 +11,7 @@ * Add os.loadavg for SunOS (Robert Mustacchi) -* Fix timeouts with floating point numbers #897 +* Fix timeouts with floating point numbers #897 (Jorge Chamorro Bieling) * Improve docs. From 8fd1c68f06221e04ccc5a40a04e0f5aa13bd1792 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 22 Apr 2011 19:31:14 -0700 Subject: [PATCH 11/27] A test that running 100 stream pipes in parallel is ok --- test/simple/test-stream-pipe-multi.js | 100 ++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 test/simple/test-stream-pipe-multi.js diff --git a/test/simple/test-stream-pipe-multi.js b/test/simple/test-stream-pipe-multi.js new file mode 100644 index 00000000000..a6931ab7a52 --- /dev/null +++ b/test/simple/test-stream-pipe-multi.js @@ -0,0 +1,100 @@ +// Test that having a bunch of streams piping in parallel +// doesn't break anything. + +var common = require("../common"); +var assert = require("assert"); +var Stream = require("stream").Stream; +var rr = []; +var ww = []; +var cnt = 100; +var chunks = 1000; +var chunkSize = 250; +var data = new Buffer(chunkSize); +var wclosed = 0; +var rclosed = 0; + +function FakeStream() { + Stream.apply(this); + this.wait = false; + this.writable = true; + this.readable = true; +} + +FakeStream.prototype = Object.create(Stream.prototype); + +FakeStream.prototype.write = function(chunk) { + console.error(this.ID, "write", this.wait) + if (this.wait) { + process.nextTick(this.emit.bind(this, "drain")); + } + this.wait = !this.wait; + return this.wait; +}; + +FakeStream.prototype.end = function() { + this.emit("end"); + process.nextTick(this.close.bind(this)); +}; + +// noop - closes happen automatically on end. +FakeStream.prototype.close = function() { + this.emit("close"); +}; + + +// expect all streams to close properly. +process.on("exit", function() { + assert.equal(cnt, wclosed, "writable streams closed"); + assert.equal(cnt, rclosed, "readable streams closed"); +}); + +for (var i = 0; i < chunkSize; i ++) { + chunkSize[i] = i % 256; +} + +for (var i = 0; i < cnt; i++) { + var r = new FakeStream(); + r.on("close", function() { + console.error(this.ID, "read close"); + rclosed++; + }); + rr.push(r); + + var w = new FakeStream(); + w.on("close", function() { + console.error(this.ID, "write close"); + wclosed++; + }); + ww.push(w); + + r.ID = w.ID = i; + r.pipe(w); +} + +// now start passing through data +// simulate a relatively fast async stream. +rr.forEach(function (r) { + var cnt = chunks; + var paused = false; + + r.on("pause", function() { + paused = true; + }); + + r.on("resume", function() { + paused = false; + step(); + }); + + function step() { + r.emit("data", data); + if (--cnt === 0) { + r.end(); + return; + } + if (paused) return; + process.nextTick(step); + } + + process.nextTick(step); +}); From 8df6f9e54469902a3d1da30a57cbfbe6900f977d Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 25 Apr 2011 12:22:18 -0700 Subject: [PATCH 12/27] Close #974 Properly report traceless errors. Also, tests for the same. --- src/node.cc | 17 ++++++++++++--- test/message/stack_overflow.js | 33 +++++++++++++++++++++++++++++ test/message/stack_overflow.out | 6 ++++++ test/message/throw_custom_error.js | 30 ++++++++++++++++++++++++++ test/message/throw_custom_error.out | 6 ++++++ test/message/throw_non_error.js | 30 ++++++++++++++++++++++++++ test/message/throw_non_error.out | 6 ++++++ 7 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 test/message/stack_overflow.js create mode 100644 test/message/stack_overflow.out create mode 100644 test/message/throw_custom_error.js create mode 100644 test/message/throw_custom_error.out create mode 100644 test/message/throw_non_error.js create mode 100644 test/message/throw_non_error.out diff --git a/src/node.cc b/src/node.cc index fa2fab2ab12..fa64bb9902d 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1274,13 +1274,24 @@ static void ReportException(TryCatch &try_catch, bool show_line) { String::Utf8Value trace(try_catch.StackTrace()); - if (trace.length() > 0) { + // range errors have a trace member set to undefined + if (trace.length() > 0 && !try_catch.StackTrace()->IsUndefined()) { fprintf(stderr, "%s\n", *trace); } else { // this really only happens for RangeErrors, since they're the only - // kind that won't have all this info in the trace. + // kind that won't have all this info in the trace, or when non-Error + // objects are thrown manually. Local er = try_catch.Exception(); - String::Utf8Value msg(!er->IsObject() ? er->ToString() + bool isErrorObject = er->IsObject() && + !(er->ToObject()->Get(String::New("message"))->IsUndefined()) && + !(er->ToObject()->Get(String::New("name"))->IsUndefined()); + + if (isErrorObject) { + String::Utf8Value name(er->ToObject()->Get(String::New("name"))); + fprintf(stderr, "%s: ", *name); + } + + String::Utf8Value msg(!isErrorObject ? er->ToString() : er->ToObject()->Get(String::New("message"))->ToString()); fprintf(stderr, "%s\n", *msg); } diff --git a/test/message/stack_overflow.js b/test/message/stack_overflow.js new file mode 100644 index 00000000000..d8f89f0c526 --- /dev/null +++ b/test/message/stack_overflow.js @@ -0,0 +1,33 @@ +// 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'); + +common.error('before'); + +// stack overflow +function stackOverflow() { + stackOverflow(); +} +stackOverflow(); + +common.error('after'); diff --git a/test/message/stack_overflow.out b/test/message/stack_overflow.out new file mode 100644 index 00000000000..8269698cf79 --- /dev/null +++ b/test/message/stack_overflow.out @@ -0,0 +1,6 @@ +before + +node.js:134 + throw e; // process.nextTick error, or 'error' event on first tick + ^ +RangeError: Maximum call stack size exceeded diff --git a/test/message/throw_custom_error.js b/test/message/throw_custom_error.js new file mode 100644 index 00000000000..ef9fd3e0f77 --- /dev/null +++ b/test/message/throw_custom_error.js @@ -0,0 +1,30 @@ +// 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'); + +common.error('before'); + +// custom error throwing +throw { name: 'MyCustomError', message: 'This is a custom message' }; + +common.error('after'); diff --git a/test/message/throw_custom_error.out b/test/message/throw_custom_error.out new file mode 100644 index 00000000000..5279758ed5c --- /dev/null +++ b/test/message/throw_custom_error.out @@ -0,0 +1,6 @@ +before + +node.js:134 + throw e; // process.nextTick error, or 'error' event on first tick + ^ +MyCustomError: This is a custom message diff --git a/test/message/throw_non_error.js b/test/message/throw_non_error.js new file mode 100644 index 00000000000..9d5d4a96114 --- /dev/null +++ b/test/message/throw_non_error.js @@ -0,0 +1,30 @@ +// 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'); + +common.error('before'); + +// custom error throwing +throw { foo : 'bar' }; + +common.error('after'); diff --git a/test/message/throw_non_error.out b/test/message/throw_non_error.out new file mode 100644 index 00000000000..7c22d2f0b87 --- /dev/null +++ b/test/message/throw_non_error.out @@ -0,0 +1,6 @@ +before + +node.js:134 + throw e; // process.nextTick error, or 'error' event on first tick + ^ +[object Object] From 68c8a69f4cdda9e6e1d80f1ed90efbabf410f20c Mon Sep 17 00:00:00 2001 From: George Miroshnykov Date: Thu, 21 Apr 2011 14:43:12 +0300 Subject: [PATCH 13/27] Close #962: Fixed typo in vm.runInNewContext docs. EDIT: Also added another typo fix to this commit. --isaacs --- doc/api/vm.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/vm.markdown b/doc/api/vm.markdown index 3fff4ba8b5d..2b7869da49f 100644 --- a/doc/api/vm.markdown +++ b/doc/api/vm.markdown @@ -33,7 +33,7 @@ Example of using `vm.runInThisContext` and `eval` to run the same code: `eval` does have access to the local scope, so `localVar` is changed. In case of syntax error in `code`, `vm.runInThisContext` emits the syntax error to stderr -and throws.an exception. +and throws an exception. ### vm.runInNewContext(code, [sandbox], [filename]) @@ -62,7 +62,7 @@ Note that running untrusted code is a tricky business requiring great care. To global variable leakage, `vm.runInNewContext` is quite useful, but safely running untrusted code requires a separate process. -In case of syntax error in `code`, `vm.runInThisContext` emits the syntax error to stderr +In case of syntax error in `code`, `vm.runInNewContext` emits the syntax error to stderr and throws an exception. From bbffd9e5025ef22156c7d21c746a51529f821664 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 26 Apr 2011 09:48:28 -0700 Subject: [PATCH 14/27] Close #983 Better JSON.parse error detection Previous pattern would only catch ILLEGAL, not { or other known-but-unexpected JSON tokens. --- lib/repl.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 33794776727..ffe2b77e469 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -184,8 +184,8 @@ function REPLServer(prompt, stream) { // It could also be an error from JSON.parse } else if (e && e.stack && - e.stack.match('Unexpected token ILLEGAL') && - e.stack.match(/Object.parse \(native\)/)) { + e.stack.match(/^SyntaxError: Unexpected token .*\n/) && + e.stack.match(/\n at Object.parse \(native\)\n/)) { throw e; } } From 7c6f0147dfa78641a65c7cb0d84f6723b29a946e Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 27 Apr 2011 11:10:10 -0700 Subject: [PATCH 15/27] Better stream.pipe() tracking. This commit does three things: 1. Uses an exposed counter rather than a hidden array for tracking dest streams that may have multiple inputs. This allows for significantly faster lookups, since the counter can be checked in constant time rather than searching an array for the dest object. (A proper O(1) WeakMap would be better, but that may have to wait for Harmony.) 2. Calls the 'end' event logic when there is an 'error' event on the source object (and then throws if there are no other error listeners.) This is important, because otherwise 'error' events would lead to memory leaks. 3. Clean up the style a bit. Function Declarations are not allowed within blocks in ES strict. Prefer Function Declarations to Function Expressions, because hoisting allows for more expressive ordering of logic. Downside: It adds "_pipeCount" as part of the Stream API. It'll work fine if the member is missing, but if anyone tries to use it for some other purpose, it can mess things up. --- lib/stream.js | 70 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/lib/stream.js b/lib/stream.js index 632c87d2e26..ca7fc28c024 100644 --- a/lib/stream.js +++ b/lib/stream.js @@ -28,13 +28,9 @@ function Stream() { util.inherits(Stream, events.EventEmitter); exports.Stream = Stream; -var pipes = []; - Stream.prototype.pipe = function(dest, options) { var source = this; - pipes.push(dest); - function ondata(chunk) { if (dest.writable) { if (false === dest.write(chunk)) source.pause(); @@ -49,31 +45,48 @@ Stream.prototype.pipe = function(dest, options) { dest.on('drain', ondrain); - /* - * If the 'end' option is not supplied, dest.end() will be called when - * source gets the 'end' event. - */ - + // If the 'end' option is not supplied, dest.end() will be called when + // source gets the 'end' or 'close' events. Only dest.end() once, and + // only when all sources have ended. if (!options || options.end !== false) { - function onend() { - var index = pipes.indexOf(dest); - pipes.splice(index, 1); - - if (pipes.indexOf(dest) > -1) { - return; - } - - dest.end(); - } + dest._pipeCount = dest._pipeCount || 0; + dest._pipeCount++; source.on('end', onend); source.on('close', onend); } - /* - * Questionable: - */ + var didOnEnd = false; + function onend() { + if (didOnEnd) return; + didOnEnd = true; + dest._pipeCount--; + + // remove the listeners + cleanup(); + + if (dest._pipeCount > 0) { + // waiting for other incoming streams to end. + return; + } + + dest.end(); + } + + // don't leave dangling pipes when there are errors. + function onerror(er) { + cleanup(); + if (this.listeners('error').length === 1) { + throw er; // Unhandled stream error in pipe. + } + } + + source.on('error', onerror); + dest.on('error', onerror); + + // guarantee that source streams can be paused and resumed, even + // if the only effect is to proxy the event back up the pipe chain. if (!source.pause) { source.pause = function() { source.emit('pause'); @@ -86,27 +99,32 @@ Stream.prototype.pipe = function(dest, options) { }; } - var onpause = function() { + function onpause() { source.pause(); } dest.on('pause', onpause); - var onresume = function() { + function onresume() { if (source.readable) source.resume(); - }; + } dest.on('resume', onresume); - var cleanup = function () { + // remove all the event listeners that were added. + function cleanup() { source.removeListener('data', ondata); dest.removeListener('drain', ondrain); + source.removeListener('end', onend); source.removeListener('close', onend); dest.removeListener('pause', onpause); dest.removeListener('resume', onresume); + source.removeListener('error', onerror); + dest.removeListener('error', onerror); + source.removeListener('end', cleanup); source.removeListener('close', cleanup); From 1343ee8d54f1743b19e5b70b8f377ec65508f9a6 Mon Sep 17 00:00:00 2001 From: koichik Date: Thu, 28 Apr 2011 16:36:04 +0900 Subject: [PATCH 16/27] Doc improvements. --- doc/api/http.markdown | 2 +- doc/api/https.markdown | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/api/http.markdown b/doc/api/http.markdown index 09efedebf75..6cecc821cfb 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -88,7 +88,7 @@ sent to the server on that socket. If a client connection emits an 'error' event - it will forwarded here. -### http.createServer(requestListener) +### http.createServer([requestListener]) Returns a new web server object. diff --git a/doc/api/https.markdown b/doc/api/https.markdown index 3c6a7c43a0c..ff383f7d399 100644 --- a/doc/api/https.markdown +++ b/doc/api/https.markdown @@ -4,7 +4,15 @@ HTTPS is the HTTP protocol over TLS/SSL. In Node this is implemented as a separate module. ## https.Server -## https.createServer + +This class is a subclass of `tls.Server` and emits events same as +`http.Server`. See `http.Server` for more information. + +## https.createServer(options, [requestListener]) + +Returns a new HTTPS web server object. The `options` is similer to +`tls.createServer()`. The `requestListener` is a function which is +automatically added to the `'request'` event. Example: From fcc04e67c87f872910e2a77c4d6888ac1568faa7 Mon Sep 17 00:00:00 2001 From: koichik Date: Thu, 28 Apr 2011 15:38:28 +0900 Subject: [PATCH 17/27] Fix SlowBuffer.write() with 'ucs2' throws ReferenceError. --- lib/buffer.js | 2 +- test/simple/test-buffer.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/buffer.js b/lib/buffer.js index ba0ae2684fc..52e5e8052dd 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -100,7 +100,7 @@ SlowBuffer.prototype.write = function(string, offset, encoding) { case 'ucs2': case 'ucs-2': - return this.ucs2Write(start, end); + return this.ucs2Write(string, offset); default: throw new Error('Unknown encoding'); diff --git a/test/simple/test-buffer.js b/test/simple/test-buffer.js index 0edb64277be..11bc58b1655 100644 --- a/test/simple/test-buffer.js +++ b/test/simple/test-buffer.js @@ -500,3 +500,8 @@ console.log(z.length) assert.equal(2, z.length); assert.equal(0x66, z[0]); assert.equal(0x6f, z[1]); + + +var b = new SlowBuffer(10); +b.write('あいうえお', 'ucs2'); +assert.equal(b.toString('ucs2'), 'あいうえお'); From eb57d1b9b197a0d5be6047c0d5e653468f73cac7 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sat, 30 Apr 2011 11:38:25 -0700 Subject: [PATCH 18/27] Upgrade V8 to 3.1.8.14 --- deps/v8/src/arm/code-stubs-arm.cc | 8 +++++++- deps/v8/src/builtins.cc | 4 ++-- deps/v8/src/runtime.cc | 2 +- deps/v8/src/version.cc | 2 +- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/deps/v8/src/arm/code-stubs-arm.cc b/deps/v8/src/arm/code-stubs-arm.cc index 84ef53d4dcd..e8f217d276a 100644 --- a/deps/v8/src/arm/code-stubs-arm.cc +++ b/deps/v8/src/arm/code-stubs-arm.cc @@ -3425,6 +3425,8 @@ void TypeRecordingBinaryOpStub::GenerateInt32Stub(MacroAssembler* masm) { // Save the left value on the stack. __ Push(r5, r4); + Label pop_and_call_runtime; + // Allocate a heap number to store the result. heap_number_result = r5; GenerateHeapResultAllocation(masm, @@ -3432,7 +3434,7 @@ void TypeRecordingBinaryOpStub::GenerateInt32Stub(MacroAssembler* masm) { heap_number_map, scratch1, scratch2, - &call_runtime); + &pop_and_call_runtime); // Load the left value from the value saved on the stack. __ Pop(r1, r0); @@ -3440,6 +3442,10 @@ void TypeRecordingBinaryOpStub::GenerateInt32Stub(MacroAssembler* masm) { // Call the C function to handle the double operation. FloatingPointHelper::CallCCodeForDoubleOperation( masm, op_, heap_number_result, scratch1); + + __ bind(&pop_and_call_runtime); + __ Drop(2); + __ b(&call_runtime); } break; diff --git a/deps/v8/src/builtins.cc b/deps/v8/src/builtins.cc index 01e8deb4e14..ff073883c70 100644 --- a/deps/v8/src/builtins.cc +++ b/deps/v8/src/builtins.cc @@ -818,8 +818,8 @@ BUILTIN(ArraySplice) { const int delta = actual_delete_count - item_count; if (actual_start > 0) { - Object** start = elms->data_start(); - memmove(start + delta, start, actual_start * kPointerSize); + AssertNoAllocation no_gc; + MoveElements(&no_gc, elms, delta, elms, 0, actual_start); } elms = LeftTrimFixedArray(elms, delta); diff --git a/deps/v8/src/runtime.cc b/deps/v8/src/runtime.cc index 4fe0e82b329..dd49d727e33 100644 --- a/deps/v8/src/runtime.cc +++ b/deps/v8/src/runtime.cc @@ -2625,7 +2625,7 @@ MUST_USE_RESULT static MaybeObject* StringReplaceRegExpWithEmptyString( end = RegExpImpl::GetCapture(match_info_array, 1); } - int length = subject->length(); + int length = subject_handle->length(); int new_length = length - (end - start); if (new_length == 0) { return Heap::empty_string(); diff --git a/deps/v8/src/version.cc b/deps/v8/src/version.cc index 39328dcb7e5..0673517eb0e 100644 --- a/deps/v8/src/version.cc +++ b/deps/v8/src/version.cc @@ -35,7 +35,7 @@ #define MAJOR_VERSION 3 #define MINOR_VERSION 1 #define BUILD_NUMBER 8 -#define PATCH_LEVEL 10 +#define PATCH_LEVEL 14 #define CANDIDATE_VERSION false // Define SONAME to have the SCons build the put a specific SONAME into the From 82bc25d5ada29b8d60787259b13feb39eed3296e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 1 May 2011 16:34:05 +0200 Subject: [PATCH 19/27] Remove oprofile flags in wscript. V8 3.1.5 (commit 550f73a) dropped oprofile support so don't pass prof=oprofile to scons. See http://codereview.chromium.org/6474037/ Fixes #998. --- wscript | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/wscript b/wscript index a07fceeeab9..7ced8c9bc86 100644 --- a/wscript +++ b/wscript @@ -143,13 +143,6 @@ def set_options(opt): , dest='openssl_libpath' ) - opt.add_option( '--oprofile' - , action='store_true' - , default=False - , help="add oprofile support" - , dest='use_oprofile' - ) - opt.add_option( '--gdb' , action='store_true' , default=False @@ -252,12 +245,8 @@ def configure(conf): conf.env["USE_SHARED_CARES"] = o.shared_cares or o.shared_cares_includes or o.shared_cares_libpath conf.env["USE_SHARED_LIBEV"] = o.shared_libev or o.shared_libev_includes or o.shared_libev_libpath - conf.env["USE_OPROFILE"] = o.use_oprofile conf.env["USE_GDBJIT"] = o.use_gdbjit - if o.use_oprofile: - conf.check(lib=['bfd', 'opagent'], uselib_store="OPROFILE") - conf.check(lib='dl', uselib_store='DL') if not sys.platform.startswith("sunos") and not sys.platform.startswith("cygwin") and not sys.platform.startswith("win32"): conf.env.append_value("CCFLAGS", "-rdynamic") @@ -567,12 +556,7 @@ def v8_cmd(bld, variant): else: snapshot = "" - if bld.env["USE_OPROFILE"]: - profile = "prof=oprofile" - else: - profile = "" - - cmd_R = sys.executable + ' "%s" -j %d -C "%s" -Y "%s" visibility=default mode=%s %s toolchain=%s library=static %s %s' + cmd_R = sys.executable + ' "%s" -j %d -C "%s" -Y "%s" visibility=default mode=%s %s toolchain=%s library=static %s' cmd = cmd_R % ( scons , Options.options.jobs @@ -582,7 +566,6 @@ def v8_cmd(bld, variant): , arch , toolchain , snapshot - , profile ) if bld.env["USE_GDBJIT"]: From 75a0cf970fb48440a93a62796ab1f128fcbe7d76 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Mon, 2 May 2011 12:13:06 -0700 Subject: [PATCH 20/27] cleartextstream.destroy() should destroy socket. This fixes a critical bug see in MJR's production. Very difficult to build a test case. Sometimes HTTPS server gets sockets that are hanging in a half-duplex state. --- lib/stream.js | 22 ++++++++++++++++++++-- lib/tls.js | 5 ----- test/simple/test-https-eof-for-eom.js | 2 +- test/simple/test-stream-pipe-cleanup.js | 4 ++++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lib/stream.js b/lib/stream.js index ca7fc28c024..a5b797482ee 100644 --- a/lib/stream.js +++ b/lib/stream.js @@ -53,7 +53,7 @@ Stream.prototype.pipe = function(dest, options) { dest._pipeCount++; source.on('end', onend); - source.on('close', onend); + source.on('close', onclose); } var didOnEnd = false; @@ -74,6 +74,24 @@ Stream.prototype.pipe = function(dest, options) { dest.end(); } + + function onclose() { + if (didOnEnd) return; + didOnEnd = true; + + dest._pipeCount--; + + // remove the listeners + cleanup(); + + if (dest._pipeCount > 0) { + // waiting for other incoming streams to end. + return; + } + + dest.destroy(); + } + // don't leave dangling pipes when there are errors. function onerror(er) { cleanup(); @@ -117,7 +135,7 @@ Stream.prototype.pipe = function(dest, options) { dest.removeListener('drain', ondrain); source.removeListener('end', onend); - source.removeListener('close', onend); + source.removeListener('close', onclose); dest.removeListener('pause', onpause); dest.removeListener('resume', onresume); diff --git a/lib/tls.js b/lib/tls.js index b34ed95204e..a2496b23837 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -608,12 +608,7 @@ SecurePair.prototype._destroy = function() { self.cleartext.writable = self.cleartext.readable = false; process.nextTick(function() { - self.encrypted.emit('end'); - if (self.encrypted.onend) self.encrypted.onend(); self.encrypted.emit('close'); - - self.cleartext.emit('end'); - if (self.cleartext.onend) self.cleartext.onend(); self.cleartext.emit('close'); }); } diff --git a/test/simple/test-https-eof-for-eom.js b/test/simple/test-https-eof-for-eom.js index 19f6520a6d1..e043c5df8ac 100644 --- a/test/simple/test-https-eof-for-eom.js +++ b/test/simple/test-https-eof-for-eom.js @@ -83,7 +83,7 @@ server.listen(common.PORT, function() { bodyBuffer += s; }); - res.on('end', function() { + res.on('close', function() { console.log('5) Client got "end" event.'); gotEnd = true; }); diff --git a/test/simple/test-stream-pipe-cleanup.js b/test/simple/test-stream-pipe-cleanup.js index 32ecd153dc8..ba3ffe128c9 100644 --- a/test/simple/test-stream-pipe-cleanup.js +++ b/test/simple/test-stream-pipe-cleanup.js @@ -36,6 +36,10 @@ Writable.prototype.end = function () { this.endCalls++; } +Writable.prototype.destroy = function () { + this.endCalls++; +} + function Readable () { this.readable = true; stream.Stream.call(this); From c409aab397816ddfbe76e15fbedb435bf4061e5d Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 4 May 2011 21:41:01 -0700 Subject: [PATCH 21/27] Assert, Debug output in normal default build --- wscript | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/wscript b/wscript index 7ced8c9bc86..3d978d7c785 100644 --- a/wscript +++ b/wscript @@ -521,7 +521,6 @@ def configure(conf): # Configure default variant conf.setenv('default') - conf.env.append_value('CPPFLAGS', '-DNDEBUG') default_compile_flags = ['-g', '-O3'] conf.env.append_value('CCFLAGS', default_compile_flags) conf.env.append_value('CXXFLAGS', default_compile_flags) @@ -716,8 +715,8 @@ def build(bld): native_cc_debug = native_cc.clone("debug") native_cc_debug.rule = javascript_in_c_debug - native_cc.rule = javascript_in_c - + native_cc.rule = javascript_in_c_debug + if bld.env["USE_DTRACE"]: dtrace = bld.new_task_gen( name = "dtrace", From ad487d3226928b7e3783c0d3b703e97a72cb5727 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 5 May 2011 15:40:45 -0700 Subject: [PATCH 22/27] Add on('error') to http request example for indexzero --- doc/api/http.markdown | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/http.markdown b/doc/api/http.markdown index 6cecc821cfb..e16d384b7ad 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -396,6 +396,10 @@ Example: }); }); + req.on('error', function(e) { + console.log('problem with request: ' + e.message); + }); + // write data to request body req.write('data\n'); req.write('data\n'); From fc8afd45c7150385897b867bb2123d81f4bb9943 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 5 May 2011 16:52:05 -0700 Subject: [PATCH 23/27] Fix crash in debugger --- lib/_debugger.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index 859dc2c1af0..adde5004807 100644 --- a/lib/_debugger.js +++ b/lib/_debugger.js @@ -291,7 +291,10 @@ Client.prototype.reqEval = function(expression, cb) { var frame = bt.frames[self.currentFrame]; var evalFrames = frame.scopes.map(function(s) { - return bt.frames[s.index].index; + if (!s) return; + var x = bt.frames[s.index]; + if (!x) return; + return x.index; }); self._reqFramesEval(expression, evalFrames, cb); From 55bff5bab90903783751677ff17b0fab1cd8157f Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 6 May 2011 16:48:44 -0700 Subject: [PATCH 24/27] TLS: simplify logic --- lib/tls.js | 183 +++++++++++++++++++++++++---------------------------- 1 file changed, 86 insertions(+), 97 deletions(-) diff --git a/lib/tls.js b/lib/tls.js index a2496b23837..49ca0c4898a 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -51,7 +51,7 @@ function CryptoStream(pair) { this.readable = this.writable = true; - this._writeState = true; + this._paused = false; this._pending = []; this._pendingCallbacks = []; this._pendingBytes = 0; @@ -90,11 +90,10 @@ CryptoStream.prototype.write = function(data /* , encoding, cb */) { this._pending.push(data); this._pendingCallbacks.push(cb); - this._pendingBytes += data.length; this.pair._writeCalled = true; - this.pair._cycle(); + this.pair.cycle(); return this._pendingBytes < 128 * 1024; }; @@ -102,14 +101,14 @@ CryptoStream.prototype.write = function(data /* , encoding, cb */) { CryptoStream.prototype.pause = function() { debug('paused ' + (this == this.pair.cleartext ? 'cleartext' : 'encrypted')); - this._writeState = false; + this._paused = true; }; CryptoStream.prototype.resume = function() { debug('resume ' + (this == this.pair.cleartext ? 'cleartext' : 'encrypted')); - this._writeState = true; - this.pair._cycle(); + this._paused = false; + this.pair.cycle(); }; @@ -147,8 +146,8 @@ function parseCertString(s) { CryptoStream.prototype.getPeerCertificate = function() { - if (this.pair._ssl) { - var c = this.pair._ssl.getPeerCertificate(); + if (this.pair.ssl) { + var c = this.pair.ssl.getPeerCertificate(); if (c) { if (c.issuer) c.issuer = parseCertString(c.issuer); @@ -162,8 +161,8 @@ CryptoStream.prototype.getPeerCertificate = function() { CryptoStream.prototype.getCipher = function(err) { - if (this.pair._ssl) { - return this.pair._ssl.getCurrentCipher(); + if (this.pair.ssl) { + return this.pair.ssl.getCurrentCipher(); } else { return null; } @@ -171,23 +170,22 @@ CryptoStream.prototype.getCipher = function(err) { CryptoStream.prototype.end = function(d) { - if (this.writable) { - if (this.pair._done) return; + if (this.pair._doneFlag) return; + if (!this.writable) return; - if (d) { - this.write(d); - } - - this._pending.push(END_OF_FILE); - this._pendingCallbacks.push(null); - - // If this is an encrypted stream then we need to disable further 'data' - // events. - - this.writable = false; - - this.pair._cycle(); + if (d) { + this.write(d); } + + this._pending.push(END_OF_FILE); + this._pendingCallbacks.push(null); + + // If this is an encrypted stream then we need to disable further 'data' + // events. + + this.writable = false; + + this.pair.cycle(); }; @@ -201,8 +199,8 @@ CryptoStream.prototype.destroySoon = function(err) { CryptoStream.prototype.destroy = function(err) { - if (this.pair._done) return; - this.pair._destroy(); + if (this.pair._doneFlag) return; + this.pair.destroy(); }; @@ -211,9 +209,9 @@ CryptoStream.prototype._done = function() { if (this.pair.cleartext._doneFlag && this.pair.encrypted._doneFlag && - !this.pair._done) { + !this.pair._doneFlag) { // If both streams are done: - this.pair._destroy(); + this.pair.destroy(); } }; @@ -241,7 +239,7 @@ CryptoStream.prototype._push = function() { return; } - while (this._writeState == true) { + while (!this._paused) { var bytesRead = 0; var chunkBytes = 0; var pool = new Buffer(16 * 4096); // alloc every time? @@ -249,18 +247,18 @@ CryptoStream.prototype._push = function() { do { chunkBytes = this._pusher(pool, bytesRead, pool.length - bytesRead); - if (this.pair._ssl && this.pair._ssl.error) { - this.pair._error(); + if (this.pair.ssl && this.pair.ssl.error) { + this.pair.error(); return; } - this.pair._maybeInitFinished(); + this.pair.maybeInitFinished(); if (chunkBytes >= 0) { bytesRead += chunkBytes; } - } while ((chunkBytes > 0) && (bytesRead < pool.length)); + } while (chunkBytes > 0 && bytesRead < pool.length); assert(bytesRead >= 0); @@ -313,7 +311,7 @@ CryptoStream.prototype._pull = function() { assert(havePending || this._pendingBytes == 0); while (this._pending.length > 0) { - if (!this.pair._ssl) break; + if (!this.pair.ssl) break; var tmp = this._pending.shift(); var cb = this._pendingCallbacks.shift(); @@ -330,7 +328,7 @@ CryptoStream.prototype._pull = function() { assert(this === this.pair.cleartext); debug('end cleartext'); - this.pair._ssl.shutdown(); + this.pair.ssl.shutdown(); // TODO check if we get EAGAIN From shutdown, would have to do it // again. should unshift END_OF_FILE back onto pending and wait for @@ -338,7 +336,7 @@ CryptoStream.prototype._pull = function() { this.pair.encrypted._destroyAfterPush = true; } - this.pair._cycle(); + this.pair.cycle(); this._done() return; } @@ -347,12 +345,12 @@ CryptoStream.prototype._pull = function() { var rv = this._puller(tmp); - if (this.pair._ssl && this.pair._ssl.error) { - this.pair._error(); + if (this.pair.ssl && this.pair.ssl.error) { + this.pair.error(); return; } - this.pair._maybeInitFinished(); + this.pair.maybeInitFinished(); if (rv === 0 || rv < 0) { this._pending.unshift(tmp); @@ -384,8 +382,8 @@ util.inherits(CleartextStream, CryptoStream); CleartextStream.prototype._internallyPendingBytes = function() { - if (this.pair._ssl) { - return this.pair._ssl.clearPending(); + if (this.pair.ssl) { + return this.pair.ssl.clearPending(); } else { return 0; } @@ -394,14 +392,14 @@ CleartextStream.prototype._internallyPendingBytes = function() { CleartextStream.prototype._puller = function(b) { debug('clearIn ' + b.length + ' bytes'); - return this.pair._ssl.clearIn(b, 0, b.length); + return this.pair.ssl.clearIn(b, 0, b.length); }; CleartextStream.prototype._pusher = function(pool, offset, length) { debug('reading from clearOut'); - if (!this.pair._ssl) return -1; - return this.pair._ssl.clearOut(pool, offset, length); + if (!this.pair.ssl) return -1; + return this.pair.ssl.clearOut(pool, offset, length); }; @@ -412,8 +410,8 @@ util.inherits(EncryptedStream, CryptoStream); EncryptedStream.prototype._internallyPendingBytes = function() { - if (this.pair._ssl) { - return this.pair._ssl.encPending(); + if (this.pair.ssl) { + return this.pair.ssl.encPending(); } else { return 0; } @@ -422,14 +420,14 @@ EncryptedStream.prototype._internallyPendingBytes = function() { EncryptedStream.prototype._puller = function(b) { debug('writing from encIn'); - return this.pair._ssl.encIn(b, 0, b.length); + return this.pair.ssl.encIn(b, 0, b.length); }; EncryptedStream.prototype._pusher = function(pool, offset, length) { debug('reading from encOut'); - if (!this.pair._ssl) return -1; - return this.pair._ssl.encOut(pool, offset, length); + if (!this.pair.ssl) return -1; + return this.pair.ssl.encOut(pool, offset, length); }; @@ -453,9 +451,7 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized) { this._isServer = isServer ? true : false; this._encWriteState = true; this._clearWriteState = true; - this._done = false; - - var crypto = require('crypto'); + this._doneFlag = false; if (!credentials) { this.credentials = crypto.createCredentials(); @@ -473,7 +469,7 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized) { this._rejectUnauthorized = rejectUnauthorized ? true : false; this._requestCert = requestCert ? true : false; - this._ssl = new Connection(this.credentials.context, + this.ssl = new Connection(this.credentials.context, this._isServer ? true : false, this._requestCert, this._rejectUnauthorized); @@ -486,8 +482,8 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized) { this.encrypted = new EncryptedStream(this); process.nextTick(function() { - self._ssl.start(); - self._cycle(); + self.ssl.start(); + self.cycle(); }); } @@ -535,59 +531,54 @@ exports.createSecurePair = function(credentials, * Because it is also called everywhere, we also check if the connection has * completed negotiation and emit 'secure' from here if it has. */ -SecurePair.prototype._cycle = function(depth) { - depth = depth ? depth : 0; - if (this._done) { - return; - } +SecurePair.prototype.cycle = function(depth) { + if (this._doneFlag) return; - if(depth == 0) this._writeCalled = false; + depth = depth ? depth : 0; + + if (depth == 0) this._writeCalled = false; var established = this._secureEstablished; - if (!this._cycleEncryptedPullLock) { - this._cycleEncryptedPullLock = true; + if (!this.cycleEncryptedPullLock) { + this.cycleEncryptedPullLock = true; debug("encrypted._pull"); this.encrypted._pull(); - this._cycleEncryptedPullLock = false; + this.cycleEncryptedPullLock = false; } - if (!this._cycleCleartextPullLock) { - this._cycleCleartextPullLock = true; + if (!this.cycleCleartextPullLock) { + this.cycleCleartextPullLock = true; debug("cleartext._pull"); this.cleartext._pull(); - this._cycleCleartextPullLock = false; + this.cycleCleartextPullLock = false; } - if (!this._cycleCleartextPushLock) { - this._cycleCleartextPushLock = true; + if (!this.cycleCleartextPushLock) { + this.cycleCleartextPushLock = true; debug("cleartext._push"); this.cleartext._push(); - this._cycleCleartextPushLock = false; + this.cycleCleartextPushLock = false; } - if (!this._cycleEncryptedPushLock) { - this._cycleEncryptedPushLock = true; + if (!this.cycleEncryptedPushLock) { + this.cycleEncryptedPushLock = true; debug("encrypted._push"); this.encrypted._push(); - this._cycleEncryptedPushLock = false; - } - - if (this._done) { - return; + this.cycleEncryptedPushLock = false; } if ((!established && this._secureEstablished) || (depth == 0 && this._writeCalled)) { // If we were not established but now we are, let's cycle again. // Or if there is some data to write... - this._cycle(depth + 1); + this.cycle(depth + 1); } }; -SecurePair.prototype._maybeInitFinished = function() { - if (this._ssl && !this._secureEstablished && this._ssl.isInitFinished()) { +SecurePair.prototype.maybeInitFinished = function() { + if (this.ssl && !this._secureEstablished && this.ssl.isInitFinished()) { this._secureEstablished = true; debug('secure established'); this.emit('secure'); @@ -595,14 +586,14 @@ SecurePair.prototype._maybeInitFinished = function() { }; -SecurePair.prototype._destroy = function() { +SecurePair.prototype.destroy = function() { var self = this; - if (!this._done) { - this._done = true; - this._ssl.error = null; - this._ssl.close(); - this._ssl = null; + if (!this._doneFlag) { + this._doneFlag = true; + this.ssl.error = null; + this.ssl.close(); + this.ssl = null; self.encrypted.writable = self.encrypted.readable = false; self.cleartext.writable = self.cleartext.readable = false; @@ -612,23 +603,21 @@ SecurePair.prototype._destroy = function() { self.cleartext.emit('close'); }); } - - this._cycle(); }; -SecurePair.prototype._error = function() { +SecurePair.prototype.error = function() { if (!this._secureEstablished) { - this._destroy(); + this.destroy(); } else { - var err = this._ssl.error; - this._ssl.error = null; + var err = this.ssl.error; + this.ssl.error = null; if (this._isServer && this._rejectUnauthorized && /peer did not return a certificate/.test(err.message)) { // Not really an error. - this._destroy(); + this.destroy(); } else { this.cleartext.emit('error', err); } @@ -749,13 +738,13 @@ function Server(/* [options], listener */) { cleartext._controlReleased = true; self.emit('secureConnection', pair.cleartext, pair.encrypted); } else { - var verifyError = pair._ssl.verifyError(); + var verifyError = pair.ssl.verifyError(); if (verifyError) { pair.cleartext.authorizationError = verifyError; if (self.rejectUnauthorized) { socket.destroy(); - pair._destroy(); + pair.destroy(); } else { cleartext._controlReleased = true; self.emit('secureConnection', pair.cleartext, pair.encrypted); @@ -851,7 +840,7 @@ exports.connect = function(port /* host, options, cb */) { socket.connect(port, host); pair.on('secure', function() { - var verifyError = pair._ssl.verifyError(); + var verifyError = pair.ssl.verifyError(); if (verifyError) { cleartext.authorized = false; From 5ab3ea3955dd90876ae8c1e9ce67395c767f787c Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Fri, 6 May 2011 16:05:02 -0700 Subject: [PATCH 25/27] Point changelog to correct branch Fixes #1002. Thanks cjavapro. --- doc/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/index.html b/doc/index.html index 1cf7e0e4b61..42dcc5c172f 100644 --- a/doc/index.html +++ b/doc/index.html @@ -24,7 +24,7 @@
    1. Download
      2. -
    2. ChangeLog
      3. +
    3. ChangeLog
    4. About
    5. v0.4.7 docs

      6. From c2b5ea218ca7ab11f0b0967b4037d9f109794096 Mon Sep 17 00:00:00 2001 From: Marcel Laverdet Date: Wed, 4 May 2011 06:51:15 +0900 Subject: [PATCH 26/27] Attempt to connect to debug process more than once The debugger would give up after only 100ms but on my system this timeout isn't enough. The startup process is now modified to try 6 times every 50ms instead. Fixes #1010. --- lib/_debugger.js | 57 +++++++++++++++++++++++++++++++----------------- src/node.cc | 2 +- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/lib/_debugger.js b/lib/_debugger.js index adde5004807..2714fc52dc8 100644 --- a/lib/_debugger.js +++ b/lib/_debugger.js @@ -1046,19 +1046,16 @@ Interface.prototype.trySpawn = function(cb) { this.pause(); - setTimeout(function() { - process.stdout.write('connecting...'); - var client = self.client = new Client(); - client.connect(exports.port); + var client = self.client = new Client(); + var connectionAttempts = 0; - client.once('ready', function() { - process.stdout.write('ok\r\n'); + client.once('ready', function() { + process.stdout.write(' ok\r\n'); - // since we did debug-brk, we're hitting a break point immediately - // continue before anything else. - client.reqContinue(function() { - if (cb) cb(); - }); + // since we did debug-brk, we're hitting a break point immediately + // continue before anything else. + client.reqContinue(function() { + if (cb) cb(); }); client.on('close', function() { @@ -1067,17 +1064,37 @@ Interface.prototype.trySpawn = function(cb) { self.killChild(); if (!self.quitting) self.term.prompt(); }); + }); - client.on('unhandledResponse', function(res) { - console.log('\r\nunhandled res:'); - console.log(res); - self.term.prompt(); - }); + client.on('unhandledResponse', function(res) { + console.log('\r\nunhandled res:'); + console.log(res); + self.term.prompt(); + }); - client.on('break', function(res) { - self.handleBreak(res.body); - }); - }, 100); + client.on('break', function(res) { + self.handleBreak(res.body); + }); + + client.on('error', connectError); + function connectError() { + // If it's failed to connect 4 times then don't catch the next error + if (connectionAttempts >= 4) { + client.removeListener('error', connectError); + } + setTimeout(attemptConnect, 50); + } + + function attemptConnect() { + ++connectionAttempts; + process.stdout.write('.'); + client.connect(exports.port); + } + + setTimeout(function() { + process.stdout.write('connecting..'); + attemptConnect(); + }, 50); }; diff --git a/src/node.cc b/src/node.cc index fa64bb9902d..e76b0e217da 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2222,7 +2222,7 @@ static void EnableDebug(bool wait_connect) { assert(r); // Print out some information. - fprintf(stderr, "debugger listening on port %d\r\n", debug_port); + fprintf(stderr, "debugger listening on port %d", debug_port); } From 110f06578da9fa4f5389269e9c86e56558a2d7dc Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Sat, 7 May 2011 12:30:53 -0700 Subject: [PATCH 27/27] Agent socket errors bubble up to req only if req exists Fixes #836. --- lib/http.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/http.js b/lib/http.js index 1e7512d5425..72944148938 100644 --- a/lib/http.js +++ b/lib/http.js @@ -1194,13 +1194,12 @@ Agent.prototype._establishNewConnection = function() { req = self.queue.shift(); assert(req._queue === self.queue); req._queue = null; - } else { - // No requests on queue? Where is the request - assert(0); } - req.emit('error', err); - req._hadError = true; // hacky + if (req) { + req.emit('error', err); + req._hadError = true; // hacky + } // clean up so that agent can handle new requests parser.finish();