Skip to content

Commit

Permalink
Merge branch 'main' of github.com:nodejs/undici
Browse files Browse the repository at this point in the history
  • Loading branch information
mcollina committed Apr 2, 2024
2 parents 7485cd9 + 8dea744 commit 3ac3682
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 65 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,6 @@ jobs:

- name: Coverage Report
if: inputs.runs-on == 'ubuntu-latest' && inputs.node-version == 20
uses: codecov/codecov-action@54bcd8715eee62d40e33596ef5e8f0f48dbbccab # v4.1.0
uses: codecov/codecov-action@c16abc29c95fcf9174b58eb7e1abf4c866893bc8 # v4.1.1
with:
token: ${{ secrets.CODECOV_TOKEN }}
2 changes: 0 additions & 2 deletions docs/docs/api/DiagnosticsChannel.md
Original file line number Diff line number Diff line change
Expand Up @@ -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']
})
```

Expand Down
39 changes: 29 additions & 10 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class Request {

this.abort = null

this.publicInterface = null

if (body == null) {
this.body = null
} else if (isStream(body)) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down Expand Up @@ -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(), response: { statusCode, headers, statusText } })
}

try {
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -309,11 +333,6 @@ class Request {
this.endHandler = null
}
}

addHeader (key, value) {
processHeader(this, key, value)
return this
}
}

function processHeader (request, key, val) {
Expand Down
3 changes: 0 additions & 3 deletions lib/core/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,6 @@ function bufferToLowerCasedHeaderName (value) {
* @returns {Record<string, string | string[]>}
*/
function parseHeaders (headers, obj) {
// For H2 support
if (!Array.isArray(headers)) return headers

if (obj === undefined) obj = {}
for (let i = 0; i < headers.length; i += 2) {
const key = headerNameToString(headers[i])
Expand Down
2 changes: 1 addition & 1 deletion lib/dispatcher/client-h1.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
16 changes: 15 additions & 1 deletion lib/dispatcher/client-h2.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,20 @@ const {
}
} = http2

function parseH2Headers (headers) {
// set-cookie is always an array. Duplicates are added to the array.
// For duplicate cookie headers, the values are joined together with '; '.
headers = Object.entries(headers).flat(2)

const result = []

for (const header of headers) {
result.push(Buffer.from(header))
}

return result
}

async function connectH2 (client, socket) {
client[kSocket] = socket

Expand Down Expand Up @@ -391,7 +405,7 @@ function writeH2 (client, request) {
const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers
request.onResponseStarted()

if (request.onHeaders(Number(statusCode), realHeaders, stream.resume.bind(stream), '') === false) {
if (request.onHeaders(Number(statusCode), parseH2Headers(realHeaders), stream.resume.bind(stream), '') === false) {
stream.pause()
}

Expand Down
23 changes: 0 additions & 23 deletions lib/web/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2141,29 +2141,6 @@ async function httpNetworkFetch (
codings = contentEncoding.toLowerCase().split(',').map((x) => x.trim())
}
location = headersList.get('location', true)
} else {
const keys = Object.keys(rawHeaders)
for (let i = 0; i < keys.length; ++i) {
// The header names are already in lowercase.
const key = keys[i]
const value = rawHeaders[key]
if (key === 'set-cookie') {
for (let j = 0; j < value.length; ++j) {
headersList.append(key, value[j], true)
}
} else {
headersList.append(key, value, true)
}
}
// For H2, The header names are already in lowercase,
// so we can avoid the `HeadersList#get` call here.
const contentEncoding = rawHeaders['content-encoding']
if (contentEncoding) {
// https://www.rfc-editor.org/rfc/rfc7231#section-3.1.2.1
// "All content-coding values are case-insensitive..."
codings = contentEncoding.toLowerCase().split(',').map((x) => x.trim()).reverse()
}
location = rawHeaders.location
}

this.body = new Readable({ read: resume })
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"@sinonjs/fake-timers": "^11.1.0",
"@types/node": "^18.0.3",
"abort-controller": "^3.0.0",
"borp": "^0.9.1",
"borp": "^0.10.0",
"c8": "^9.1.0",
"cross-env": "^7.0.3",
"dns-packet": "^5.4.0",
Expand All @@ -112,7 +112,7 @@
"proxy": "^2.1.1",
"snazzy": "^9.0.0",
"standard": "^17.0.0",
"tsd": "^0.30.1",
"tsd": "^0.31.0",
"typescript": "^5.0.2",
"ws": "^8.11.0"
},
Expand Down
3 changes: 2 additions & 1 deletion test/fetch/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test('[Fetch] Issue#2311', async (t) => {
res.end(body)
})

const { strictEqual } = tspl(t, { plan: 1 })
const { strictEqual } = tspl(t, { plan: 2 })

server.listen()
await once(server, 'listening')
Expand Down Expand Up @@ -65,6 +65,7 @@ test('[Fetch] Issue#2311', async (t) => {
t.after(closeClientAndServerAsPromise(client, server))

strictEqual(responseBody, expectedBody)
strictEqual(response.headers.get('x-custom-h2'), 'foo')
})

test('[Fetch] Simple GET with h2', async (t) => {
Expand Down
6 changes: 4 additions & 2 deletions test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -851,8 +851,10 @@ test('Should handle h2 request with body (string or buffer) - dispatch', async t
},
onHeaders (statusCode, headers) {
t.strictEqual(statusCode, 200)
t.strictEqual(headers['content-type'], 'text/plain; charset=utf-8')
t.strictEqual(headers['x-custom-h2'], 'foo')
t.strictEqual(headers[0].toString('utf-8'), 'content-type')
t.strictEqual(headers[1].toString('utf-8'), 'text/plain; charset=utf-8')
t.strictEqual(headers[2].toString('utf-8'), 'x-custom-h2')
t.strictEqual(headers[3].toString('utf-8'), 'foo')
},
onData (chunk) {
response.push(chunk)
Expand Down
7 changes: 2 additions & 5 deletions test/node-test/diagnostics-channel/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -77,8 +75,7 @@ test('Diagnostics channel - get', (t) => {
'GET / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar',
'hello: world'
'bar: bar'
]

assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down
8 changes: 2 additions & 6 deletions test/node-test/diagnostics-channel/post-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -79,8 +76,7 @@ test('Diagnostics channel - post stream', (t) => {
'POST / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar',
'hello: world'
'bar: bar'
]

assert.equal(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down
8 changes: 2 additions & 6 deletions test/node-test/diagnostics-channel/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand Down Expand Up @@ -77,8 +74,7 @@ test('Diagnostics channel - post', (t) => {
'POST / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar',
'hello: world'
'bar: bar'
]

assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down
3 changes: 1 addition & 2 deletions types/diagnostics-channel.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ declare namespace DiagnosticsChannel {
completed: boolean;
method?: Dispatcher.HttpMethod;
path: string;
headers: string;
addHeader(key: string, value: string): Request;
headers: any;
}
interface Response {
statusCode: number;
Expand Down

0 comments on commit 3ac3682

Please sign in to comment.