From 482e23f1fc5b84e8797a753bbc26177870073ea4 Mon Sep 17 00:00:00 2001 From: Jimb Esser Date: Wed, 31 Oct 2018 12:55:00 -0700 Subject: [PATCH 1/2] child_process: allow 'http_parser' monkey patching again Lazy load _http_common and HTTPParser so that the 'http_parser' binding can be monkey patched before any internal modules require it. This also probably improves startup performance minimally for programs that never require the HTTP stack. Fixes: https://github.com/nodejs/node/issues/23716 Fixes: https://github.com/creationix/http-parser-js/issues/57 --- lib/internal/child_process.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index ddb7e58d7c8887..2f19f28b591cd5 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -38,8 +38,6 @@ const { owner_symbol } = require('internal/async_hooks').symbols; const { convertToValidSignal } = require('internal/util'); const { isArrayBufferView } = require('internal/util/types'); const spawn_sync = internalBinding('spawn_sync'); -const { HTTPParser } = internalBinding('http_parser'); -const { freeParser } = require('_http_common'); const { kStateSymbol } = require('internal/dgram'); const { @@ -57,6 +55,10 @@ const { SocketListSend, SocketListReceive } = SocketList; // Lazy loaded for startup performance. let StringDecoder; +// Lazy loaded for startup performance and to allow monkey patching of +// internalBinding('http_parser').HTTPParser. +let freeParser; +let HTTPParser; const MAX_HANDLE_RETRANSMISSIONS = 3; @@ -121,6 +123,12 @@ const handleConversion = { handle.onread = nop; socket._handle = null; socket.setTimeout(0); + + if (freeParser === undefined) + freeParser = require('_http_common').freeParser; + if (HTTPParser === undefined) + HTTPParser = internalBinding('http_parser').HTTPParser; + // In case of an HTTP connection socket, release the associated // resources if (socket.parser && socket.parser instanceof HTTPParser) { From f6c03d2e0b7c90256153c42156e10aa1c05bd173 Mon Sep 17 00:00:00 2001 From: Jimb Esser Date: Mon, 5 Nov 2018 07:57:10 -0800 Subject: [PATCH 2/2] http_parser: test monkey patchability Test that _http_common is not required before the first line of user-level code is run, allowing monkey patchability by modules like http-parser-js. PR-URL: https://github.com/nodejs/node/pull/24006 --- test/parallel/test-http-parser-lazy-loaded.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 test/parallel/test-http-parser-lazy-loaded.js diff --git a/test/parallel/test-http-parser-lazy-loaded.js b/test/parallel/test-http-parser-lazy-loaded.js new file mode 100644 index 00000000000000..c1eb29fb163d00 --- /dev/null +++ b/test/parallel/test-http-parser-lazy-loaded.js @@ -0,0 +1,37 @@ +// Flags: --expose-internals + +'use strict'; + +const { internalBinding } = require('internal/test/binding'); + +// Monkey patch before requiring anything +class DummyParser { + constructor(type) { + this.test_type = type; + } +} +DummyParser.REQUEST = Symbol(); +internalBinding('http_parser').HTTPParser = DummyParser; + +const common = require('../common'); +const assert = require('assert'); +const { spawn } = require('child_process'); +const { parsers } = require('_http_common'); + +// Test _http_common was not loaded before monkey patching +const parser = parsers.alloc(); +assert.strictEqual(parser instanceof DummyParser, true); +assert.strictEqual(parser.test_type, DummyParser.REQUEST); + +if (process.argv[2] !== 'child') { + // Also test in a child process with IPC (specific case of https://github.com/nodejs/node/issues/23716) + const child = spawn(process.execPath, [ + '--expose-internals', __filename, 'child' + ], { + stdio: ['inherit', 'inherit', 'inherit', 'ipc'] + }); + child.on('exit', common.mustCall((code, signal) => { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); + })); +}