From 21f3bf2787409c670c6b840efe71d06e0fec44a3 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Fri, 14 Oct 2022 11:25:04 -0400 Subject: [PATCH 1/2] feat: add stream-safe-creation.any.js WPT --- lib/fetch/body.js | 8 ++- lib/fetch/index.js | 12 ++++- test/wpt/status/fetch.status.json | 8 +++ .../api/basic/stream-safe-creation.any.js | 54 +++++++++++++++++++ 4 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 test/wpt/tests/fetch/api/basic/stream-safe-creation.any.js diff --git a/lib/fetch/body.js b/lib/fetch/body.js index e5eb610b1a4..0f45e8570dd 100644 --- a/lib/fetch/body.js +++ b/lib/fetch/body.js @@ -16,6 +16,7 @@ const { File } = require('./file') const { StringDecoder } = require('string_decoder') const { parseMIMEType } = require('./dataURL') +/** @type {globalThis['ReadableStream']} */ let ReadableStream async function * blobGen (blob) { @@ -196,11 +197,13 @@ function extractBody (object, keepalive = false) { }, async cancel (reason) { await iterator.return() - } + }, + type: undefined }) } else if (!stream) { // TODO: Spec doesn't say anything about this? stream = new ReadableStream({ + start () {}, async pull (controller) { controller.enqueue( typeof source === 'string' ? new TextEncoder().encode(source) : source @@ -218,7 +221,8 @@ function extractBody (object, keepalive = false) { } } }) - } + }, + type: undefined }) } diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 663d274f0e8..85648a12a25 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -57,6 +57,7 @@ const { TransformStream } = require('stream/web') /** @type {import('buffer').resolveObjectURL} */ let resolveObjectURL +/** @type {globalThis['ReadableStream']} */ let ReadableStream const nodeVersion = process.versions.node.split('.') @@ -930,6 +931,14 @@ async function fetchFinale (fetchParams, response) { start () {}, transform: identityTransformAlgorithm, flush: processResponseEndOfBody + }, { + size () { + return 1 + } + }, { + size () { + return 1 + } }) // 4. Set response’s body to the result of piping response’s body through transformStream. @@ -1757,8 +1766,7 @@ async function httpNetworkFetch ( async cancel (reason) { await cancelAlgorithm(reason) } - }, - { highWaterMark: 0 } + } ) // 17. Run these steps, but abort when the ongoing fetch is terminated: diff --git a/test/wpt/status/fetch.status.json b/test/wpt/status/fetch.status.json index e375ee0128e..197c53f0902 100644 --- a/test/wpt/status/fetch.status.json +++ b/test/wpt/status/fetch.status.json @@ -40,5 +40,13 @@ "fail": [ "Fetch with POST with text body on 421 response should be retried once on new connection." ] + }, + "stream-safe-creation.any.js": { + "fail": [ + "throwing Object.prototype.type accessor should not affect stream creation by 'fetch'", + "Object.prototype.type accessor returning invalid value should not affect stream creation by 'fetch'", + "throwing Object.prototype.highWaterMark accessor should not affect stream creation by 'fetch'", + "Object.prototype.highWaterMark accessor returning invalid value should not affect stream creation by 'fetch'" + ] } } \ No newline at end of file diff --git a/test/wpt/tests/fetch/api/basic/stream-safe-creation.any.js b/test/wpt/tests/fetch/api/basic/stream-safe-creation.any.js new file mode 100644 index 00000000000..382efc1a8b4 --- /dev/null +++ b/test/wpt/tests/fetch/api/basic/stream-safe-creation.any.js @@ -0,0 +1,54 @@ +// META: global=window,worker + +// These tests verify that stream creation is not affected by changes to +// Object.prototype. + +const creationCases = { + fetch: async () => fetch(location.href), + request: () => new Request(location.href, {method: 'POST', body: 'hi'}), + response: () => new Response('bye'), + consumeEmptyResponse: () => new Response().text(), + consumeNonEmptyResponse: () => new Response(new Uint8Array([64])).text(), + consumeEmptyRequest: () => new Request(location.href).text(), + consumeNonEmptyRequest: () => new Request(location.href, + {method: 'POST', body: 'yes'}).arrayBuffer(), +}; + +for (const creationCase of Object.keys(creationCases)) { + for (const accessorName of ['start', 'type', 'size', 'highWaterMark']) { + promise_test(async t => { + Object.defineProperty(Object.prototype, accessorName, { + get() { throw Error(`Object.prototype.${accessorName} was accessed`); }, + configurable: true + }); + t.add_cleanup(() => { + delete Object.prototype[accessorName]; + return Promise.resolve(); + }); + await creationCases[creationCase](); + }, `throwing Object.prototype.${accessorName} accessor should not affect ` + + `stream creation by '${creationCase}'`); + + promise_test(async t => { + // -1 is a convenient value which is invalid, and should cause the + // constructor to throw, for all four fields. + Object.prototype[accessorName] = -1; + t.add_cleanup(() => { + delete Object.prototype[accessorName]; + return Promise.resolve(); + }); + await creationCases[creationCase](); + }, `Object.prototype.${accessorName} accessor returning invalid value ` + + `should not affect stream creation by '${creationCase}'`); + } + + promise_test(async t => { + Object.prototype.start = controller => controller.error(new Error('start')); + t.add_cleanup(() => { + delete Object.prototype.start; + return Promise.resolve(); + }); + await creationCases[creationCase](); + }, `Object.prototype.start function which errors the stream should not ` + + `affect stream creation by '${creationCase}'`); +} From d6149a8f19aeb8beacd3c0f8982eada42f879ef8 Mon Sep 17 00:00:00 2001 From: Khafra <42794878+KhafraDev@users.noreply.github.com> Date: Fri, 14 Oct 2022 11:41:17 -0400 Subject: [PATCH 2/2] fix: add back highWaterMark option --- lib/fetch/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 85648a12a25..386c90da1af 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1766,6 +1766,12 @@ async function httpNetworkFetch ( async cancel (reason) { await cancelAlgorithm(reason) } + }, + { + highWaterMark: 0, + size () { + return 1 + } } )