diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index a079e028e7f..fc4e0db9ec4 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -201,8 +201,6 @@ class Parser { * @param {*} llhttp */ constructor (client, socket, { exports }) { - assert(Number.isFinite(client[kMaxHeadersSize]) && client[kMaxHeadersSize] > 0) - this.llhttp = exports this.ptr = this.llhttp.llhttp_alloc(constants.TYPE.RESPONSE) this.client = client diff --git a/lib/dispatcher/client.js b/lib/dispatcher/client.js index 65380f19167..3939b89b5b4 100644 --- a/lib/dispatcher/client.js +++ b/lib/dispatcher/client.js @@ -1,5 +1,3 @@ -// @ts-check - 'use strict' const assert = require('node:assert') @@ -60,6 +58,13 @@ const connectH2 = require('./client-h2.js') const kClosedResolve = Symbol('kClosedResolve') +const getDefaultNodeMaxHeaderSize = http && + http.maxHeaderSize && + Number.isInteger(http.maxHeaderSize) && + http.maxHeaderSize > 0 + ? () => http.maxHeaderSize + : () => { throw new InvalidArgumentError('http module not available or http.maxHeaderSize invalid') } + const noop = () => {} function getPipelining (client) { @@ -123,8 +128,14 @@ class Client extends DispatcherBase { throw new InvalidArgumentError('unsupported maxKeepAliveTimeout, use keepAliveMaxTimeout instead') } - if (maxHeaderSize != null && !Number.isFinite(maxHeaderSize)) { - throw new InvalidArgumentError('invalid maxHeaderSize') + if (maxHeaderSize != null) { + if (!Number.isInteger(maxHeaderSize) || maxHeaderSize < 1) { + throw new InvalidArgumentError('invalid maxHeaderSize') + } + } else { + // If maxHeaderSize is not provided, use the default value from the http module + // or if that is not available, throw an error. + maxHeaderSize = getDefaultNodeMaxHeaderSize() } if (socketPath != null && typeof socketPath !== 'string') { @@ -204,7 +215,7 @@ class Client extends DispatcherBase { this[kUrl] = util.parseOrigin(url) this[kConnector] = connect this[kPipelining] = pipelining != null ? pipelining : 1 - this[kMaxHeadersSize] = maxHeaderSize || http.maxHeaderSize + this[kMaxHeadersSize] = maxHeaderSize this[kKeepAliveDefaultTimeout] = keepAliveTimeout == null ? 4e3 : keepAliveTimeout this[kKeepAliveMaxTimeout] = keepAliveMaxTimeout == null ? 600e3 : keepAliveMaxTimeout this[kKeepAliveTimeoutThreshold] = keepAliveTimeoutThreshold == null ? 2e3 : keepAliveTimeoutThreshold diff --git a/test/client-errors.js b/test/client-errors.js deleted file mode 100644 index 0c935d7cdac..00000000000 --- a/test/client-errors.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict' - -const { tspl } = require('@matteo.collina/tspl') -const { test, after } = require('node:test') -const { Client } = require('..') -const net = require('node:net') - -// TODO: move to test/node-test/client-connect.js -test('parser error', async (t) => { - t = tspl(t, { plan: 2 }) - - const server = net.createServer() - server.once('connection', (socket) => { - socket.write('asd\n\r213123') - }) - after(() => server.close()) - - server.listen(0, () => { - const client = new Client(`http://localhost:${server.address().port}`) - after(() => client.destroy()) - - client.request({ path: '/', method: 'GET' }, (err) => { - t.ok(err) - client.close((err) => { - t.ifError(err) - }) - }) - }) - - await t.completed -}) diff --git a/test/client-node-max-header-size.js b/test/client-node-max-header-size.js index 379c5e6970a..4c23113dde8 100644 --- a/test/client-node-max-header-size.js +++ b/test/client-node-max-header-size.js @@ -41,4 +41,23 @@ describe("Node.js' --max-http-header-size cli option", () => { await t.completed }) + + test('--max-http-header-size with Client API', async (t) => { + t = tspl(t, { plan: 6 }) + const command = 'node -e "new (require(\'.\').Client)(new URL(\'http://localhost:200\'))"' + + exec(`${command} --max-http-header-size=0`, { stdio: 'pipe' }, (err, stdout, stderr) => { + t.strictEqual(err.code, 1) + t.strictEqual(stdout, '') + t.match(stderr, /http module not available or http.maxHeaderSize invalid/, '--max-http-header-size=0 should result in an Error when using the Client API') + }) + + exec(command, { stdio: 'pipe' }, (err, stdout, stderr) => { + t.ifError(err) + t.strictEqual(stdout, '') + t.strictEqual(stderr, '', 'default max-http-header-size should not throw') + }) + + await t.completed + }) }) diff --git a/test/node-test/client-errors.js b/test/node-test/client-errors.js index d53e9ffc146..39f04678dc6 100644 --- a/test/node-test/client-errors.js +++ b/test/node-test/client-errors.js @@ -1,12 +1,13 @@ 'use strict' -const { test } = require('node:test') const assert = require('node:assert') +const https = require('node:https') +const net = require('node:net') +const { Readable } = require('node:stream') +const { test, after } = require('node:test') const { Client, Pool, errors } = require('../..') const { createServer } = require('node:http') -const https = require('node:https') const pem = require('https-pem') -const { Readable } = require('node:stream') const { tspl } = require('@matteo.collina/tspl') const { kSocket } = require('../../lib/core/symbols') @@ -399,6 +400,46 @@ test('invalid options throws', (t, done) => { assert.strictEqual(err.message, 'invalid maxHeaderSize') } + try { + new Client(new URL('http://localhost:200'), { // eslint-disable-line + maxHeaderSize: 0 + }) + assert.ok(0) + } catch (err) { + assert.ok(err instanceof errors.InvalidArgumentError) + assert.strictEqual(err.message, 'invalid maxHeaderSize') + } + + try { + new Client(new URL('http://localhost:200'), { // eslint-disable-line + maxHeaderSize: 0 + }) + assert.ok(0) + } catch (err) { + assert.ok(err instanceof errors.InvalidArgumentError) + assert.strictEqual(err.message, 'invalid maxHeaderSize') + } + + try { + new Client(new URL('http://localhost:200'), { // eslint-disable-line + maxHeaderSize: -10 + }) + assert.ok(0) + } catch (err) { + assert.ok(err instanceof errors.InvalidArgumentError) + assert.strictEqual(err.message, 'invalid maxHeaderSize') + } + + try { + new Client(new URL('http://localhost:200'), { // eslint-disable-line + maxHeaderSize: 1.5 + }) + assert.ok(0) + } catch (err) { + assert.ok(err instanceof errors.InvalidArgumentError) + assert.strictEqual(err.message, 'invalid maxHeaderSize') + } + try { new Client(1) // eslint-disable-line assert.ok(0) @@ -1324,3 +1365,27 @@ test('SocketError should expose socket details (tls)', async (t) => { await p.completed }) + +test('parser error', async (t) => { + t = tspl(t, { plan: 2 }) + + const server = net.createServer() + server.once('connection', (socket) => { + socket.write('asd\n\r213123') + }) + after(() => server.close()) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + after(() => client.destroy()) + + client.request({ path: '/', method: 'GET' }, (err) => { + t.ok(err) + client.close((err) => { + t.ifError(err) + }) + }) + }) + + await t.completed +})