From c83d09b1b3594043d0f4c05e1d406071dfd55226 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 23 Feb 2024 22:31:39 +0100 Subject: [PATCH 1/2] test: deflake test-http2-large-write-multiple-requests If the server is not referenced, it might go away too soon and the client may not get enough ends for it to close itself, resulting a timeout. This patch updates the test to simply close the server when enough requests have been processed, and keep the server referenced while the test is ongoing. Drive-by: add more logs to facilitate debugging. --- .../test-http2-large-write-multiple-requests.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-http2-large-write-multiple-requests.js b/test/parallel/test-http2-large-write-multiple-requests.js index 0d65c3479b409d..ba0488fd48f925 100644 --- a/test/parallel/test-http2-large-write-multiple-requests.js +++ b/test/parallel/test-http2-large-write-multiple-requests.js @@ -3,6 +3,10 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +// This tests that the http2 server sends data early when it accumulates +// enough from ongoing requests to avoid DoS as mitigation for +// CVE-2019-9517 and CVE-2019-9511. +// Added by https://github.com/nodejs/node/commit/8a4a193 const fixtures = require('../common/fixtures'); const assert = require('assert'); const http2 = require('http2'); @@ -12,18 +16,21 @@ const content = fixtures.readSync('person-large.jpg'); const server = http2.createServer({ maxSessionMemory: 1000 }); +let streamCount = 0; server.on('stream', (stream, headers) => { stream.respond({ 'content-type': 'image/jpeg', ':status': 200 }); stream.end(content); + console.log('server sends content', ++streamCount); }); server.unref(); server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}/`); + let endCount = 0; let finished = 0; for (let i = 0; i < 100; i++) { const req = client.request({ ':path': '/' }).end(); @@ -32,8 +39,16 @@ server.listen(0, common.mustCall(() => { chunks.push(chunk); }); req.on('end', common.mustCall(() => { + console.log('client receives content', ++endCount); assert.deepStrictEqual(Buffer.concat(chunks), content); - if (++finished === 100) client.close(); + + if (++finished === 100) { + client.close(); + server.close(); + } })); + req.on('error', (e) => { + console.log('client error', e); + }); } })); From 72382d2ee9a890c4602f050a0adecd9473178c0f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 24 Feb 2024 23:58:52 +0100 Subject: [PATCH 2/2] fixup! test: deflake test-http2-large-write-multiple-requests Co-authored-by: Luigi Pinca --- test/parallel/test-http2-large-write-multiple-requests.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/parallel/test-http2-large-write-multiple-requests.js b/test/parallel/test-http2-large-write-multiple-requests.js index ba0488fd48f925..bcbb1434cbec91 100644 --- a/test/parallel/test-http2-large-write-multiple-requests.js +++ b/test/parallel/test-http2-large-write-multiple-requests.js @@ -25,7 +25,6 @@ server.on('stream', (stream, headers) => { stream.end(content); console.log('server sends content', ++streamCount); }); -server.unref(); server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}/`);