From 6b20f5d96654ca5a3856514180588703b1ab124a Mon Sep 17 00:00:00 2001 From: Stefan Rusu Date: Thu, 14 Jul 2011 12:52:43 +0300 Subject: [PATCH 1/6] Fixes #1304. The Connection instance may be destroyed by abort() when process.nextTick is executed. --- lib/tls.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/tls.js b/lib/tls.js index 49c1ce84f8e..e83ae4ef936 100644 --- a/lib/tls.js +++ b/lib/tls.js @@ -530,7 +530,10 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized, this.encrypted = new EncryptedStream(this); process.nextTick(function() { - self.ssl.start(); + /* The Connection may be destroyed by an abort call */ + if (self.ssl) { + self.ssl.start(); + } self.cycle(); }); } From e901998d0d50fdde19f1c4dcfaf162cfc1901848 Mon Sep 17 00:00:00 2001 From: Stefan Rusu Date: Thu, 14 Jul 2011 15:33:04 +0300 Subject: [PATCH 2/6] Fixes #1085. The agent end event may call detachSocket() after the socket is detached and destroyed by abort(). This patch avoids that behavior. --- lib/http.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/http.js b/lib/http.js index af36d5f01fc..b61be65386d 100644 --- a/lib/http.js +++ b/lib/http.js @@ -1325,7 +1325,10 @@ Agent.prototype._establishNewConnection = function() { debug('AGENT socket keep-alive'); } - req.detachSocket(socket); + // The socket may already be detached and destroyed by an abort call + if (socket._httpMessage) { + req.detachSocket(socket); + } assert(!socket._httpMessage); From e25f41746878b359fe869aa112a87ac230063789 Mon Sep 17 00:00:00 2001 From: Stefan Rusu Date: Thu, 14 Jul 2011 17:26:25 +0300 Subject: [PATCH 3/6] Adds a regression test for #1304. --- test/simple/test-regress-GH-1304.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/simple/test-regress-GH-1304.js diff --git a/test/simple/test-regress-GH-1304.js b/test/simple/test-regress-GH-1304.js new file mode 100644 index 00000000000..90fd0939676 --- /dev/null +++ b/test/simple/test-regress-GH-1304.js @@ -0,0 +1,19 @@ +// Regression test for GH-1304 +// https://github.com/joyent/node/issues/1304 +// +// This test works by creating a HTTPS connection to a server, then aborting +// it. The purpose of this test is to have a tls module thou shalt not fail if +// the request is aborted. + +var common = require('../common'); +var https = require('https'); + +var opt = { + host: 'encrypted.google.com', + path: '/' +}; + +var req = https.get(opt, function (res) { +}); + +req.abort(); From 54a399dde4a58f7720537073940235af329193c4 Mon Sep 17 00:00:00 2001 From: Stefan Rusu Date: Thu, 14 Jul 2011 17:30:19 +0300 Subject: [PATCH 4/6] Forgot to use space indenting for the #1304 regression test. --- test/simple/test-regress-GH-1304.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/simple/test-regress-GH-1304.js b/test/simple/test-regress-GH-1304.js index 90fd0939676..ddc22ac8dab 100644 --- a/test/simple/test-regress-GH-1304.js +++ b/test/simple/test-regress-GH-1304.js @@ -9,8 +9,8 @@ var common = require('../common'); var https = require('https'); var opt = { - host: 'encrypted.google.com', - path: '/' + host: 'encrypted.google.com', + path: '/' }; var req = https.get(opt, function (res) { From 31fd5685f614cbc654025f628b80575dd58b6be7 Mon Sep 17 00:00:00 2001 From: Stefan Rusu Date: Thu, 14 Jul 2011 17:43:07 +0300 Subject: [PATCH 5/6] Adds a regression test for #1085. --- test/simple/test-regress-GH-1085.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 test/simple/test-regress-GH-1085.js diff --git a/test/simple/test-regress-GH-1085.js b/test/simple/test-regress-GH-1085.js new file mode 100644 index 00000000000..5c06975f58c --- /dev/null +++ b/test/simple/test-regress-GH-1085.js @@ -0,0 +1,21 @@ +// Regression test for GH-1085 +// https://github.com/joyent/node/issues/1085 +// +// This test works by requestig a small file over HTTP and aborting the request +// into the data listener of the response. The purpose of this test is to make +// an abort call into the response data listner thou shalt not throw assertion +// errors in detachSocket(). + +var common = require('../common'); +var http = require('http'); + +var opt = { + host: 'nodejs.org', + path: '/favicon.ico' +}; + +var req = http.get(opt, function (res) { + res.addListener('data', function (chunk) { + req.abort(); + }); +}); From a9fd8ab59f1f6279969306f51d785f069ea4abc5 Mon Sep 17 00:00:00 2001 From: SaltwaterC Date: Thu, 14 Jul 2011 20:33:41 +0300 Subject: [PATCH 6/6] Updates the regression tests for #1085, #1304 as suggested by @koichik. --- test/simple/test-regress-GH-1085.js | 26 ++++++++++++++++++++------ test/simple/test-regress-GH-1304.js | 25 ++++++++++++++++++++----- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/test/simple/test-regress-GH-1085.js b/test/simple/test-regress-GH-1085.js index 5c06975f58c..1d94726c65f 100644 --- a/test/simple/test-regress-GH-1085.js +++ b/test/simple/test-regress-GH-1085.js @@ -1,12 +1,26 @@ -// Regression test for GH-1085 -// https://github.com/joyent/node/issues/1085 +// Copyright Joyent, Inc. and other Node contributors. // -// This test works by requestig a small file over HTTP and aborting the request -// into the data listener of the response. The purpose of this test is to make -// an abort call into the response data listner thou shalt not throw assertion -// errors in detachSocket(). +// 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 opt = { diff --git a/test/simple/test-regress-GH-1304.js b/test/simple/test-regress-GH-1304.js index ddc22ac8dab..511c266a9b5 100644 --- a/test/simple/test-regress-GH-1304.js +++ b/test/simple/test-regress-GH-1304.js @@ -1,11 +1,26 @@ -// Regression test for GH-1304 -// https://github.com/joyent/node/issues/1304 +// Copyright Joyent, Inc. and other Node contributors. // -// This test works by creating a HTTPS connection to a server, then aborting -// it. The purpose of this test is to have a tls module thou shalt not fail if -// the request is aborted. +// 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 https = require('https'); var opt = {