Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't store realm on Request/Response #3146

Merged
merged 2 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions lib/web/cache/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -502,8 +502,7 @@ class Cache {
const requestObject = fromInnerRequest(
request,
new AbortController().signal,
'immutable',
{ settingsObject: request.client }
'immutable'
)
// 5.4.2.1
requestList.push(requestObject)
Expand Down Expand Up @@ -779,7 +778,7 @@ class Cache {
// 5.5.2
for (const response of responses) {
// 5.5.2.1
const responseObject = fromInnerResponse(response, 'immutable', { settingsObject: {} })
const responseObject = fromInnerResponse(response, 'immutable')

responseList.push(responseObject.clone())

Expand Down
58 changes: 21 additions & 37 deletions lib/web/eventsource/eventsource.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
const { pipeline } = require('node:stream')
const { fetching } = require('../fetch')
const { makeRequest } = require('../fetch/request')
const { getGlobalOrigin } = require('../fetch/global')
const { webidl } = require('../fetch/webidl')
const { EventSourceStream } = require('./eventsource-stream')
const { parseMIMEType } = require('../fetch/data-url')
const { MessageEvent } = require('../websocket/events')
const { isNetworkError } = require('../fetch/response')
const { delay } = require('./util')
const { kEnumerableProperty } = require('../../core/util')
const { environmentSettingsObject } = require('../fetch/util')

let experimentalWarned = false

Expand Down Expand Up @@ -65,12 +65,6 @@ const ANONYMOUS = 'anonymous'
*/
const USE_CREDENTIALS = 'use-credentials'

/**
* @typedef {object} EventSourceInit
* @property {boolean} [withCredentials] indicates whether the request
* should include credentials.
*/

/**
* The EventSource interface is used to receive server-sent events. It
* connects to a server over HTTP and receives events in text/event-stream
Expand All @@ -97,12 +91,9 @@ class EventSource extends EventTarget {
#dispatcher

/**
* @type {object}
* @property {string} lastEventId
* @property {number} reconnectionTime
* @property {any} reconnectionTimer
* @type {import('./eventsource-stream').eventSourceSettings}
*/
#settings = null
#state

/**
* Creates a new EventSource object.
Expand All @@ -127,24 +118,21 @@ class EventSource extends EventTarget {
eventSourceInitDict = webidl.converters.EventSourceInitDict(eventSourceInitDict)

this.#dispatcher = eventSourceInitDict.dispatcher

// 2. Let settings be ev's relevant settings object.
// https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object
this.#settings = {
origin: getGlobalOrigin(),
policyContainer: {
referrerPolicy: 'no-referrer'
},
this.#state = {
lastEventId: '',
reconnectionTime: defaultReconnectionTime
}

// 2. Let settings be ev's relevant settings object.
// https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object
const settings = environmentSettingsObject

let urlRecord

try {
// 3. Let urlRecord be the result of encoding-parsing a URL given url, relative to settings.
urlRecord = new URL(url, this.#settings.origin)
this.#settings.origin = urlRecord.origin
urlRecord = new URL(url, settings.settingsObject.baseUrl)
this.#state.origin = urlRecord.origin
} catch (e) {
// 4. If urlRecord is failure, then throw a "SyntaxError" DOMException.
throw new DOMException(e, 'SyntaxError')
Expand Down Expand Up @@ -178,7 +166,7 @@ class EventSource extends EventTarget {
}

// 9. Set request's client to settings.
initRequest.client = this.#settings
initRequest.client = environmentSettingsObject.settingsObject

// 10. User agents may set (`Accept`, `text/event-stream`) in request's header list.
initRequest.headersList = [['accept', { name: 'accept', value: 'text/event-stream' }]]
Expand Down Expand Up @@ -229,7 +217,7 @@ class EventSource extends EventTarget {

this.#readyState = CONNECTING

const fetchParam = {
const fetchParams = {
request: this.#request,
dispatcher: this.#dispatcher
}
Expand All @@ -245,10 +233,10 @@ class EventSource extends EventTarget {
}

// 15. Fetch request, with processResponseEndOfBody set to processEventSourceEndOfBody...
fetchParam.processResponseEndOfBody = processEventSourceEndOfBody
fetchParams.processResponseEndOfBody = processEventSourceEndOfBody

// and processResponse set to the following steps given response res:
fetchParam.processResponse = (response) => {
fetchParams.processResponse = (response) => {
// 1. If res is an aborted network error, then fail the connection.

if (isNetworkError(response)) {
Expand Down Expand Up @@ -297,10 +285,10 @@ class EventSource extends EventTarget {
this.dispatchEvent(new Event('open'))

// If redirected to a different origin, set the origin to the new origin.
this.#settings.origin = response.urlList[response.urlList.length - 1].origin
this.#state.origin = response.urlList[response.urlList.length - 1].origin

const eventSourceStream = new EventSourceStream({
eventSourceSettings: this.#settings,
eventSourceSettings: this.#state,
push: (event) => {
this.dispatchEvent(new MessageEvent(
event.type,
Expand All @@ -321,7 +309,7 @@ class EventSource extends EventTarget {
})
}

this.#controller = fetching(fetchParam)
this.#controller = fetching(fetchParams)
}

/**
Expand All @@ -346,7 +334,7 @@ class EventSource extends EventTarget {
this.dispatchEvent(new Event('error'))

// 2. Wait a delay equal to the reconnection time of the event source.
await delay(this.#settings.reconnectionTime)
await delay(this.#state.reconnectionTime)

// 5. Queue a task to run the following steps:

Expand All @@ -361,8 +349,8 @@ class EventSource extends EventTarget {
// string, encoded as UTF-8.
// 2. Set (`Last-Event-ID`, lastEventIDValue) in request's header
// list.
if (this.#settings.lastEventId !== '') {
this.#request.headersList.set('last-event-id', this.#settings.lastEventId, true)
if (this.#state.lastEventId.length) {
this.#request.headersList.set('last-event-id', this.#state.lastEventId, true)
}

// 4. Fetch request and process the response obtained in this fashion, if any, as described earlier in this section.
Expand All @@ -378,12 +366,8 @@ class EventSource extends EventTarget {

if (this.#readyState === CLOSED) return
this.#readyState = CLOSED
clearTimeout(this.#settings.reconnectionTimer)
this.#controller.abort()

if (this.#request) {
this.#request = null
}
this.#request = null
}

get onopen () {
Expand Down
3 changes: 1 addition & 2 deletions lib/web/fetch/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ function fetch (input, init = undefined) {
let responseObject = null

// 8. Let relevantRealm be this’s relevant Realm.
const relevantRealm = null

// 9. Let locallyAborted be false.
let locallyAborted = false
Expand Down Expand Up @@ -229,7 +228,7 @@ function fetch (input, init = undefined) {

// 4. Set responseObject to the result of creating a Response object,
// given response, "immutable", and relevantRealm.
responseObject = fromInnerResponse(response, 'immutable', relevantRealm)
responseObject = fromInnerResponse(response, 'immutable')

// 5. Resolve p with responseObject.
p.resolve(responseObject)
Expand Down
19 changes: 7 additions & 12 deletions lib/web/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const {
requestDuplex
} = require('./constants')
const { kEnumerableProperty } = util
const { kHeaders, kSignal, kState, kGuard, kRealm, kDispatcher } = require('./symbols')
const { kHeaders, kSignal, kState, kGuard, kDispatcher } = require('./symbols')
const { webidl } = require('./webidl')
const { URLSerializer } = require('./data-url')
const { kHeadersList, kConstruct } = require('../../core/symbols')
Expand Down Expand Up @@ -53,17 +53,14 @@ class Request {
input = webidl.converters.RequestInfo(input)
init = webidl.converters.RequestInit(init)

// https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object
this[kRealm] = environmentSettingsObject

// 1. Let request be null.
let request = null

// 2. Let fallbackMode be null.
let fallbackMode = null

// 3. Let baseURL be this’s relevant settings object’s API base URL.
const baseUrl = this[kRealm].settingsObject.baseUrl
const baseUrl = environmentSettingsObject.settingsObject.baseUrl

// 4. Let signal be null.
let signal = null
Expand Down Expand Up @@ -110,7 +107,7 @@ class Request {
}

// 7. Let origin be this’s relevant settings object’s origin.
const origin = this[kRealm].settingsObject.origin
const origin = environmentSettingsObject.settingsObject.origin

// 8. Let window be "client".
let window = 'client'
Expand Down Expand Up @@ -146,7 +143,7 @@ class Request {
// unsafe-request flag Set.
unsafeRequest: request.unsafeRequest,
// client This’s relevant settings object.
client: this[kRealm].settingsObject,
client: environmentSettingsObject.settingsObject,
// window window.
window,
// priority request’s priority.
Expand Down Expand Up @@ -235,7 +232,7 @@ class Request {
// then set request’s referrer to "client".
if (
(parsedReferrer.protocol === 'about:' && parsedReferrer.hostname === 'client') ||
(origin && !sameOrigin(parsedReferrer, this[kRealm].settingsObject.baseUrl))
(origin && !sameOrigin(parsedReferrer, environmentSettingsObject.settingsObject.baseUrl))
) {
request.referrer = 'client'
} else {
Expand Down Expand Up @@ -759,7 +756,7 @@ class Request {
}

// 4. Return clonedRequestObject.
return fromInnerRequest(clonedRequest, ac.signal, this[kHeaders][kGuard], this[kRealm])
return fromInnerRequest(clonedRequest, ac.signal, this[kHeaders][kGuard])
}

[nodeUtil.inspect.custom] (depth, options) {
Expand Down Expand Up @@ -862,13 +859,11 @@ function cloneRequest (request) {
* @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) {
function fromInnerRequest (innerRequest, signal, guard) {
const request = new Request(kConstruct)
request[kState] = innerRequest
request[kRealm] = realm
request[kSignal] = signal
request[kHeaders] = new Headers(kConstruct)
request[kHeaders][kHeadersList] = innerRequest.headersList
Expand Down
16 changes: 6 additions & 10 deletions lib/web/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const {
redirectStatusSet,
nullBodyStatus
} = require('./constants')
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const { kState, kHeaders, kGuard } = require('./symbols')
const { webidl } = require('./webidl')
const { FormData } = require('./formdata')
const { URLSerializer } = require('./data-url')
Expand All @@ -36,7 +36,7 @@ class Response {
// The static error() method steps are to return the result of creating a
// Response object, given a new network error, "immutable", and this’s
// relevant Realm.
const responseObject = fromInnerResponse(makeNetworkError(), 'immutable', relevantRealm)
const responseObject = fromInnerResponse(makeNetworkError(), 'immutable')

return responseObject
}
Expand All @@ -59,7 +59,7 @@ class Response {

// 3. Let responseObject be the result of creating a Response object, given a new response,
// "response", and this’s relevant Realm.
const responseObject = fromInnerResponse(makeResponse({}), 'response', relevantRealm)
const responseObject = fromInnerResponse(makeResponse({}), 'response')

// 4. Perform initialize a response given responseObject, init, and (body, "application/json").
initializeResponse(responseObject, init, { body: body[0], type: 'application/json' })
Expand Down Expand Up @@ -93,7 +93,7 @@ class Response {

// 4. Let responseObject be the result of creating a Response object,
// given a new response, "immutable", and this’s relevant Realm.
const responseObject = fromInnerResponse(makeResponse({}), 'immutable', relevantRealm)
const responseObject = fromInnerResponse(makeResponse({}), 'immutable')

// 5. Set responseObject’s response’s status to status.
responseObject[kState].status = status
Expand All @@ -120,8 +120,6 @@ class Response {

init = webidl.converters.ResponseInit(init)

this[kRealm] = relevantRealm

// 1. Set this’s response to a new response.
this[kState] = makeResponse({})

Expand Down Expand Up @@ -243,7 +241,7 @@ class Response {

// 3. Return the result of creating a Response object, given
// clonedResponse, this’s headers’s guard, and this’s relevant Realm.
return fromInnerResponse(clonedResponse, this[kHeaders][kGuard], this[kRealm])
return fromInnerResponse(clonedResponse, this[kHeaders][kGuard])
}

[nodeUtil.inspect.custom] (depth, options) {
Expand Down Expand Up @@ -504,13 +502,11 @@ function initializeResponse (response, init, body) {
* @see https://fetch.spec.whatwg.org/#response-create
* @param {any} innerResponse
* @param {'request' | 'immutable' | 'request-no-cors' | 'response' | 'none'} guard
* @param {any} [realm]
* @returns {Response}
*/
function fromInnerResponse (innerResponse, guard, realm) {
function fromInnerResponse (innerResponse, guard) {
const response = new Response(kConstruct)
response[kState] = innerResponse
response[kRealm] = realm
response[kHeaders] = new Headers(kConstruct)
response[kHeaders][kHeadersList] = innerResponse.headersList
response[kHeaders][kGuard] = guard
Expand Down
1 change: 0 additions & 1 deletion lib/web/fetch/symbols.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ module.exports = {
kSignal: Symbol('signal'),
kState: Symbol('state'),
kGuard: Symbol('guard'),
kRealm: Symbol('realm'),
kDispatcher: Symbol('dispatcher')
}
9 changes: 3 additions & 6 deletions test/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const {
Blob: ThirdPartyBlob,
FormData: ThirdPartyFormData
} = require('formdata-node')
const { kState, kGuard, kRealm, kSignal, kHeaders } = require('../../lib/web/fetch/symbols')
const { kState, kGuard, kSignal, kHeaders } = require('../../lib/web/fetch/symbols')
const { kHeadersList } = require('../../lib/core/symbols')

const hasSignalReason = 'reason' in AbortSignal.prototype
Expand Down Expand Up @@ -487,17 +487,14 @@ test('Issue#2465', async (t) => {
})

test('fromInnerRequest', () => {
const realm = { settingsObject: {} }
const innerRequest = makeRequest({
urlList: [new URL('http://asd')],
client: realm.settingsObject
urlList: [new URL('http://asd')]
})
const signal = new AbortController().signal
const request = fromInnerRequest(innerRequest, signal, 'immutable', realm)
const request = fromInnerRequest(innerRequest, signal, 'immutable')

// check property
assert.strictEqual(request[kState], innerRequest)
assert.strictEqual(request[kRealm], realm)
assert.strictEqual(request[kSignal], signal)
assert.strictEqual(request[kHeaders][kHeadersList], innerRequest.headersList)
assert.strictEqual(request[kHeaders][kGuard], 'immutable')
Expand Down