+12.9.1
12.9.0
12.8.1
12.8.0
@@ -39,6 +40,28 @@
* [io.js](CHANGELOG_IOJS.md)
* [Archive](CHANGELOG_ARCHIVE.md)
+
+## 2019-08-26, Version 12.9.1 (Current), @targos
+
+### Notable changes
+
+This release fixes two regressions in the **http** module:
+
+* Fixes an event listener leak in the HTTP client. This resulted in lots of
+ warnings during npm/yarn installs (Robert Nagy) [#29245](https://github.com/nodejs/node/pull/29245).
+* Fixes a regression preventing the `'end'` event from being emitted for
+ keepalive requests in case the full body was not parsed (Matteo Collina) [#29263](https://github.com/nodejs/node/pull/29263).
+
+### Commits
+
+* [[`3cc8fca299`](https://github.com/nodejs/node/commit/3cc8fca299)] - **crypto**: handle i2d\_SSL\_SESSION() error return (Ben Noordhuis) [#29225](https://github.com/nodejs/node/pull/29225)
+* [[`ae0a0e97ba`](https://github.com/nodejs/node/commit/ae0a0e97ba)] - **http**: reset parser.incoming when server request is finished (Anna Henningsen) [#29297](https://github.com/nodejs/node/pull/29297)
+* [[`dedbd119c5`](https://github.com/nodejs/node/commit/dedbd119c5)] - **http**: fix event listener leak (Robert Nagy) [#29245](https://github.com/nodejs/node/pull/29245)
+* [[`f8f8754d43`](https://github.com/nodejs/node/commit/f8f8754d43)] - ***Revert*** "**http**: reset parser.incoming when server response is finished" (Matteo Collina) [#29263](https://github.com/nodejs/node/pull/29263)
+* [[`a6abfcb423`](https://github.com/nodejs/node/commit/a6abfcb423)] - **src**: remove unused using declarations (Daniel Bevenius) [#29222](https://github.com/nodejs/node/pull/29222)
+* [[`ff6330a6ac`](https://github.com/nodejs/node/commit/ff6330a6ac)] - **test**: fix 'timeout' typos (cjihrig) [#29234](https://github.com/nodejs/node/pull/29234)
+* [[`3c7a1a9090`](https://github.com/nodejs/node/commit/3c7a1a9090)] - **test, http**: add regression test for keepalive 'end' event (Matteo Collina) [#29263](https://github.com/nodejs/node/pull/29263)
+
## 2019-08-20, Version 12.9.0 (Current), @targos
diff --git a/lib/_http_client.js b/lib/_http_client.js
index cea02eefa30763..a5a869b327c79b 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -502,6 +502,7 @@ function socketOnData(d) {
!statusIsInformational(parser.incoming.statusCode)) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
+ socket.removeListener('drain', ondrain);
freeParser(parser, req, socket);
}
}
@@ -609,6 +610,7 @@ function responseKeepAlive(res, req) {
}
socket.removeListener('close', socketCloseListener);
socket.removeListener('error', socketErrorListener);
+ socket.removeListener('drain', ondrain);
socket.once('error', freeSocketErrorListener);
// There are cases where _handle === null. Avoid those. Passing null to
// nextTick() will call getDefaultTriggerAsyncId() to retrieve the id.
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 68492f3eaaeec0..58f0973ddd8b02 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -614,6 +614,19 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) {
}
}
+function clearIncoming(req) {
+ req = req || this;
+ const parser = req.socket && req.socket.parser;
+ // Reset the .incoming property so that the request object can be gc'ed.
+ if (parser && parser.incoming === req) {
+ if (req.readableEnded) {
+ parser.incoming = null;
+ } else {
+ req.on('end', clearIncoming);
+ }
+ }
+}
+
function resOnFinish(req, res, socket, state, server) {
// Usually the first incoming element should be our request. it may
// be that in the case abortIncoming() was called that the incoming
@@ -621,8 +634,7 @@ function resOnFinish(req, res, socket, state, server) {
assert(state.incoming.length === 0 || state.incoming[0] === req);
state.incoming.shift();
- // Reset the .incoming property so that the request object can be gc'ed.
- if (socket.parser) socket.parser.incoming = null;
+ clearIncoming(req);
// If the user never called req.read(), and didn't pipe() or
// .resume() or .on('data'), then we call req._dump() so that the
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 69c110d6be2d22..2d30e0b8038ce4 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -49,7 +49,6 @@ using v8::HandleScope;
using v8::IndexedPropertyHandlerConfiguration;
using v8::Integer;
using v8::Isolate;
-using v8::Just;
using v8::Local;
using v8::Maybe;
using v8::MaybeLocal;
@@ -68,7 +67,6 @@ using v8::ScriptCompiler;
using v8::ScriptOrigin;
using v8::ScriptOrModule;
using v8::String;
-using v8::Symbol;
using v8::Uint32;
using v8::UnboundScript;
using v8::Value;
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index 70da2e310ea15c..78d89b71f4c317 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -2317,11 +2317,12 @@ void SSLWrap::GetSession(const FunctionCallbackInfo& args) {
return;
int slen = i2d_SSL_SESSION(sess, nullptr);
- CHECK_GT(slen, 0);
+ if (slen <= 0)
+ return; // Invalid or malformed session.
AllocatedBuffer sbuf = env->AllocateManaged(slen);
unsigned char* p = reinterpret_cast(sbuf.data());
- i2d_SSL_SESSION(sess, &p);
+ CHECK_LT(0, i2d_SSL_SESSION(sess, &p));
args.GetReturnValue().Set(sbuf.ToBuffer().ToLocalChecked());
}
diff --git a/src/node_version.h b/src/node_version.h
index e3158f2d72dfea..2a241bf23d74cc 100644
--- a/src/node_version.h
+++ b/src/node_version.h
@@ -29,7 +29,7 @@
#define NODE_VERSION_IS_LTS 0
#define NODE_VERSION_LTS_CODENAME ""
-#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/test/common/index.js b/test/common/index.js
index 13604d06e14a36..98a26872223cb9 100644
--- a/test/common/index.js
+++ b/test/common/index.js
@@ -401,7 +401,7 @@ function canCreateSymLink() {
'System32', 'whoami.exe');
try {
- const output = execSync(`${whoamiPath} /priv`, { timout: 1000 });
+ const output = execSync(`${whoamiPath} /priv`, { timeout: 1000 });
return output.includes('SeCreateSymbolicLinkPrivilege');
} catch {
return false;
diff --git a/test/common/tmpdir.js b/test/common/tmpdir.js
index 33b2264a8d69f5..2be6e530275fbd 100644
--- a/test/common/tmpdir.js
+++ b/test/common/tmpdir.js
@@ -28,7 +28,7 @@ function rimrafSync(pathname, { spawn = true } = {}) {
if (spawn && process.platform === 'win32' && st.isDirectory()) {
try {
// Try `rmdir` first.
- execSync(`rmdir /q /s ${pathname}`, { timout: 1000 });
+ execSync(`rmdir /q /s ${pathname}`, { timeout: 1000 });
} catch (e) {
// Attempt failed. Log and carry on.
debug(e);
diff --git a/test/parallel/test-http-agent-keepalive.js b/test/parallel/test-http-agent-keepalive.js
index 5902c5867968cf..8cfc568b1e76e4 100644
--- a/test/parallel/test-http-agent-keepalive.js
+++ b/test/parallel/test-http-agent-keepalive.js
@@ -52,7 +52,7 @@ function get(path, callback) {
port: server.address().port,
agent: agent,
path: path
- }, callback);
+ }, callback).on('socket', common.mustCall(checkListeners));
}
function checkDataAndSockets(body) {
@@ -134,3 +134,12 @@ server.listen(0, common.mustCall(() => {
}));
}));
}));
+
+// Check for listener leaks when reusing sockets.
+function checkListeners(socket) {
+ assert.strictEqual(socket.listenerCount('data'), 1);
+ assert.strictEqual(socket.listenerCount('drain'), 1);
+ assert.strictEqual(socket.listenerCount('error'), 1);
+ // Sockets have onReadableStreamEnd.
+ assert.strictEqual(socket.listenerCount('end'), 2);
+}
diff --git a/test/parallel/test-http-server-keepalive-end.js b/test/parallel/test-http-server-keepalive-end.js
new file mode 100644
index 00000000000000..8ebdecb97c3d8c
--- /dev/null
+++ b/test/parallel/test-http-server-keepalive-end.js
@@ -0,0 +1,38 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+// Make sure that for HTTP keepalive requests, the .on('end') event is emitted
+// on the incoming request object, and that the parser instance does not hold
+// on to that request object afterwards.
+
+const server = createServer(common.mustCall((req, res) => {
+ req.on('end', common.mustCall(() => {
+ const parser = req.socket.parser;
+ assert.strictEqual(parser.incoming, req);
+ process.nextTick(() => {
+ assert.strictEqual(parser.incoming, null);
+ });
+ }));
+ res.end('hello world');
+}));
+
+server.unref();
+
+server.listen(0, common.mustCall(() => {
+ const client = connect(server.address().port);
+
+ const req = [
+ 'POST / HTTP/1.1',
+ `Host: localhost:${server.address().port}`,
+ 'Connection: keep-alive',
+ 'Content-Length: 11',
+ '',
+ 'hello world',
+ ''
+ ].join('\r\n');
+
+ client.end(req);
+}));
diff --git a/test/parallel/test-http-server-keepalive-req-gc.js b/test/parallel/test-http-server-keepalive-req-gc.js
new file mode 100644
index 00000000000000..aa4bf1a3de9c83
--- /dev/null
+++ b/test/parallel/test-http-server-keepalive-req-gc.js
@@ -0,0 +1,47 @@
+// Flags: --expose-gc
+'use strict';
+const common = require('../common');
+const onGC = require('../common/ongc');
+const { createServer } = require('http');
+const { connect } = require('net');
+
+if (common.isWindows) {
+ // TODO(addaleax): Investigate why and remove the skip.
+ common.skip('This test is flaky on Windows.');
+}
+
+// Make sure that for HTTP keepalive requests, the req object can be
+// garbage collected once the request is finished.
+// Refs: https://github.com/nodejs/node/issues/9668
+
+let client;
+const server = createServer(common.mustCall((req, res) => {
+ onGC(req, { ongc: common.mustCall() });
+ req.resume();
+ req.on('end', common.mustCall(() => {
+ setImmediate(() => {
+ client.end();
+ global.gc();
+ });
+ }));
+ res.end('hello world');
+}));
+
+server.unref();
+
+server.listen(0, common.mustCall(() => {
+ client = connect(server.address().port);
+
+ const req = [
+ 'POST / HTTP/1.1',
+ `Host: localhost:${server.address().port}`,
+ 'Connection: keep-alive',
+ 'Content-Length: 11',
+ '',
+ 'hello world',
+ ''
+ ].join('\r\n');
+
+ client.write(req);
+ client.unref();
+}));
|