From c914c56262f5ff4e6163c8652c723cc02a0f09a9 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:24:30 +0100 Subject: [PATCH 01/12] fix: lazy decode body Don't start decoding the body before headers and status has been processed. Fixes: https://github.com/nodejs/undici/issues/1271 --- lib/fetch/index.js | 36 +++++++++++++++--------------------- tmp.js | 3 +++ 2 files changed, 18 insertions(+), 21 deletions(-) create mode 100644 tmp.js diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 0eae43cb28f..3aa9028e6c8 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -762,8 +762,8 @@ async function schemeFetch (fetchParams) { switch (scheme) { case 'about:': { // If request’s current URL’s path is the string "blank", then return a new response - // whose status message is `OK`, header list is « (`Content-Type`, `text/html;charset=utf-8`) », - // and body is the empty byte sequence. + // whose status message is `OK`, header list is « (`Content-Type`, `text/html;charset=utf-8`) », + // and body is the empty byte sequence. if (path === 'blank') { const resp = makeResponse({ statusText: 'OK', @@ -771,7 +771,7 @@ async function schemeFetch (fetchParams) { 'content-type', 'text/html;charset=utf-8' ] }) - + resp.urlList = [new URL('about:blank')] return resp } @@ -784,12 +784,12 @@ async function schemeFetch (fetchParams) { context.on('terminated', onRequestAborted) - // 1. Run these steps, but abort when the ongoing fetch is terminated: + // 1. Run these steps, but abort when the ongoing fetch is terminated: // 1a. Let blob be request’s current URL’s blob URL entry’s object. // https://w3c.github.io/FileAPI/#blob-url-entry // P.S. Thank God this method is available in node. const currentURL = requestCurrentURL(request) - + // https://github.com/web-platform-tests/wpt/blob/7b0ebaccc62b566a1965396e5be7bb2bc06f841f/FileAPI/url/resources/fetch-tests.js#L52-L56 // Buffer.resolveObjectURL does not ignore URL queries. if (currentURL.search.length !== 0) { @@ -803,7 +803,7 @@ async function schemeFetch (fetchParams) { return makeNetworkError('invalid method') } - // 3a. Let response be a new response whose status message is `OK`. + // 3a. Let response be a new response whose status message is `OK`. const response = makeResponse({ statusText: 'OK', urlList: [currentURL] }) // 4a. Append (`Content-Length`, blob’s size attribute value) to response’s header list. @@ -1816,7 +1816,7 @@ function httpNetworkFetch ( maxRedirections: 0 }, { - decoder: null, + body: new PassThrough().on('error', () => {}), abort: null, context, @@ -1876,16 +1876,10 @@ function httpNetworkFetch ( } } - if (decoders.length > 1) { - pipeline(...decoders, () => {}) - } else if (decoders.length === 0) { - // TODO (perf): Avoid intermediate. - decoders.push(new PassThrough()) - } - - this.decoder = decoders[0].on('drain', resume) - - const iterator = decoders[decoders.length - 1][Symbol.asyncIterator]() + const src = decoders.length + ? pipeline(this.body, ...decoders, () => {}) + : this.body + const iterator = src[Symbol.asyncIterator]() pullAlgorithm = async (controller) => { // 4. Set bytes to the result of handling content codings given @@ -1895,7 +1889,7 @@ function httpNetworkFetch ( const { done, value } = await iterator.next() bytes = done ? undefined : value } catch (err) { - if (this.decoder.writableEnded && !timingInfo.encodedBodySize) { + if (this.body.writableEnded && !timingInfo.encodedBodySize) { // zlib doesn't like empty streams. bytes = undefined } else { @@ -1969,15 +1963,15 @@ function httpNetworkFetch ( // 4. See pullAlgorithm... - return this.decoder.write(bytes) + return this.body.write(bytes) }, onComplete () { - this.decoder.end() + this.body.end() }, onError (error) { - this.decoder?.destroy(error) + this.body?.destroy(error) this.context.terminate({ reason: error }) diff --git a/tmp.js b/tmp.js new file mode 100644 index 00000000000..3ab59fec8ba --- /dev/null +++ b/tmp.js @@ -0,0 +1,3 @@ +const { fetch } = require('.') + +fetch('https://www.luzernerzeitung.ch/kultur/literatur-im-pool-ld.2260742') From c13444b55b791a5e3c90702f96dcb652ba752f0c Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:36:25 +0100 Subject: [PATCH 02/12] fixup --- lib/fetch/index.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 3aa9028e6c8..1331ad19bcc 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1735,26 +1735,19 @@ function httpNetworkFetch ( ReadableStream = require('stream/web').ReadableStream } - let pullResolve - const stream = new ReadableStream( { async start (controller) { context.controller = controller }, async pull (controller) { - if (!pullAlgorithm) { - await new Promise((resolve) => { - pullResolve = resolve - }) - } await pullAlgorithm(controller) }, async cancel (reason) { await cancelAlgorithm(reason) } }, - { highWaterMark } + { highWaterMark: 0 } ) // 16. Run these steps, but abort when the ongoing fetch is terminated: @@ -1816,7 +1809,7 @@ function httpNetworkFetch ( maxRedirections: 0 }, { - body: new PassThrough().on('error', () => {}), + body: new PassThrough({ highWaterMark }).on('error', () => {}), abort: null, context, From 3f093b357297276bdfe46fc5362afecdf724e363 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:37:45 +0100 Subject: [PATCH 03/12] fixup --- lib/fetch/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 1331ad19bcc..6f5604139e7 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1721,7 +1721,7 @@ function httpNetworkFetch ( // 12. Let highWaterMark be a non-negative, non-NaN number, chosen by // the user agent. - const highWaterMark = 64 * 1024 // Same as nodejs fs streams. + highWaterMark = 0 // We already buffer plenty in socket + stream + decoders. // 13. Let sizeAlgorithm be an algorithm that accepts a chunk object // and returns a non-negative, non-NaN, non-infinite number, chosen by the user agent. @@ -1747,7 +1747,7 @@ function httpNetworkFetch ( await cancelAlgorithm(reason) } }, - { highWaterMark: 0 } + { highWaterMark } ) // 16. Run these steps, but abort when the ongoing fetch is terminated: @@ -1809,7 +1809,7 @@ function httpNetworkFetch ( maxRedirections: 0 }, { - body: new PassThrough({ highWaterMark }).on('error', () => {}), + body: new PassThrough().on('error', () => {}), abort: null, context, From ebfb47ea8526782a3b594cead36c079a069c1858 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:38:20 +0100 Subject: [PATCH 04/12] fixup --- lib/fetch/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 6f5604139e7..80b7b64821b 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1964,7 +1964,7 @@ function httpNetworkFetch ( }, onError (error) { - this.body?.destroy(error) + this.body.destroy(error) this.context.terminate({ reason: error }) From 95c5f10b80254955ccd9dc4c2020c522ed1fbd9a Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:43:34 +0100 Subject: [PATCH 05/12] fixup --- lib/fetch/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 80b7b64821b..2e771b5b607 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1869,6 +1869,8 @@ function httpNetworkFetch ( } } + this.body.on('drain', resume) + const src = decoders.length ? pipeline(this.body, ...decoders, () => {}) : this.body From 5f82471d0fd3048fc2b56ee67aed60d202495a06 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 14:44:22 +0100 Subject: [PATCH 06/12] fixup --- tmp.js | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 tmp.js diff --git a/tmp.js b/tmp.js deleted file mode 100644 index 3ab59fec8ba..00000000000 --- a/tmp.js +++ /dev/null @@ -1,3 +0,0 @@ -const { fetch } = require('.') - -fetch('https://www.luzernerzeitung.ch/kultur/literatur-im-pool-ld.2260742') From faf8e1e5941676043f941b1d076cc81448d92ec9 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:41:16 +0100 Subject: [PATCH 07/12] fixup --- lib/fetch/index.js | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 2e771b5b607..0ea70baeb6e 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -48,7 +48,7 @@ const { } = require('./constants') const { kHeadersList } = require('../core/symbols') const EE = require('events') -const { PassThrough, pipeline } = require('stream') +const { Readable, pipeline } = require('stream') const { isErrored, isReadable } = require('../core/util') const { kIsMockActive } = require('../mock/mock-symbols') const { dataURLProcessor } = require('./dataURL') @@ -1721,7 +1721,7 @@ function httpNetworkFetch ( // 12. Let highWaterMark be a non-negative, non-NaN number, chosen by // the user agent. - highWaterMark = 0 // We already buffer plenty in socket + stream + decoders. + const highWaterMark = 0 // We already buffer plenty in socket + stream + decoders. // 13. Let sizeAlgorithm be an algorithm that accepts a chunk object // and returns a non-negative, non-NaN, non-infinite number, chosen by the user agent. @@ -1809,7 +1809,7 @@ function httpNetworkFetch ( maxRedirections: 0 }, { - body: new PassThrough().on('error', () => {}), + body: null, abort: null, context, @@ -1869,11 +1869,11 @@ function httpNetworkFetch ( } } - this.body.on('drain', resume) + this.body = new Readable({ read: resume }) const src = decoders.length ? pipeline(this.body, ...decoders, () => {}) - : this.body + : this.body.on('error', () => {}) const iterator = src[Symbol.asyncIterator]() pullAlgorithm = async (controller) => { @@ -1928,11 +1928,6 @@ function httpNetworkFetch ( return controller.desiredSize > 0 } - if (pullResolve) { - pullResolve() - pullResolve = null - } - resolve(response) return true @@ -1958,15 +1953,15 @@ function httpNetworkFetch ( // 4. See pullAlgorithm... - return this.body.write(bytes) + return this.body.push(bytes) }, onComplete () { - this.body.end() + this.body.push(null) }, onError (error) { - this.body.destroy(error) + this.body?.destroy(error) this.context.terminate({ reason: error }) From eb2ba38fc246e2c5343fee977d9a424fca6fbec2 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:41:45 +0100 Subject: [PATCH 08/12] fixup --- lib/fetch/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 0ea70baeb6e..8d196b0021f 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1884,7 +1884,7 @@ function httpNetworkFetch ( const { done, value } = await iterator.next() bytes = done ? undefined : value } catch (err) { - if (this.body.writableEnded && !timingInfo.encodedBodySize) { + if (this.body.readableEnded && !timingInfo.encodedBodySize) { // zlib doesn't like empty streams. bytes = undefined } else { From 952a769f2d6baf8be2c5cf96a17ba60b4c27bb2a Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:43:18 +0100 Subject: [PATCH 09/12] fixup --- lib/fetch/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 8d196b0021f..879ff560618 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1884,7 +1884,7 @@ function httpNetworkFetch ( const { done, value } = await iterator.next() bytes = done ? undefined : value } catch (err) { - if (this.body.readableEnded && !timingInfo.encodedBodySize) { + if (this.body._readableState.ended && !timingInfo.encodedBodySize) { // zlib doesn't like empty streams. bytes = undefined } else { From c94cbee4b36fd3c575ee8de3732d61f8c5d79870 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:46:28 +0100 Subject: [PATCH 10/12] fixup --- lib/fetch/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 879ff560618..d898d580db6 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1877,6 +1877,8 @@ function httpNetworkFetch ( const iterator = src[Symbol.asyncIterator]() pullAlgorithm = async (controller) => { + // 1-3. See onData... + // 4. Set bytes to the result of handling content codings given // codings and bytes. let bytes From a7c7b2d56a7a77f720672b5002fa4446206636bf Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:49:20 +0100 Subject: [PATCH 11/12] fixup --- lib/fetch/index.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index d898d580db6..15653ac5327 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1940,20 +1940,20 @@ function httpNetworkFetch ( return } - // 1. If one or more bytes have been transmitted from response’s - // message body, then: + // 1. If one or more bytes have been transmitted from response’s + // message body, then: - // 1. Let bytes be the transmitted bytes. + // 1. Let bytes be the transmitted bytes. const bytes = chunk - // 2. Let codings be the result of extracting header list values - // given `Content-Encoding` and response’s header list. - // See pullAlgorithm. + // 2. Let codings be the result of extracting header list values + // given `Content-Encoding` and response’s header list. + // See pullAlgorithm. - // 3. Increase timingInfo’s encoded body size by bytes’s length. + // 3. Increase timingInfo’s encoded body size by bytes’s length. timingInfo.encodedBodySize += bytes.byteLength - // 4. See pullAlgorithm... + // 4. See pullAlgorithm... return this.body.push(bytes) }, From 1d2cb46b43b7f853ca84dbfd8c8a271ed06ed7f7 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 12 Mar 2022 15:50:48 +0100 Subject: [PATCH 12/12] fixup --- lib/fetch/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fetch/index.js b/lib/fetch/index.js index 15653ac5327..9bc4f1e77d9 100644 --- a/lib/fetch/index.js +++ b/lib/fetch/index.js @@ -1899,10 +1899,10 @@ function httpNetworkFetch ( // body is done normally and stream is readable, then close // stream, finalize response for fetchParams and response, and // abort these in-parallel steps. - finalizeResponse(fetchParams, response) - controller.close() + finalizeResponse(fetchParams, response) + return }