fix(fetch): multiple data: url fixes & move tests to wpt runner (nodejs#1678)

Author: KhafraDev

lib/fetch/dataURL.js

@@ -1,5 +1,6 @@
 const assert = require('assert')
 const { atob } = require('buffer')
+const { isValidHTTPToken } = require('./util')
 
 const encoder = new TextEncoder()
 
@@ -376,9 +377,7 @@ function parseMIMEType (input) {
       // 1. Set parameterValue to the result of collecting
       // an HTTP quoted string from input, given position
       // and the extract-value flag.
-      // Undici implementation note: extract-value is never
-      // defined or mentioned anywhere.
-      parameterValue = collectAnHTTPQuotedString(input, position/*, extractValue */)
+      parameterValue = collectAnHTTPQuotedString(input, position, true)
 
       // 2. Collect a sequence of code points that are not
       // U+003B (;) from input, given position.
@@ -400,7 +399,8 @@ function parseMIMEType (input) {
       )
 
       // 2. Remove any trailing HTTP whitespace from parameterValue.
-      parameterValue = parameterValue.trim()
+      // Note: it says "trailing" whitespace; leading is fine.
+      parameterValue = parameterValue.trimEnd()
 
       // 3. If parameterValue is the empty string, then continue.
       if (parameterValue.length === 0) {
@@ -547,11 +547,56 @@ function collectAnHTTPQuotedString (input, position, extractValue) {
   return input.slice(positionStart, position.position)
 }
 
+/**
+ * @see https://mimesniff.spec.whatwg.org/#serialize-a-mime-type
+ */
+function serializeAMimeType (mimeType) {
+  assert(mimeType !== 'failure')
+  const { type, subtype, parameters } = mimeType
+
+  // 1. Let serialization be the concatenation of mimeType’s
+  //    type, U+002F (/), and mimeType’s subtype.
+  let serialization = `${type}/${subtype}`
+
+  // 2. For each name → value of mimeType’s parameters:
+  for (let [name, value] of parameters.entries()) {
+    // 1. Append U+003B (;) to serialization.
+    serialization += ';'
+
+    // 2. Append name to serialization.
+    serialization += name
+
+    // 3. Append U+003D (=) to serialization.
+    serialization += '='
+
+    // 4. If value does not solely contain HTTP token code
+    //    points or value is the empty string, then:
+    if (!isValidHTTPToken(value)) {
+      // 1. Precede each occurence of U+0022 (") or
+      //    U+005C (\) in value with U+005C (\).
+      value = value.replace(/(\\|")/g, '\\$1')
+
+      // 2. Prepend U+0022 (") to value.
+      value = '"' + value
+
+      // 3. Append U+0022 (") to value.
+      value += '"'
+    }
+
+    // 5. Append value to serialization.
+    serialization += value
+  }
+
+  // 3. Return serialization.
+  return serialization
+}
+
 module.exports = {
   dataURLProcessor,
   URLSerializer,
   collectASequenceOfCodePoints,
   stringPercentDecode,
   parseMIMEType,
-  collectAnHTTPQuotedString
+  collectAnHTTPQuotedString,
+  serializeAMimeType
 }

lib/fetch/index.js

@@ -52,7 +52,7 @@ const { kHeadersList } = require('../core/symbols')
 const EE = require('events')
 const { Readable, pipeline } = require('stream')
 const { isErrored, isReadable } = require('../core/util')
-const { dataURLProcessor } = require('./dataURL')
+const { dataURLProcessor, serializeAMimeType } = require('./dataURL')
 const { TransformStream } = require('stream/web')
 
 /** @type {import('buffer').resolveObjectURL} */
@@ -832,33 +832,15 @@ async function schemeFetch (fetchParams) {
       }
 
       // 3. Let mimeType be dataURLStruct’s MIME type, serialized.
-      const { mimeType } = dataURLStruct
-
-      /** @type {string} */
-      let contentType = `${mimeType.type}/${mimeType.subtype}`
-      const contentTypeParams = []
-
-      if (mimeType.parameters.size > 0) {
-        contentType += ';'
-      }
-
-      for (const [key, value] of mimeType.parameters) {
-        if (value.length > 0) {
-          contentTypeParams.push(`${key}=${value}`)
-        } else {
-          contentTypeParams.push(key)
-        }
-      }
-
-      contentType += contentTypeParams.join(',')
+      const mimeType = serializeAMimeType(dataURLStruct.mimeType)
 
       // 4. Return a response whose status message is `OK`,
       //    header list is « (`Content-Type`, mimeType) »,
       //    and body is dataURLStruct’s body.
       return makeResponse({
         statusText: 'OK',
         headersList: [
-          ['content-type', contentType]
+          ['content-type', mimeType]
         ],
         body: extractBody(dataURLStruct.body)[0]
       })

test/fetch/data-uri.js

@@ -9,8 +9,6 @@ const {
   collectAnHTTPQuotedString
 } = require('../../lib/fetch/dataURL')
 const { fetch } = require('../..')
-const base64tests = require('./resources/base64.json')
-const dataURLtests = require('./resources/data-urls.json')
 
 test('https://url.spec.whatwg.org/#concept-url-serializer', (t) => {
   t.test('url scheme gets appended', (t) => {
@@ -121,7 +119,7 @@ test('https://mimesniff.spec.whatwg.org/#parse-a-mime-type', (t) => {
   t.same(parseMIMEType('text/html;charset="shift_jis"iso-2022-jp'), {
     type: 'text',
     subtype: 'html',
-    parameters: new Map([['charset', '"shift_jis"']])
+    parameters: new Map([['charset', 'shift_jis']])
   })
 
   t.same(parseMIMEType('application/javascript'), {
@@ -161,71 +159,18 @@ test('https://fetch.spec.whatwg.org/#collect-an-http-quoted-string', (t) => {
   t.end()
 })
 
-// https://github.com/web-platform-tests/wpt/blob/master/fetch/data-urls/resources/base64.json
-// https://github.com/web-platform-tests/wpt/blob/master/fetch/data-urls/base64.any.js
-test('base64.any.js', async (t) => {
-  for (const [input, output] of base64tests) {
-    const dataURL = `data:;base64,${input}`
-
-    if (output === null) {
-      await t.rejects(fetch(dataURL), TypeError)
-      continue
-    }
-
-    try {
-      const res = await fetch(dataURL)
-      const body = await res.arrayBuffer()
-
-      t.same(
-        new Uint8Array(body),
-        new Uint8Array(output)
-      )
-    } catch (e) {
-      t.fail(`failed to fetch ${dataURL}`)
-    }
+// https://github.com/nodejs/undici/issues/1574
+test('too long base64 url', async (t) => {
+  const inputStr = 'a'.repeat(1 << 20)
+  const base64 = Buffer.from(inputStr).toString('base64')
+  const dataURIPrefix = 'data:application/octet-stream;base64,'
+  const dataURL = dataURIPrefix + base64
+  try {
+    const res = await fetch(dataURL)
+    const buf = await res.arrayBuffer()
+    const outputStr = Buffer.from(buf).toString('ascii')
+    t.same(outputStr, inputStr)
+  } catch (e) {
+    t.fail(`failed to fetch ${dataURL}`)
   }
 })
-
-test('processing.any.js', async (t) => {
-  for (const [input, expectedMimeType, expectedBody = null] of dataURLtests) {
-    if (expectedMimeType === null) {
-      try {
-        await fetch(input)
-        t.fail(`fetching "${input}" was expected to fail`)
-      } catch (e) {
-        t.ok(e, 'got expected error')
-        continue
-      }
-    }
-
-    try {
-      const res = await fetch(input)
-      const body = await res.arrayBuffer()
-
-      t.same(
-        new Uint8Array(body),
-        new Uint8Array(expectedBody)
-      )
-    } catch (e) {
-      t.fail(`failed on '${input}'`)
-    }
-  }
-
-  // https://github.com/nodejs/undici/issues/1574
-  test('too long base64 url', async (t) => {
-    const inputStr = 'a'.repeat(1 << 20)
-    const base64 = Buffer.from(inputStr).toString('base64')
-    const dataURIPrefix = 'data:application/octet-stream;base64,'
-    const dataURL = dataURIPrefix + base64
-    try {
-      const res = await fetch(dataURL)
-      const buf = await res.arrayBuffer()
-      const outputStr = Buffer.from(buf).toString('ascii')
-      t.same(outputStr, inputStr)
-    } catch (e) {
-      t.fail(`failed to fetch ${dataURL}`)
-    }
-  })
-
-  t.end()
-})

test/wpt/runner/runner/runner.mjs

@@ -36,7 +36,10 @@ export class WPTRunner extends EventEmitter {
     super()
 
     this.#folderPath = join(testPath, folder)
-    this.#files.push(...WPTRunner.walk(this.#folderPath, () => true))
+    this.#files.push(...WPTRunner.walk(
+      this.#folderPath,
+      (file) => file.endsWith('.js')
+    ))
     this.#status = JSON.parse(readFileSync(join(statusPath, `${folder}.status.json`)))
     this.#url = url
 

test/wpt/runner/runner/worker.mjs

@@ -1,3 +1,4 @@
+import { join } from 'node:path'
 import { runInThisContext } from 'node:vm'
 import { parentPort, workerData } from 'node:worker_threads'
 import {
@@ -10,7 +11,10 @@ import {
   Headers
 } from '../../../../index.js'
 
-const { initScripts, meta, test, url } = workerData
+const { initScripts, meta, test, url, path } = workerData
+
+const basePath = join(process.cwd(), 'test/wpt/tests')
+const urlPath = path.slice(basePath.length)
 
 const globalPropertyDescriptors = {
   writable: true,
@@ -86,7 +90,7 @@ add_completion_callback((_, status) => {
   })
 })
 
-setGlobalOrigin(url)
+setGlobalOrigin(new URL(urlPath, url))
 
 // Inject any script the user provided before
 // running the tests.

test/wpt/server/server.mjs

@@ -6,7 +6,7 @@ import { fileURLToPath } from 'node:url'
 import { createReadStream } from 'node:fs'
 import { setTimeout as sleep } from 'node:timers/promises'
 
-const resources = fileURLToPath(join(import.meta.url, '../../runner/resources'))
+const tests = fileURLToPath(join(import.meta.url, '../../tests'))
 
 // https://web-platform-tests.org/tools/wptserve/docs/stash.html
 class Stash extends Map {
@@ -30,13 +30,16 @@ const server = createServer(async (req, res) => {
   const fullUrl = new URL(req.url, `http://localhost:${server.address().port}`)
 
   switch (fullUrl.pathname) {
-    case '/resources/data.json': {
+    case '/fetch/data-urls/resources/base64.json':
+    case '/fetch/data-urls/resources/data-urls.json':
+    case '/fetch/api/resources/empty.txt':
+    case '/fetch/api/resources/data.json': {
       // https://github.com/web-platform-tests/wpt/blob/6ae3f702a332e8399fab778c831db6b7dca3f1c6/fetch/api/resources/data.json
-      return createReadStream(join(resources, 'data.json'))
+      return createReadStream(join(tests, fullUrl.pathname))
         .on('end', () => res.end())
         .pipe(res)
     }
-    case '/resources/infinite-slow-response.py': {
+    case '/fetch/api/resources/infinite-slow-response.py': {
       // https://github.com/web-platform-tests/wpt/blob/master/fetch/api/resources/infinite-slow-response.py
       const stateKey = fullUrl.searchParams.get('stateKey') ?? ''
       const abortKey = fullUrl.searchParams.get('abortKey') ?? ''
@@ -66,7 +69,7 @@ const server = createServer(async (req, res) => {
 
       return res.end()
     }
-    case '/resources/stash-take.py': {
+    case '/fetch/api/resources/stash-take.py': {
       // https://github.com/web-platform-tests/wpt/blob/6ae3f702a332e8399fab778c831db6b7dca3f1c6/fetch/api/resources/stash-take.py
 
       const key = fullUrl.searchParams.get('key')
@@ -77,11 +80,6 @@ const server = createServer(async (req, res) => {
       res.write(JSON.stringify(took))
       return res.end()
     }
-    case '/resources/empty.txt': {
-      return createReadStream(join(resources, 'empty.txt'))
-        .on('end', () => res.end())
-        .pipe(res)
-    }
     default: {
       res.statusCode = 200
       res.end('body')

test/wpt/runner/resources/data.json renamed to test/wpt/tests/fetch/api/resources/data.json

@@ -0,0 +1,18 @@
+// META: global=window,worker
+
+promise_test(() => fetch("resources/base64.json").then(res => res.json()).then(runBase64Tests), "Setup.");
+function runBase64Tests(tests) {
+  for(let i = 0; i < tests.length; i++) {
+    const input = tests[i][0],
+          output = tests[i][1],
+          dataURL = "data:;base64," + input;
+    promise_test(t => {
+      if(output === null) {
+        return promise_rejects_js(t, TypeError, fetch(dataURL));
+      }
+      return fetch(dataURL).then(res => res.arrayBuffer()).then(body => {
+        assert_array_equals(new Uint8Array(body), output);
+      });
+    }, "data: URL base64 handling: " + format_value(input));
+  }
+}

test/wpt/runner/resources/empty.txt renamed to test/wpt/tests/fetch/api/resources/empty.txt

@@ -0,0 +1,22 @@
+// META: global=window,worker
+
+promise_test(() => fetch("resources/data-urls.json").then(res => res.json()).then(runDataURLTests), "Setup.");
+function runDataURLTests(tests) {
+  for(let i = 0; i < tests.length; i++) {
+    const input = tests[i][0],
+          expectedMimeType = tests[i][1],
+          expectedBody = expectedMimeType !== null ? tests[i][2] : null;
+    promise_test(t => {
+      if(expectedMimeType === null) {
+        return promise_rejects_js(t, TypeError, fetch(input));
+      } else {
+        return fetch(input).then(res => {
+          return res.arrayBuffer().then(body => {
+            assert_array_equals(new Uint8Array(body), expectedBody);
+            assert_equals(res.headers.get("content-type"), expectedMimeType); // We could assert this earlier, but this fails often
+          });
+        });
+      }
+    }, format_value(input));
+  }
+}