fix(cache): set AbortSignal (nodejs#2612)

* fix(cache): set AbortSignal * use utility * add test * fix test * fix test * use fromInnerRequest
crysmags · Feb 27, 2024 · 26b8ebb · 26b8ebb
1 parent 9028aed
commit 26b8ebb
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 24 deletions.
diff --git a/lib/cache/cache.js b/lib/cache/cache.js
@@ -6,9 +6,9 @@ const { kEnumerableProperty, isDisturbed } = require('../core/util')
 const { kHeadersList } = require('../core/symbols')
 const { webidl } = require('../fetch/webidl')
 const { Response, cloneResponse } = require('../fetch/response')
-const { Request } = require('../fetch/request')
+const { Request, fromInnerRequest } = require('../fetch/request')
 const { Headers } = require('../fetch/headers')
-const { kState, kHeaders, kGuard, kRealm } = require('../fetch/symbols')
+const { kState, kHeaders, kGuard } = require('../fetch/symbols')
 const { fetching } = require('../fetch/index')
 const { urlIsHttpHttpsScheme, createDeferredPromise, readAllBytes } = require('../fetch/util')
 const assert = require('assert')
@@ -444,7 +444,7 @@ class Cache {
    * @see https://w3c.github.io/ServiceWorker/#dom-cache-keys
    * @param {any} request
    * @param {import('../../types/cache').CacheQueryOptions} options
-   * @returns {readonly Request[]}
+   * @returns {Promise<readonly Request[]>}
    */
   async keys (request = undefined, options = {}) {
     webidl.brandCheck(this, Cache)
@@ -503,13 +503,12 @@ class Cache {
 
       // 5.4.2
       for (const request of requests) {
-        const requestObject = new Request(kConstruct)
-        requestObject[kState] = request
-        requestObject[kHeaders] = new Headers(kConstruct)
-        requestObject[kHeaders][kHeadersList] = request.headersList
-        requestObject[kHeaders][kGuard] = 'immutable'
-        requestObject[kRealm] = request.client
-
+        const requestObject = fromInnerRequest(
+          request,
+          new AbortController().signal,
+          'immutable',
+          { settingsObject: request.client }
+        )
         // 5.4.2.1
         requestList.push(requestObject)
       }

diff --git a/lib/cache/cachestorage.js b/lib/cache/cachestorage.js
@@ -114,7 +114,7 @@ class CacheStorage {
 
   /**
    * @see https://w3c.github.io/ServiceWorker/#cache-storage-keys
-   * @returns {string[]}
+   * @returns {Promise<string[]>}
    */
   async keys () {
     webidl.brandCheck(this, CacheStorage)

diff --git a/lib/fetch/request.js b/lib/fetch/request.js
@@ -751,14 +751,6 @@ class Request {
 
     // 3. Let clonedRequestObject be the result of creating a Request object,
     // given clonedRequest, this’s headers’s guard, and this’s relevant Realm.
-    const clonedRequestObject = new Request(kConstruct)
-    clonedRequestObject[kState] = clonedRequest
-    clonedRequestObject[kRealm] = this[kRealm]
-    clonedRequestObject[kHeaders] = new Headers(kConstruct)
-    clonedRequestObject[kHeaders][kHeadersList] = clonedRequest.headersList
-    clonedRequestObject[kHeaders][kGuard] = this[kHeaders][kGuard]
-    clonedRequestObject[kHeaders][kRealm] = this[kHeaders][kRealm]
-
     // 4. Make clonedRequestObject’s signal follow this’s signal.
     const ac = new AbortController()
     if (this.signal.aborted) {
@@ -771,10 +763,9 @@ class Request {
         }
       )
     }
-    clonedRequestObject[kSignal] = ac.signal
 
     // 4. Return clonedRequestObject.
-    return clonedRequestObject
+    return fromInnerRequest(clonedRequest, ac.signal, this[kHeaders][kGuard], this[kRealm])
   }
 }
 
@@ -844,6 +835,26 @@ function cloneRequest (request) {
   return newRequest
 }
 
+/**
+ * @param {any} innerRequest
+ * @param {AbortSignal} signal
+ * @param {'request' | 'immutable' | 'request-no-cors' | 'response' | 'none'} guard
+ * @param {any} [realm]
+ * @returns {Request}
+ */
+function fromInnerRequest (innerRequest, signal, guard, realm) {
+  const request = new Request(kConstruct)
+  request[kState] = innerRequest
+  request[kRealm] = realm
+  request[kSignal] = signal
+  request[kSignal][kRealm] = realm
+  request[kHeaders] = new Headers(kConstruct)
+  request[kHeaders][kHeadersList] = innerRequest.headersList
+  request[kHeaders][kGuard] = guard
+  request[kHeaders][kRealm] = realm
+  return request
+}
+
 Object.defineProperties(Request.prototype, {
   method: kEnumerableProperty,
   url: kEnumerableProperty,
@@ -970,4 +981,4 @@ webidl.converters.RequestInit = webidl.dictionaryConverter([
   }
 ])
 
-module.exports = { Request, makeRequest }
+module.exports = { Request, makeRequest, fromInnerRequest }
diff --git a/test/fetch/request.js b/test/fetch/request.js
@@ -10,10 +10,13 @@ const {
   Headers,
   fetch
 } = require('../../')
+const { fromInnerRequest, makeRequest } = require('../../lib/fetch/request')
 const {
   Blob: ThirdPartyBlob,
   FormData: ThirdPartyFormData
 } = require('formdata-node')
+const { kState, kGuard, kRealm, kSignal, kHeaders } = require('../../lib/fetch/symbols')
+const { kHeadersList } = require('../../lib/core/symbols')
 
 const hasSignalReason = 'reason' in AbortSignal.prototype
 
@@ -472,9 +475,8 @@ test('Clone the set-cookie header when Request is passed as the first parameter
 
 // Tests for optimization introduced in https://github.com/nodejs/undici/pull/2456
 test('keys to object prototypes method', (t) => {
-  const { ok } = tspl(t, { plan: 1 })
   const request = new Request('http://localhost', { method: 'hasOwnProperty' })
-  ok(typeof request.method === 'string')
+  assert(typeof request.method === 'string')
 })
 
 // https://github.com/nodejs/undici/issues/2465
@@ -483,3 +485,22 @@ test('Issue#2465', async (t) => {
   const request = new Request('http://localhost', { body: new SharedArrayBuffer(0), method: 'POST' })
   strictEqual(await request.text(), '[object SharedArrayBuffer]')
 })
+
+test('fromInnerRequest', () => {
+  const realm = { settingsObject: {} }
+  const innerRequest = makeRequest({
+    urlList: [new URL('http://asd')],
+    client: realm.settingsObject
+  })
+  const signal = new AbortController().signal
+  const request = fromInnerRequest(innerRequest, signal, 'immutable', realm)
+
+  // check property
+  assert.strictEqual(request[kState], innerRequest)
+  assert.strictEqual(request[kRealm], realm)
+  assert.strictEqual(request[kSignal], signal)
+  assert.strictEqual(request[kSignal][kRealm], realm)
+  assert.strictEqual(request[kHeaders][kHeadersList], innerRequest.headersList)
+  assert.strictEqual(request[kHeaders][kGuard], 'immutable')
+  assert.strictEqual(request[kHeaders][kRealm], realm)
+})