From 89dfbd7da97d80c0127fcb632788f86044355cd6 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 31 Mar 2024 11:55:36 +0200 Subject: [PATCH] fix: don't leak internal class --- docs/docs/api/DiagnosticsChannel.md | 2 - lib/core/request.js | 39 ++++++++++++++----- lib/dispatcher/client-h1.js | 2 +- test/node-test/diagnostics-channel/get.js | 14 +++---- .../diagnostics-channel/post-stream.js | 15 +++---- test/node-test/diagnostics-channel/post.js | 15 +++---- test/types/diagnostics-channel.test-d.ts | 3 -- types/diagnostics-channel.d.ts | 14 +++---- 8 files changed, 53 insertions(+), 51 deletions(-) diff --git a/docs/docs/api/DiagnosticsChannel.md b/docs/docs/api/DiagnosticsChannel.md index 099c072f6c6..cb0421d84d4 100644 --- a/docs/docs/api/DiagnosticsChannel.md +++ b/docs/docs/api/DiagnosticsChannel.md @@ -20,8 +20,6 @@ diagnosticsChannel.channel('undici:request:create').subscribe(({ request }) => { console.log('method', request.method) console.log('path', request.path) console.log('headers') // array of strings, e.g: ['foo', 'bar'] - request.addHeader('hello', 'world') - console.log('headers', request.headers) // e.g. ['foo', 'bar', 'hello', 'world'] }) ``` diff --git a/lib/core/request.js b/lib/core/request.js index 37839d3c949..38f94038ff6 100644 --- a/lib/core/request.js +++ b/lib/core/request.js @@ -91,6 +91,8 @@ class Request { this.abort = null + this.publicInterface = null + if (body == null) { this.body = null } else if (isStream(body)) { @@ -187,10 +189,32 @@ class Request { this[kHandler] = handler if (channels.create.hasSubscribers) { - channels.create.publish({ request: this }) + channels.create.publish({ request: this.getPublicInterface() }) } } + getPublicInterface () { + const self = this + this.publicInterface ??= { + get origin () { + return self.origin + }, + get method () { + return self.method + }, + get path () { + return self.path + }, + get headers () { + return self.headers + }, + get completed () { + return self.completed + } + } + return this.publicInterface + } + onBodySent (chunk) { if (this[kHandler].onBodySent) { try { @@ -203,7 +227,7 @@ class Request { onRequestSent () { if (channels.bodySent.hasSubscribers) { - channels.bodySent.publish({ request: this }) + channels.bodySent.publish({ request: this.getPublicInterface() }) } if (this[kHandler].onRequestSent) { @@ -236,7 +260,7 @@ class Request { assert(!this.completed) if (channels.headers.hasSubscribers) { - channels.headers.publish({ request: this, response: { statusCode, headers, statusText } }) + channels.headers.publish({ request: this.getPublicInterface(), headers, statusText, statusCode }) } try { @@ -272,7 +296,7 @@ class Request { this.completed = true if (channels.trailers.hasSubscribers) { - channels.trailers.publish({ request: this, trailers }) + channels.trailers.publish({ request: this.getPublicInterface(), trailers }) } try { @@ -287,7 +311,7 @@ class Request { this.onFinally() if (channels.error.hasSubscribers) { - channels.error.publish({ request: this, error }) + channels.error.publish({ request: this.getPublicInterface(), error }) } if (this.aborted) { @@ -309,11 +333,6 @@ class Request { this.endHandler = null } } - - addHeader (key, value) { - processHeader(this, key, value) - return this - } } function processHeader (request, key, val) { diff --git a/lib/dispatcher/client-h1.js b/lib/dispatcher/client-h1.js index 62a3e29ef24..da70f7d8313 100644 --- a/lib/dispatcher/client-h1.js +++ b/lib/dispatcher/client-h1.js @@ -993,7 +993,7 @@ function writeH1 (client, request) { } if (channels.sendHeaders.hasSubscribers) { - channels.sendHeaders.publish({ request, headers: header, socket }) + channels.sendHeaders.publish({ request: request.getPublicInterface(), headers: header, socket }) } /* istanbul ignore else: assertion */ diff --git a/test/node-test/diagnostics-channel/get.js b/test/node-test/diagnostics-channel/get.js index 397dfa3bc5f..34b8429fbf3 100644 --- a/test/node-test/diagnostics-channel/get.js +++ b/test/node-test/diagnostics-channel/get.js @@ -7,7 +7,7 @@ const { Client } = require('../../..') const { createServer } = require('node:http') test('Diagnostics channel - get', (t) => { - const assert = tspl(t, { plan: 32 }) + const assert = tspl(t, { plan: 31 }) const server = createServer((req, res) => { res.setHeader('Content-Type', 'text/plain') res.setHeader('trailer', 'foo') @@ -33,8 +33,6 @@ test('Diagnostics channel - get', (t) => { assert.equal(request.method, 'GET') assert.equal(request.path, '/') assert.deepStrictEqual(request.headers, ['bar', 'bar']) - request.addHeader('hello', 'world') - assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world']) }) let _connector @@ -84,16 +82,16 @@ test('Diagnostics channel - get', (t) => { assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n') }) - diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, response }) => { + diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, headers, statusCode, statusText }) => { assert.equal(_req, request) - assert.equal(response.statusCode, 200) + assert.equal(statusCode, 200) const expectedHeaders = [ Buffer.from('Content-Type'), Buffer.from('text/plain'), Buffer.from('trailer'), Buffer.from('foo'), Buffer.from('Date'), - response.headers[5], // This is a date + headers[5], // This is a date Buffer.from('Connection'), Buffer.from('keep-alive'), Buffer.from('Keep-Alive'), @@ -101,8 +99,8 @@ test('Diagnostics channel - get', (t) => { Buffer.from('Transfer-Encoding'), Buffer.from('chunked') ] - assert.deepStrictEqual(response.headers, expectedHeaders) - assert.equal(response.statusText, 'OK') + assert.deepStrictEqual(headers, expectedHeaders) + assert.equal(statusText, 'OK') }) let endEmitted = false diff --git a/test/node-test/diagnostics-channel/post-stream.js b/test/node-test/diagnostics-channel/post-stream.js index 881873a7c1c..03de7ef25b5 100644 --- a/test/node-test/diagnostics-channel/post-stream.js +++ b/test/node-test/diagnostics-channel/post-stream.js @@ -8,7 +8,7 @@ const { Client } = require('../../..') const { createServer } = require('node:http') test('Diagnostics channel - post stream', (t) => { - const assert = tspl(t, { plan: 33 }) + const assert = tspl(t, { plan: 31 }) const server = createServer((req, res) => { req.resume() res.setHeader('Content-Type', 'text/plain') @@ -34,9 +34,6 @@ test('Diagnostics channel - post stream', (t) => { assert.equal(request.method, 'POST') assert.equal(request.path, '/') assert.deepStrictEqual(request.headers, ['bar', 'bar']) - request.addHeader('hello', 'world') - assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world']) - assert.deepStrictEqual(request.body, body) }) let _connector @@ -86,16 +83,16 @@ test('Diagnostics channel - post stream', (t) => { assert.equal(headers, expectedHeaders.join('\r\n') + '\r\n') }) - diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, response }) => { + diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, headers, statusCode, statusText }) => { assert.equal(_req, request) - assert.equal(response.statusCode, 200) + assert.equal(statusCode, 200) const expectedHeaders = [ Buffer.from('Content-Type'), Buffer.from('text/plain'), Buffer.from('trailer'), Buffer.from('foo'), Buffer.from('Date'), - response.headers[5], // This is a date + headers[5], // This is a date Buffer.from('Connection'), Buffer.from('keep-alive'), Buffer.from('Keep-Alive'), @@ -103,8 +100,8 @@ test('Diagnostics channel - post stream', (t) => { Buffer.from('Transfer-Encoding'), Buffer.from('chunked') ] - assert.deepStrictEqual(response.headers, expectedHeaders) - assert.equal(response.statusText, 'OK') + assert.deepStrictEqual(headers, expectedHeaders) + assert.equal(statusText, 'OK') }) diagnosticsChannel.channel('undici:request:bodySent').subscribe(({ request }) => { diff --git a/test/node-test/diagnostics-channel/post.js b/test/node-test/diagnostics-channel/post.js index 1408ffbf023..4b606bd68c7 100644 --- a/test/node-test/diagnostics-channel/post.js +++ b/test/node-test/diagnostics-channel/post.js @@ -7,7 +7,7 @@ const { Client } = require('../../../') const { createServer } = require('node:http') test('Diagnostics channel - post', (t) => { - const assert = tspl(t, { plan: 33 }) + const assert = tspl(t, { plan: 31 }) const server = createServer((req, res) => { req.resume() res.setHeader('Content-Type', 'text/plain') @@ -32,9 +32,6 @@ test('Diagnostics channel - post', (t) => { assert.equal(request.method, 'POST') assert.equal(request.path, '/') assert.deepStrictEqual(request.headers, ['bar', 'bar']) - request.addHeader('hello', 'world') - assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world']) - assert.deepStrictEqual(request.body, Buffer.from('hello world')) }) let _connector @@ -84,16 +81,16 @@ test('Diagnostics channel - post', (t) => { assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n') }) - diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, response }) => { + diagnosticsChannel.channel('undici:request:headers').subscribe(({ request, headers, statusCode, statusText }) => { assert.equal(_req, request) - assert.equal(response.statusCode, 200) + assert.equal(statusCode, 200) const expectedHeaders = [ Buffer.from('Content-Type'), Buffer.from('text/plain'), Buffer.from('trailer'), Buffer.from('foo'), Buffer.from('Date'), - response.headers[5], // This is a date + headers[5], // This is a date Buffer.from('Connection'), Buffer.from('keep-alive'), Buffer.from('Keep-Alive'), @@ -101,8 +98,8 @@ test('Diagnostics channel - post', (t) => { Buffer.from('Transfer-Encoding'), Buffer.from('chunked') ] - assert.deepStrictEqual(response.headers, expectedHeaders) - assert.equal(response.statusText, 'OK') + assert.deepStrictEqual(headers, expectedHeaders) + assert.equal(statusText, 'OK') }) diagnosticsChannel.channel('undici:request:bodySent').subscribe(({ request }) => { diff --git a/test/types/diagnostics-channel.test-d.ts b/test/types/diagnostics-channel.test-d.ts index 334404c385f..7f752ff9f0e 100644 --- a/test/types/diagnostics-channel.test-d.ts +++ b/test/types/diagnostics-channel.test-d.ts @@ -8,9 +8,6 @@ const request = { method: "GET" as const, path: "", headers: "", - addHeader: (key: string, value: string) => { - return request; - }, }; const response = { diff --git a/types/diagnostics-channel.d.ts b/types/diagnostics-channel.d.ts index 85d44823978..c52fc90cf5b 100644 --- a/types/diagnostics-channel.d.ts +++ b/types/diagnostics-channel.d.ts @@ -6,16 +6,10 @@ import Dispatcher from "./dispatcher"; declare namespace DiagnosticsChannel { interface Request { origin?: string | URL; - completed: boolean; method?: Dispatcher.HttpMethod; path: string; - headers: string; - addHeader(key: string, value: string): Request; - } - interface Response { - statusCode: number; - statusText: string; - headers: Array; + headers: any; + completed: boolean; } type Error = unknown; interface ConnectParams { @@ -34,7 +28,9 @@ declare namespace DiagnosticsChannel { } export interface RequestHeadersMessage { request: Request; - response: Response; + headers: Array; + statusCode: number; + statusText: string; } export interface RequestTrailersMessage { request: Request;