Skip to content

Commit

Permalink
Merge branch 'main' into error-body
Browse files Browse the repository at this point in the history
  • Loading branch information
ronag authored Apr 14, 2023
2 parents f9c4fac + 9aa639b commit eae7f70
Show file tree
Hide file tree
Showing 18 changed files with 103 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/scorecard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
persist-credentials: false

- name: "Run analysis"
uses: ossf/scorecard-action@e38b1902ae4f44df626f11ba0734b14fb91f8f86 # v2.1.2
uses: ossf/scorecard-action@80e868c13c90f172d68d1f4501dee99e2479f7af # v2.1.3
with:
results_file: results.sarif
results_format: sarif
Expand Down
4 changes: 2 additions & 2 deletions build/wasm.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ execSync(`${WASI_ROOT}/bin/clang \

const base64Wasm = readFileSync(join(WASM_OUT, 'llhttp.wasm')).toString('base64')
writeFileSync(
join(WASM_OUT, 'llhttp.wasm.js'),
join(WASM_OUT, 'llhttp-wasm.js'),
`module.exports = "${base64Wasm}";\n`
)

Expand Down Expand Up @@ -74,6 +74,6 @@ execSync(`${WASI_ROOT}/bin/clang \

const base64WasmSimd = readFileSync(join(WASM_OUT, 'llhttp_simd.wasm')).toString('base64')
writeFileSync(
join(WASM_OUT, 'llhttp_simd.wasm.js'),
join(WASM_OUT, 'llhttp_simd-wasm.js'),
`module.exports = "${base64WasmSimd}";\n`
)
11 changes: 8 additions & 3 deletions lib/api/api-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ class RequestHandler extends AsyncResource {
throw new InvalidArgumentError('invalid opts')
}

const { signal, method, opaque, body, onInfo, responseHeaders, throwOnError } = opts
const { signal, method, opaque, body, onInfo, responseHeaders, throwOnError, highWaterMark } = opts

try {
if (typeof callback !== 'function') {
throw new InvalidArgumentError('invalid callback')
}

if (highWaterMark && (typeof highWaterMark !== 'number' || highWaterMark < 0)) {
throw new InvalidArgumentError('invalid highWaterMark')
}

if (signal && typeof signal.on !== 'function' && typeof signal.addEventListener !== 'function') {
throw new InvalidArgumentError('signal must be an EventEmitter or EventTarget')
}
Expand Down Expand Up @@ -53,6 +57,7 @@ class RequestHandler extends AsyncResource {
this.context = null
this.onInfo = onInfo || null
this.throwOnError = throwOnError
this.highWaterMark = highWaterMark

if (util.isStream(body)) {
body.on('error', (err) => {
Expand All @@ -73,7 +78,7 @@ class RequestHandler extends AsyncResource {
}

onHeaders (statusCode, rawHeaders, resume, statusMessage) {
const { callback, opaque, abort, context, responseHeaders } = this
const { callback, opaque, abort, context, highWaterMark, responseHeaders } = this

const headers = responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)

Expand All @@ -86,7 +91,7 @@ class RequestHandler extends AsyncResource {

const parsedHeaders = responseHeaders === 'raw' ? util.parseHeaders(rawHeaders) : headers
const contentType = parsedHeaders['content-type']
const body = new Readable(resume, abort, contentType)
const body = new Readable({ resume, abort, contentType, highWaterMark })

this.callback = null
this.res = body
Expand Down
9 changes: 7 additions & 2 deletions lib/api/readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,16 @@ const kAbort = Symbol('abort')
const kContentType = Symbol('kContentType')

module.exports = class BodyReadable extends Readable {
constructor (resume, abort, contentType = '') {
constructor ({
resume,
abort,
contentType = '',
highWaterMark = 64 * 1024 // Same as nodejs fs streams.
}) {
super({
autoDestroy: true,
read: resume,
highWaterMark: 64 * 1024 // Same as nodejs fs streams.
highWaterMark
})

this._readableState.dataEmitted = false
Expand Down
6 changes: 3 additions & 3 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,19 +359,19 @@ const createRedirectInterceptor = require('./interceptor/redirectInterceptor')
const EMPTY_BUF = Buffer.alloc(0)

async function lazyllhttp () {
const llhttpWasmData = process.env.JEST_WORKER_ID ? require('./llhttp/llhttp.wasm.js') : undefined
const llhttpWasmData = process.env.JEST_WORKER_ID ? require('./llhttp/llhttp-wasm.js') : undefined

let mod
try {
mod = await WebAssembly.compile(Buffer.from(require('./llhttp/llhttp_simd.wasm.js'), 'base64'))
mod = await WebAssembly.compile(Buffer.from(require('./llhttp/llhttp_simd-wasm.js'), 'base64'))
} catch (e) {
/* istanbul ignore next */

// We could check if the error was caused by the simd option not
// being enabled, but the occurring of this other error
// * https://github.com/emscripten-core/emscripten/issues/11495
// got me to remove that check to avoid breaking Node 12.
mod = await WebAssembly.compile(Buffer.from(llhttpWasmData || require('./llhttp/llhttp.wasm.js'), 'base64'))
mod = await WebAssembly.compile(Buffer.from(llhttpWasmData || require('./llhttp/llhttp-wasm.js'), 'base64'))
}

return await WebAssembly.instantiate(mod, {
Expand Down
1 change: 1 addition & 0 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ class Request {
this.headers += `content-type: ${contentType}\r\n`
}
this.body = bodyStream.stream
this.contentLength = bodyStream.length
} else if (util.isBlobLike(body) && this.contentType == null && body.type) {
this.contentType = body.type
this.headers += `content-type: ${body.type}\r\n`
Expand Down
8 changes: 6 additions & 2 deletions lib/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const { getGlobalOrigin } = require('./global')
const { URLSerializer } = require('./dataURL')
const { kHeadersList } = require('../core/symbols')
const assert = require('assert')
const { setMaxListeners, getEventListeners, defaultMaxListeners } = require('events')
const { getMaxListeners, setMaxListeners, getEventListeners, defaultMaxListeners } = require('events')

let TransformStream = globalThis.TransformStream

Expand Down Expand Up @@ -372,7 +372,11 @@ class Request {
// Third-party AbortControllers may not work with these.
// See, https://github.com/nodejs/undici/pull/1910#issuecomment-1464495619.
try {
if (getEventListeners(signal, 'abort').length >= defaultMaxListeners) {
// If the max amount of listeners is equal to the default, increase it
// This is only available in node >= v19.9.0
if (typeof getMaxListeners === 'function' && getMaxListeners(signal) === defaultMaxListeners) {
setMaxListeners(100, signal)
} else if (getEventListeners(signal, 'abort').length >= defaultMaxListeners) {
setMaxListeners(100, signal)
}
} catch {}
Expand Down
4 changes: 3 additions & 1 deletion lib/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,9 @@ function tryUpgradeRequestToAPotentiallyTrustworthyURL (request) {
*/
function sameOrigin (A, B) {
// 1. If A and B are the same opaque origin, then return true.
// "opaque origin" is an internal value we cannot access, ignore.
if (A.origin === B.origin && A.origin === 'null') {
return true
}

// 2. If A and B are both tuple origins and their schemes,
// hosts, and port are identical, then return true.
Expand Down
File renamed without changes.
File renamed without changes.
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@
"build:wasm": "node build/wasm.js --docker",
"lint": "standard | snazzy",
"lint:fix": "standard --fix | snazzy",
"test": "npm run test:tap && npm run test:node-fetch && npm run test:fetch && npm run test:cookies && npm run test:wpt && npm run test:websocket && npm run test:jest && tsd",
"test": "npm run test:tap && npm run test:node-fetch && npm run test:fetch && npm run test:cookies && npm run test:wpt && npm run test:websocket && npm run test:jest && npm run test:typescript",
"test:cookies": "node scripts/verifyVersion 16 || tap test/cookie/*.js",
"test:node-fetch": "node scripts/verifyVersion.js 16 || mocha test/node-fetch",
"test:fetch": "node scripts/verifyVersion.js 16 || (npm run build:node && tap --expose-gc test/fetch/*.js && tap test/webidl/*.js)",
"test:jest": "node scripts/verifyVersion.js 14 || jest",
"test:tap": "tap test/*.js test/diagnostics-channel/*.js",
"test:tdd": "tap test/*.js test/diagnostics-channel/*.js -w",
"test:typescript": "tsd && tsc test/imports/undici-import.ts",
"test:typescript": "node scripts/verifyVersion.js 14 || tsd && tsc --skipLibCheck test/imports/undici-import.ts",
"test:websocket": "node scripts/verifyVersion.js 18 || tap test/websocket/*.js",
"test:wpt": "node scripts/verifyVersion 18 || (node test/wpt/start-fetch.mjs && node test/wpt/start-FileAPI.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node --no-warnings test/wpt/start-websockets.mjs)",
"coverage": "nyc --reporter=text --reporter=html npm run test",
Expand Down Expand Up @@ -98,7 +98,7 @@
"standard": "^17.0.0",
"table": "^6.8.0",
"tap": "^16.1.0",
"tsd": "^0.27.0",
"tsd": "^0.28.1",
"typescript": "^5.0.2",
"wait-on": "^7.0.1",
"ws": "^8.11.0"
Expand Down
23 changes: 23 additions & 0 deletions test/client-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,29 @@ test('request dump with abort signal', (t) => {
})
})

test('request hwm', (t) => {
t.plan(2)
const server = createServer((req, res) => {
res.write('hello')
})
t.teardown(server.close.bind(server))

server.listen(0, () => {
const client = new Client(`http://localhost:${server.address().port}`)
t.teardown(client.destroy.bind(client))

client.request({
path: '/',
method: 'GET',
highWaterMark: 1000
}, (err, { body }) => {
t.error(err)
t.same(body.readableHighWaterMark, 1000)
body.dump()
})
})
})

test('request abort before headers', (t) => {
t.plan(6)

Expand Down
10 changes: 10 additions & 0 deletions test/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ test('sameOrigin', (t) => {
t.end()
})

t.test('file:// urls', (t) => {
// urls with opaque origins should return true

const a = new URL('file:///C:/undici')
const b = new URL('file:///var/undici')

t.ok(util.sameOrigin(a, b))
t.end()
})

t.end()
})

Expand Down
30 changes: 30 additions & 0 deletions test/issue-2065.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict'

const { test, skip } = require('tap')
const { nodeMajor, nodeMinor } = require('../lib/core/util')
const { createServer } = require('http')
const { once } = require('events')
const { File, FormData, request } = require('..')

if (nodeMajor < 16 || (nodeMajor === 16 && nodeMinor < 8)) {
skip('FormData is not available in node < v16.8.0')
process.exit()
}

test('undici.request with a FormData body should set content-length header', async (t) => {
const server = createServer((req, res) => {
t.ok(req.headers['content-length'])
res.end()
}).listen(0)

t.teardown(server.close.bind(server))
await once(server, 'listening')

const body = new FormData()
body.set('file', new File(['abc'], 'abc.txt'))

await request(`http://localhost:${server.address().port}`, {
method: 'POST',
body
})
})
4 changes: 2 additions & 2 deletions test/node-fetch/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -1648,15 +1648,15 @@ describe('node-fetch', () => {

it('should allow manual redirect handling', function () {
this.timeout(5000)
const url = 'https://httpbin.org/status/302'
const url = `${base}redirect/302`
const options = {
redirect: 'manual'
}
return fetch(url, options).then(res => {
expect(res.status).to.equal(302)
expect(res.url).to.equal(url)
expect(res.type).to.equal('basic')
expect(res.headers.get('Location')).to.equal('/redirect/1')
expect(res.headers.get('Location')).to.equal('/inspect')
expect(res.ok).to.be.false
})
})
Expand Down
2 changes: 1 addition & 1 deletion test/readable.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ test('avoid body reordering', async function (t) {
}
function abort () {
}
const r = new Readable(resume, abort)
const r = new Readable({ resume, abort })

r.push(Buffer.from('hello'))

Expand Down
8 changes: 1 addition & 7 deletions types/connector.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,7 @@ declare namespace buildConnector {
export type Callback = (...args: CallbackArgs) => void
type CallbackArgs = [null, Socket | TLSSocket] | [Error, null]

export type connector = connectorAsync | connectorSync

interface connectorSync {
(options: buildConnector.Options): Socket | TLSSocket
}

interface connectorAsync {
export interface connector {
(options: buildConnector.Options, callback: buildConnector.Callback): void
}
}
2 changes: 2 additions & 0 deletions types/dispatcher.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ declare namespace Dispatcher {
onInfo?: (info: { statusCode: number, headers: Record<string, string | string[]> }) => void;
/** Default: `null` */
responseHeader?: 'raw' | null;
/** Default: `64 KiB` */
highWaterMark?: number;
}
export interface PipelineOptions extends RequestOptions {
/** `true` if the `handler` will return an object stream. Default: `false` */
Expand Down

0 comments on commit eae7f70

Please sign in to comment.