Skip to content

Commit

Permalink
fix(cache): set AbortSignal (#2612)
Browse files Browse the repository at this point in the history
* fix(cache): set AbortSignal

* use utility

* add test

* fix test

* fix test

* use fromInnerRequest
  • Loading branch information
tsctx authored Jan 19, 2024
1 parent c55b353 commit 12d39f7
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 24 deletions.
19 changes: 9 additions & 10 deletions lib/cache/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/cache/cachestorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 22 additions & 11 deletions lib/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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])
}
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -970,4 +981,4 @@ webidl.converters.RequestInit = webidl.dictionaryConverter([
}
])

module.exports = { Request, makeRequest }
module.exports = { Request, makeRequest, fromInnerRequest }
25 changes: 23 additions & 2 deletions test/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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)
})

0 comments on commit 12d39f7

Please sign in to comment.