Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tests in v5.x for Node 20 #4104

Merged
merged 7 commits into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions .github/workflows/fuzz.yml

This file was deleted.

2 changes: 1 addition & 1 deletion .taprc
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ jsx: false
flow: false
coverage: false
expose-gc: true
timeout: 60
timeout: 120
check-coverage: false
7 changes: 3 additions & 4 deletions lib/cookies/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict'

const { parseSetCookie } = require('./parse')
const { stringify, getHeadersList } = require('./util')
const { stringify } = require('./util')
const { webidl } = require('../fetch/webidl')
const { Headers } = require('../fetch/headers')

Expand Down Expand Up @@ -77,14 +77,13 @@ function getSetCookies (headers) {

webidl.brandCheck(headers, Headers, { strict: false })

const cookies = getHeadersList(headers).cookies
const cookies = headers.getSetCookie()

if (!cookies) {
return []
}

// In older versions of undici, cookies is a list of name:value.
return cookies.map((pair) => parseSetCookie(Array.isArray(pair) ? pair[1] : pair))
return cookies.map((pair) => parseSetCookie(pair))
}

/**
Expand Down
35 changes: 9 additions & 26 deletions lib/cookies/util.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
'use strict'

const assert = require('assert')
const { kHeadersList } = require('../core/symbols')

/**
* @param {string} value
* @returns {boolean}
*/
function isCTLExcludingHtab (value) {
if (value.length === 0) {
return false
Expand Down Expand Up @@ -263,29 +264,11 @@ function stringify (cookie) {
return out.join('; ')
}

let kHeadersListNode

function getHeadersList (headers) {
if (headers[kHeadersList]) {
return headers[kHeadersList]
}

if (!kHeadersListNode) {
kHeadersListNode = Object.getOwnPropertySymbols(headers).find(
(symbol) => symbol.description === 'headers list'
)

assert(kHeadersListNode, 'Headers cannot be parsed')
}

const headersList = headers[kHeadersListNode]
assert(headersList)

return headersList
}

module.exports = {
isCTLExcludingHtab,
stringify,
getHeadersList
validateCookieName,
validateCookiePath,
validateCookieValue,
toIMFDate,
stringify
}
4 changes: 4 additions & 0 deletions lib/fetch/headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const {
isValidHeaderName,
isValidHeaderValue
} = require('./util')
const util = require('util')
const { webidl } = require('./webidl')
const assert = require('assert')

Expand Down Expand Up @@ -563,6 +564,9 @@ Object.defineProperties(Headers.prototype, {
[Symbol.toStringTag]: {
value: 'Headers',
configurable: true
},
[util.inspect.custom]: {
enumerable: false
}
})

Expand Down
4 changes: 3 additions & 1 deletion test/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const hasIPv6 = (() => {
)
})()

const isNode20OrHigher = Number(process.versions.node.split('.')[0]) >= 20

test('basic get', (t) => {
t.plan(24)

Expand Down Expand Up @@ -1896,7 +1898,7 @@ test('async iterator early return closes early', (t) => {
})
})

test('async iterator yield unsupported TypedArray', (t) => {
test('async iterator yield unsupported TypedArray', { skip: isNode20OrHigher }, (t) => {
t.plan(3)
const server = createServer((req, res) => {
req.on('end', () => {
Expand Down
3 changes: 1 addition & 2 deletions test/cookie/global-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const {
getSetCookies,
setCookie
} = require('../..')
const { getHeadersList } = require('../../lib/cookies/util')

/* global Headers */

Expand Down Expand Up @@ -41,7 +40,7 @@ test('Using global Headers', (t) => {
'set-cookie': 'undici=getSetCookies; Secure'
})

const supportsCookies = getHeadersList(headers).cookies
const supportsCookies = headers.getSetCookie()

if (!supportsCookies) {
t.same(getSetCookies(headers), [])
Expand Down
13 changes: 9 additions & 4 deletions test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -402,11 +402,16 @@ test(
}
})
} catch (error) {
t.equal(
error.message,
'Client network socket disconnected before secure TLS connection was established'
t.ok(
error.message === 'Client network socket disconnected before secure TLS connection was established' ||
error.message.includes('SSL routines:ssl3_read_bytes:tlsv1 alert no application protocol'),
`Error message should be as expected: ${error.message}`
)
t.ok(
error.code === 'ECONNRESET' ||
error.code === 'ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL',
`Error code should be as expected: ${error.code}`
)
t.equal(error.code, 'ECONNRESET')
}
}
)
Expand Down
45 changes: 41 additions & 4 deletions test/redirect-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@ const {
startRedirectingWithQueryParams
} = require('./utils/redirecting-servers')
const { createReadable, createReadableStream } = require('./utils/stream')
const Agent = undici.Agent

for (const factory of [
(server, opts) => new undici.Agent(opts),
(server, opts) => new undici.Pool(`http://${server}`, opts),
(server, opts) => new undici.Client(`http://${server}`, opts)
]) {
const request = (t, server, opts, ...args) => {
const dispatcher = factory(server, opts)
opts = opts || {}
const dispatcher = factory(server, {
keepAliveMaxTimeout: 1000,
keepAliveTimeout: 1000,
...opts
})
t.teardown(() => dispatcher.close())
return undici.request(args[0], { ...args[1], dispatcher }, args[2])
}
Expand Down Expand Up @@ -348,9 +354,16 @@ for (const factory of [
t.test('should follow redirections when going cross origin', async t => {
const [server1, server2, server3] = await startRedirectingChainServers(t)

const agent = new Agent({
keepAliveTimeout: 1000,
keepAliveMaxTimeout: 1000
})
t.teardown(agent.close.bind(agent))

const { statusCode, headers, body: bodyStream, context: { history } } = await undici.request(`http://${server1}`, {
method: 'POST',
maxRedirections: 10
maxRedirections: 10,
dispatcher: agent
})

const body = await bodyStream.text()
Expand All @@ -371,10 +384,17 @@ t.test('should follow redirections when going cross origin', async t => {
t.test('should handle errors (callback)', t => {
t.plan(1)

const agent = new Agent({
keepAliveTimeout: 1000,
keepAliveMaxTimeout: 1000
})
t.teardown(agent.close.bind(agent))

undici.request(
'http://localhost:0',
{
maxRedirections: 10
maxRedirections: 10,
dispatcher: agent
},
error => {
t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/)
Expand All @@ -383,18 +403,29 @@ t.test('should handle errors (callback)', t => {
})

t.test('should handle errors (promise)', async t => {
const agent = new Agent({
keepAliveTimeout: 1000,
keepAliveMaxTimeout: 1000
})
t.teardown(agent.close.bind(agent))
try {
await undici.request('http://localhost:0', { maxRedirections: 10 })
await undici.request('http://localhost:0', { maxRedirections: 10, dispatcher: agent })
t.fail('Did not throw')
} catch (error) {
t.match(error.code, /EADDRNOTAVAIL|ECONNREFUSED/)
}
})

t.test('removes authorization header on third party origin', async t => {
const agent = new Agent({
keepAliveTimeout: 1000,
keepAliveMaxTimeout: 1000
})
t.teardown(agent.close.bind(agent))
const [server1] = await startRedirectingWithAuthorization(t, 'secret')
const { body: bodyStream } = await undici.request(`http://${server1}`, {
maxRedirections: 10,
dispatcher: agent,
headers: {
authorization: 'secret'
}
Expand All @@ -406,9 +437,15 @@ t.test('removes authorization header on third party origin', async t => {
})

t.test('removes cookie header on third party origin', async t => {
const agent = new Agent({
keepAliveTimeout: 1000,
keepAliveMaxTimeout: 1000
})
t.teardown(agent.close.bind(agent))
const [server1] = await startRedirectingWithCookie(t, 'a=b')
const { body: bodyStream } = await undici.request(`http://${server1}`, {
maxRedirections: 10,
dispatcher: agent,
headers: {
cookie: 'a=b'
}
Expand Down
7 changes: 6 additions & 1 deletion test/utils/redirecting-servers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ function close (server) {
return new Promise(resolve => {
if (isNode20) {
server.closeAllConnections()
server.close(function () {
resolve()
})
}
server.close(resolve)
server.close()
server.unref()
resolve()
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/wpt/runner/runner.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { fileURLToPath } from 'node:url'
import { Worker } from 'node:worker_threads'
import { colors, handlePipes, normalizeName, parseMeta, resolveStatusPath } from './util.mjs'

const basePath = fileURLToPath(join(import.meta.url, '../..'))
const basePath = fileURLToPath(new URL('..', import.meta.url))
const testPath = join(basePath, 'tests')
const statusPath = join(basePath, 'status')

Expand Down Expand Up @@ -131,7 +131,7 @@ export class WPTRunner extends EventEmitter {
}

async run () {
const workerPath = join(fileURLToPath(import.meta.url), '../worker.mjs')
const workerPath = fileURLToPath(new URL('../worker.mjs', import.meta.url))
/** @type {Set<Worker>} */
const activeWorkers = new Set()
let finishedFiles = 1
Expand Down
Loading