Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http_parser: move process.binding('http_parser') to internalBinding #22329

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 36 additions & 32 deletions benchmark/http/bench-parser.js
Original file line number Diff line number Diff line change
@@ -1,50 +1,54 @@
'use strict';

const common = require('../common');
const HTTPParser = process.binding('http_parser').HTTPParser;
const REQUEST = HTTPParser.REQUEST;
const kOnHeaders = HTTPParser.kOnHeaders | 0;
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
const kOnBody = HTTPParser.kOnBody | 0;
const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
const CRLF = '\r\n';

const bench = common.createBenchmark(main, {
len: [4, 8, 16, 32],
n: [1e5],
n: [1e5]
}, {
flags: ['--expose-internals', '--no-warnings']
});

function main({ len, n }) {
var header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;

for (var i = 0; i < len; i++) {
header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
const { internalBinding } = require('internal/test/binding');
const { HTTPParser } = internalBinding('http_parser');
const REQUEST = HTTPParser.REQUEST;
const kOnHeaders = HTTPParser.kOnHeaders | 0;
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
const kOnBody = HTTPParser.kOnBody | 0;
const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0;
const CRLF = '\r\n';

function processHeader(header, n) {
const parser = newParser(REQUEST);

bench.start();
for (var i = 0; i < n; i++) {
parser.execute(header, 0, header.length);
parser.reinitialize(REQUEST);
}
bench.end(n);
}
header += CRLF;

processHeader(Buffer.from(header), n);
}
function newParser(type) {
const parser = new HTTPParser(type);

function processHeader(header, n) {
const parser = newParser(REQUEST);
parser.headers = [];

bench.start();
for (var i = 0; i < n; i++) {
parser.execute(header, 0, header.length);
parser.reinitialize(REQUEST);
}
bench.end(n);
}
parser[kOnHeaders] = function() { };
parser[kOnHeadersComplete] = function() { };
parser[kOnBody] = function() { };
parser[kOnMessageComplete] = function() { };

function newParser(type) {
const parser = new HTTPParser(type);
return parser;
}

parser.headers = [];
let header = `GET /hello HTTP/1.1${CRLF}Content-Type: text/plain${CRLF}`;

parser[kOnHeaders] = function() { };
parser[kOnHeadersComplete] = function() { };
parser[kOnBody] = function() { };
parser[kOnMessageComplete] = function() { };
for (var i = 0; i < len; i++) {
header += `X-Filler${i}: ${Math.random().toString(36).substr(2)}${CRLF}`;
}
header += CRLF;

return parser;
processHeader(Buffer.from(header), n);
}
3 changes: 2 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
const util = require('util');
const net = require('net');
const url = require('url');
const { HTTPParser } = process.binding('http_parser');
const { internalBinding } = require('internal/bootstrap/loaders');
const { HTTPParser } = internalBinding('http_parser');
const assert = require('assert').ok;
const {
_checkIsHttpToken: checkIsHttpToken,
Expand Down
3 changes: 2 additions & 1 deletion lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

'use strict';

const { methods, HTTPParser } = process.binding('http_parser');
const { internalBinding } = require('internal/bootstrap/loaders');
const { methods, HTTPParser } = internalBinding('http_parser');

const FreeList = require('internal/freelist');
const { ondrain } = require('internal/http');
Expand Down
3 changes: 2 additions & 1 deletion lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@

const util = require('util');
const net = require('net');
const { HTTPParser } = process.binding('http_parser');
const { internalBinding } = require('internal/bootstrap/loaders');
const { HTTPParser } = internalBinding('http_parser');
const assert = require('assert').ok;
const {
parsers,
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@
// that are whitelisted for access via process.binding()... this is used
// to provide a transition path for modules that are being moved over to
// internalBinding.
const internalBindingWhitelist = new SafeSet(['uv']);
const internalBindingWhitelist = new SafeSet(['uv', 'http_parser']);
process.binding = function binding(name) {
return internalBindingWhitelist.has(name) ?
internalBinding(name) :
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const dgram = require('dgram');
const util = require('util');
const assert = require('assert');

const { internalBinding } = require('internal/bootstrap/loaders');

const { Process } = process.binding('process_wrap');
const { WriteWrap } = process.binding('stream_wrap');
const { Pipe, constants: PipeConstants } = process.binding('pipe_wrap');
Expand All @@ -32,12 +34,10 @@ const { owner_symbol } = require('internal/async_hooks').symbols;
const { convertToValidSignal } = require('internal/util');
const { isUint8Array } = require('internal/util/types');
const spawn_sync = process.binding('spawn_sync');
const { HTTPParser } = process.binding('http_parser');
const { HTTPParser } = internalBinding('http_parser');
const { freeParser } = require('_http_common');
const { kStateSymbol } = require('internal/dgram');

const { internalBinding } = require('internal/bootstrap/loaders');

const {
UV_EACCES,
UV_EAGAIN,
Expand Down
2 changes: 1 addition & 1 deletion src/node_http_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,4 +775,4 @@ void Initialize(Local<Object> target,
} // anonymous namespace
} // namespace node

NODE_BUILTIN_MODULE_CONTEXT_AWARE(http_parser, node::Initialize)
NODE_MODULE_CONTEXT_AWARE_INTERNAL(http_parser, node::Initialize)
17 changes: 11 additions & 6 deletions test/async-hooks/test-httpparser.request.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';

const common = require('../common');
Expand All @@ -6,17 +7,21 @@ const tick = require('./tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const binding = process.binding('http_parser');
const HTTPParser = binding.HTTPParser;
const hooks = initHooks();
hooks.enable();

// The hooks.enable() must come before require('internal/test/binding')
// because internal/test/binding schedules a process warning on nextTick.
// If this order is not preserved, the hooks check will fail because it
// will not be notified about the nextTick creation but will see the
// callback event.
const { internalBinding } = require('internal/test/binding');
const { HTTPParser } = internalBinding('http_parser');

const REQUEST = HTTPParser.REQUEST;

const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;

const hooks = initHooks();

hooks.enable();

const request = Buffer.from(
'GET /hello HTTP/1.1\r\n\r\n'
);
Expand Down
18 changes: 12 additions & 6 deletions test/async-hooks/test-httpparser.response.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Flags: --expose-internals
'use strict';

const common = require('../common');
Expand All @@ -6,17 +7,22 @@ const tick = require('./tick');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const binding = process.binding('http_parser');
const HTTPParser = binding.HTTPParser;
const hooks = initHooks();

hooks.enable();

// The hooks.enable() must come before require('internal/test/binding')
// because internal/test/binding schedules a process warning on nextTick.
// If this order is not preserved, the hooks check will fail because it
// will not be notified about the nextTick creation but will see the
// callback event.
const { internalBinding } = require('internal/test/binding');
const { HTTPParser } = internalBinding('http_parser');

const RESPONSE = HTTPParser.RESPONSE;
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
const kOnBody = HTTPParser.kOnBody | 0;

const hooks = initHooks();

hooks.enable();

const request = Buffer.from(
'HTTP/1.1 200 OK\r\n' +
'Content-Type: text/plain\r\n' +
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-http-parser-bad-ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
// Run this program with valgrind or efence with --expose_gc to expose the
// problem.

// Flags: --expose_gc
// Flags: --expose_gc --expose-internals

require('../common');
const assert = require('assert');
const HTTPParser = process.binding('http_parser').HTTPParser;
const { internalBinding } = require('internal/test/binding');
const { HTTPParser } = internalBinding('http_parser');

const kOnHeaders = HTTPParser.kOnHeaders | 0;
const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-http-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

// Flags: --expose-internals

'use strict';
const { mustCall, mustNotCall } = require('../common');
const assert = require('assert');

const { methods, HTTPParser } = process.binding('http_parser');
const { internalBinding } = require('internal/test/binding');
const { methods, HTTPParser } = internalBinding('http_parser');
const { REQUEST, RESPONSE } = HTTPParser;

const kOnHeaders = HTTPParser.kOnHeaders | 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ const assert = require('assert');
// Assert that whitelisted internalBinding modules are accessible via
// process.binding().
assert(process.binding('uv'));
assert(process.binding('http_parser'));
5 changes: 3 additions & 2 deletions test/sequential/test-async-wrap-getasyncid.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict';
// Flags: --expose-gc
// Flags: --expose-gc --expose-internals

const common = require('../common');
const { internalBinding } = require('internal/test/binding');
const assert = require('assert');
const fs = require('fs');
const fsPromises = fs.promises;
Expand Down Expand Up @@ -146,7 +147,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check


{
const HTTPParser = process.binding('http_parser').HTTPParser;
const { HTTPParser } = internalBinding('http_parser');
testInitialized(new HTTPParser(), 'HTTPParser');
}

Expand Down
4 changes: 3 additions & 1 deletion test/sequential/test-http-regr-gh-2928.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// This test is designed to fail with a segmentation fault in Node.js 4.1.0 and
// execute without issues in Node.js 4.1.1 and up.

// Flags: --expose-internals
'use strict';
const common = require('../common');
const assert = require('assert');
const httpCommon = require('_http_common');
const HTTPParser = process.binding('http_parser').HTTPParser;
const { internalBinding } = require('internal/test/binding');
const { HTTPParser } = internalBinding('http_parser');
const net = require('net');

const COUNT = httpCommon.parsers.max + 1;
Expand Down