Skip to content

Commit 2d5cbdf

Browse files
authored
fix: don't leak internal class (#3024)
1 parent d7f10e1 commit 2d5cbdf

File tree

6 files changed

+36
-30
lines changed

6 files changed

+36
-30
lines changed

docs/docs/api/DiagnosticsChannel.md

-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ diagnosticsChannel.channel('undici:request:create').subscribe(({ request }) => {
2020
console.log('method', request.method)
2121
console.log('path', request.path)
2222
console.log('headers') // array of strings, e.g: ['foo', 'bar']
23-
request.addHeader('hello', 'world')
24-
console.log('headers', request.headers) // e.g. ['foo', 'bar', 'hello', 'world']
2523
})
2624
```
2725

lib/core/request.js

+29-10
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,8 @@ class Request {
9191

9292
this.abort = null
9393

94+
this.publicInterface = null
95+
9496
if (body == null) {
9597
this.body = null
9698
} else if (isStream(body)) {
@@ -187,10 +189,32 @@ class Request {
187189
this[kHandler] = handler
188190

189191
if (channels.create.hasSubscribers) {
190-
channels.create.publish({ request: this })
192+
channels.create.publish({ request: this.getPublicInterface() })
191193
}
192194
}
193195

196+
getPublicInterface () {
197+
const self = this
198+
this.publicInterface ??= {
199+
get origin () {
200+
return self.origin
201+
},
202+
get method () {
203+
return self.method
204+
},
205+
get path () {
206+
return self.path
207+
},
208+
get headers () {
209+
return self.headers
210+
},
211+
get completed () {
212+
return self.completed
213+
}
214+
}
215+
return this.publicInterface
216+
}
217+
194218
onBodySent (chunk) {
195219
if (this[kHandler].onBodySent) {
196220
try {
@@ -203,7 +227,7 @@ class Request {
203227

204228
onRequestSent () {
205229
if (channels.bodySent.hasSubscribers) {
206-
channels.bodySent.publish({ request: this })
230+
channels.bodySent.publish({ request: this.getPublicInterface() })
207231
}
208232

209233
if (this[kHandler].onRequestSent) {
@@ -236,7 +260,7 @@ class Request {
236260
assert(!this.completed)
237261

238262
if (channels.headers.hasSubscribers) {
239-
channels.headers.publish({ request: this, response: { statusCode, headers, statusText } })
263+
channels.headers.publish({ request: this.getPublicInterface(), response: { statusCode, headers, statusText } })
240264
}
241265

242266
try {
@@ -272,7 +296,7 @@ class Request {
272296

273297
this.completed = true
274298
if (channels.trailers.hasSubscribers) {
275-
channels.trailers.publish({ request: this, trailers })
299+
channels.trailers.publish({ request: this.getPublicInterface(), trailers })
276300
}
277301

278302
try {
@@ -287,7 +311,7 @@ class Request {
287311
this.onFinally()
288312

289313
if (channels.error.hasSubscribers) {
290-
channels.error.publish({ request: this, error })
314+
channels.error.publish({ request: this.getPublicInterface(), error })
291315
}
292316

293317
if (this.aborted) {
@@ -309,11 +333,6 @@ class Request {
309333
this.endHandler = null
310334
}
311335
}
312-
313-
addHeader (key, value) {
314-
processHeader(this, key, value)
315-
return this
316-
}
317336
}
318337

319338
function processHeader (request, key, val) {

lib/dispatcher/client-h1.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ function writeH1 (client, request) {
993993
}
994994

995995
if (channels.sendHeaders.hasSubscribers) {
996-
channels.sendHeaders.publish({ request, headers: header, socket })
996+
channels.sendHeaders.publish({ request: request.getPublicInterface(), headers: header, socket })
997997
}
998998

999999
/* istanbul ignore else: assertion */

test/node-test/diagnostics-channel/get.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const { Client } = require('../../..')
77
const { createServer } = require('node:http')
88

99
test('Diagnostics channel - get', (t) => {
10-
const assert = tspl(t, { plan: 32 })
10+
const assert = tspl(t, { plan: 31 })
1111
const server = createServer((req, res) => {
1212
res.setHeader('Content-Type', 'text/plain')
1313
res.setHeader('trailer', 'foo')
@@ -33,8 +33,6 @@ test('Diagnostics channel - get', (t) => {
3333
assert.equal(request.method, 'GET')
3434
assert.equal(request.path, '/')
3535
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
36-
request.addHeader('hello', 'world')
37-
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
3836
})
3937

4038
let _connector
@@ -77,8 +75,7 @@ test('Diagnostics channel - get', (t) => {
7775
'GET / HTTP/1.1',
7876
`host: localhost:${server.address().port}`,
7977
'connection: keep-alive',
80-
'bar: bar',
81-
'hello: world'
78+
'bar: bar'
8279
]
8380

8481
assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')

test/node-test/diagnostics-channel/post-stream.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { Client } = require('../../..')
88
const { createServer } = require('node:http')
99

1010
test('Diagnostics channel - post stream', (t) => {
11-
const assert = tspl(t, { plan: 33 })
11+
const assert = tspl(t, { plan: 31 })
1212
const server = createServer((req, res) => {
1313
req.resume()
1414
res.setHeader('Content-Type', 'text/plain')
@@ -34,9 +34,6 @@ test('Diagnostics channel - post stream', (t) => {
3434
assert.equal(request.method, 'POST')
3535
assert.equal(request.path, '/')
3636
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
37-
request.addHeader('hello', 'world')
38-
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
39-
assert.deepStrictEqual(request.body, body)
4037
})
4138

4239
let _connector
@@ -79,8 +76,7 @@ test('Diagnostics channel - post stream', (t) => {
7976
'POST / HTTP/1.1',
8077
`host: localhost:${server.address().port}`,
8178
'connection: keep-alive',
82-
'bar: bar',
83-
'hello: world'
79+
'bar: bar'
8480
]
8581

8682
assert.equal(headers, expectedHeaders.join('\r\n') + '\r\n')

test/node-test/diagnostics-channel/post.js

+2-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const { Client } = require('../../../')
77
const { createServer } = require('node:http')
88

99
test('Diagnostics channel - post', (t) => {
10-
const assert = tspl(t, { plan: 33 })
10+
const assert = tspl(t, { plan: 31 })
1111
const server = createServer((req, res) => {
1212
req.resume()
1313
res.setHeader('Content-Type', 'text/plain')
@@ -32,9 +32,6 @@ test('Diagnostics channel - post', (t) => {
3232
assert.equal(request.method, 'POST')
3333
assert.equal(request.path, '/')
3434
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
35-
request.addHeader('hello', 'world')
36-
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
37-
assert.deepStrictEqual(request.body, Buffer.from('hello world'))
3835
})
3936

4037
let _connector
@@ -77,8 +74,7 @@ test('Diagnostics channel - post', (t) => {
7774
'POST / HTTP/1.1',
7875
`host: localhost:${server.address().port}`,
7976
'connection: keep-alive',
80-
'bar: bar',
81-
'hello: world'
77+
'bar: bar'
8278
]
8379

8480
assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')

0 commit comments

Comments
 (0)