Set fetch mode to cors for image resource fetching#50
Set fetch mode to cors for image resource fetching#50marcoscaceres wants to merge 5 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the “fetching an image resource” algorithm to explicitly set the Fetch request’s mode to cors, requiring cross-origin servers to opt in via CORS headers for image resources to be usable.
Changes:
- Set
request/Modeto"cors"when creating a new request in the image resource fetch algorithm. - Expand the accompanying note to explain the CORS opt-in rationale.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Setting mode to "cors" unconditionally ensures that all image resource fetches use CORS, regardless of whether the caller passes a pre-configured Request or lets the algorithm create one. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add optional mode parameter (default "cors"), restricted to "cors" or "same-origin" via assert. - Replace TypeError on null client with assert, since callers like the manifest spec are UA-internal algorithms where throwing a TypeError is meaningless. - Move mode assignment outside the conditional branch so it applies unconditionally to all requests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
"cors" and "same-origin" are string literals, not defined terms. Use backtick code formatting instead of [=...=] cross-references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
marcoscaceres
force-pushed
the
cors-mode-default
branch
from
6eedb6c to
e0139e6
Compare
The Web App Manifest spec's "processing a manifest" algorithm now accepts an optional environment settings object (client) parameter, used to set the request client when fetching manifest image resources. This enables CSP and service worker interception for those fetches. Pass the link element's node document's relevant settings object as the client parameter when invoking "process the manifest". See: w3c/manifest#1171 See: w3c/image-resource#50
saschanaz
left a comment
saschanaz
left a comment
There was a problem hiding this comment.
I think ManifestIcons.sys.mjs is dead code right now, we do use ManifestObtainer but then the actual code path to get the icon is this which doesn't really care about manifest. And per the title of https://phabricator.services.mozilla.com/D279402 we definitely don't use manifest icon.
|
@copilot can you check @saschanaz’s comment above against geckos code? What are some suggestions for how we should proceed? |
The Web App Manifest spec's "processing a manifest" algorithm now accepts an optional environment settings object (client) parameter, used to set the request client when fetching manifest image resources. This enables CSP and service worker interception for those fetches. See w3c/manifest#1171 & w3c/image-resource#50 for context.
Summary
The "fetching an image resource" algorithm currently does not set a request mode, which defaults to
no-cors. This means cross-origin servers don't need to opt-in to having their resources used as image resources.This PR makes three changes to the algorithm:
modeparameter (default"cors", restricted to"cors"or"same-origin"). This lets callers opt for stricter same-origin fetching when needed, while defaulting to cors.if request is undefinedbranch, so it applies whether the algorithm creates the request or a caller passes one in.TypeErrorwith asserts for the client null-check and mode validation. The algorithm can be called from UA-internal contexts (e.g., manifest processing) where throwing a TypeError is meaningless. Asserts correctly signal spec bugs rather than runtime errors.Rationale
Access-Control-Allow-Originheaders for their resources to be usable.mode: "cors"inManifestIcons.sys.mjs).Context
This came up during review of w3c/manifest#1171, which adds a normative requirement for user agents to fetch manifest image resources using this algorithm. Checking browser implementations:
mode: "cors"when fetching manifest icons.DownloadImageInFramethrough the document frame (mode handling is internal).With
no-cors, a manifest author can reference cross-origin icon URLs that cause the user agent to make requests to servers that haven't opted in. While a simple GET still reaches the server regardless of mode,corsensures the response is a network error without CORS headers, making the icon unusable. This disincentivizes using arbitrary third-party URLs as icon sources and is consistent with howfetch()itself defaults tocors.Preview | Diff