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.

Sign up for GitHub

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

Ignore unsupported fetch options and properties as recommended #3598

Merged
merged 1 commit into from
Feb 24, 2025
+27 −58
Merged

Ignore unsupported fetch options and properties as recommended #3598

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 27 additions & 58 deletions src/workerd/api/http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,42 +676,22 @@ struct RequestInitializerDict {
// and passed on to the next worker? Then `cf` is just one such field: it's not special,
// it's only named `cf` because the consumer is Cloudflare code.

// These control CORS policy. This doesn't matter on the edge because CSRF is not possible
// here:
// 1. We don't have the user's credentials (e.g. cookies) for any other origin, so we couldn't
// forge a request from the user even if we wanted to.
// 2. We aren't behind the user's firewall, so we also can't forge requests to unauthenticated
// internal network services.
jsg::WontImplement mode;

// These control CORS policy. This doesn't matter on the edge because CSRF is not possible
// here:
// 1. We don't have the user's credentials (e.g. cookies) for any other origin, so we couldn't
// forge a request from the user even if we wanted to.
// 2. We aren't behind the user's firewall, so we also can't forge requests to unauthenticated
// internal network services.
jsg::WontImplement credentials;
// The fetch standard defines additional properties that are really only relevant in browser
// implementations that implement CORS. The WinterTC has determined that for non-browser
// environments, these should be silently ignoredif the runtime has no use for them.
// * mode
// * credentials
// * referrer
// * referrerPolicy
// * keepalive
// * window

// In browsers this controls the local browser cache. For Cloudflare Workers it could control the
// Cloudflare edge cache. While the standard defines a number of values for this property, our
// implementation supports only three: undefined (identifying the default caching behavior that
// has been implemented by the runtime), "no-store", and "no-cache".
jsg::Optional<kj::String> cache;

// These control how the `Referer` and `Origin` headers are initialized by the browser.
// Browser-side JavaScript is normally not permitted to set these headers, because servers
// sometimes use the headers to defend against CSRF. On the edge, CSRF is not a risk (see
// comments about `mode` and `credentials`, above), hence protecting the Referer and Origin
// headers is not necessary, so we treat them as regular-old headers instead.
jsg::WontImplement referrer;

// These control how the `Referer` and `Origin` headers are initialized by the browser.
// Browser-side JavaScript is normally not permitted to set these headers, because servers
// sometimes use the headers to defend against CSRF. On the edge, CSRF is not a risk (see
// comments about `mode` and `credentials`, above), hence protecting the Referer and Origin
// headers is not necessary, so we treat them as regular-old headers instead.
jsg::WontImplement referrerPolicy;

// Subresource integrity (check response against a given hash).
// We do not implement integrity checking, however, we will accept either an undefined
// or empty string value for the property. If any other value is given we will throw.
Expand All @@ -727,13 +707,6 @@ struct RequestInitializerDict {
// `null`.
jsg::Optional<kj::Maybe<jsg::Ref<AbortSignal>>> signal;

// We do not support keepalive currently and may never?
// Per the spec, keepalive is "a boolean indicating whether or not request can
// outlive the global in which it was created." We could choose to explicitly indicate
// that we do not support this option but for now we'll just ignore it.
// jsg::Optional<bool> keepalive;
// TODO(conform): Won't support?

// The duplex option controls whether or not a fetch is expected to send the entire request
// before processing the response. The default value ("half"), which is currently the only
// option supported by the standard, dictates that the request is fully sent before handling
Expand All @@ -751,8 +724,7 @@ struct RequestInitializerDict {
// jsg::Optional<kj::String> priority;
// TODO(conform): Might support later?

JSG_STRUCT(method, headers, body, redirect, fetcher, cf, mode, credentials, cache,
referrer, referrerPolicy, integrity, signal);
JSG_STRUCT(method, headers, body, redirect, fetcher, cf, cache, integrity, signal);
JSG_STRUCT_TS_OVERRIDE_DYNAMIC(CompatibilityFlags::Reader flags) {
if(flags.getCacheOptionEnabled()) {
if(flags.getCacheNoCache()) {
Expand Down Expand Up @@ -876,10 +848,6 @@ class Request final: public Body {
// Returns the `cf` field containing Cloudflare feature flags.
jsg::Optional<jsg::JsObject> getCf(jsg::Lock& js);

// We do not implement support for the keepalive option but we do want to at least provide
// the standard property, hard-coded to always be false.
bool getKeepalive() { return false; }

// The duplex option controls whether or not a fetch is expected to send the entire request
// before processing the response. The default value ("half"), which is currently the only
// option supported by the standard, dictates that the request is fully sent before handling
Expand All @@ -889,18 +857,23 @@ class Request final: public Body {
// jsg::JsValue getDuplex(jsg::Lock& js) { return js.v8Undefined(); }
// TODO(conform): Might implement?

// These relate to CORS support, which we do not implement. In the
// Request initializer we will explicitly throw if any attempt is
// made to specify these. For the accessors tho, we want it to always
// just return undefined rather than throw, which helps with code
// portability across multiple runtimes. The spec says that the default
// value for mode when not specified *should* be 'no-cors`, but that
// value implies strict limitations that we do not follow. In discussion
// with other implementers with the same issues, it was decided that
// simply returning undefined for these was the best option.
// jsg::JsValue getMode(jsg::Lock& js) { return js.v8Undefined(); }
// jsg::JsValue getCredentials(jsg::Lock& js) { return js.v8Undefined(); }
// TODO(conform): Won't implement?
// These relate to CORS support, which we do not implement. WinterTC has determined that
// non-browser implementations that do not implement CORS support should ignore these
// entirely as if they were not defined.
// * destination
// * mode
// * credentials
// * referrer
// * referrerPolicy
// * isReloadNavigation
// * isHistoryNavigation
// * keepalive (see below)

// We do not implement support for the keepalive option but we do want to at least provide
// the standard property, hard-coded to always be false. WinterTC actually recommends that
// this one just be left undefined but we already had this returning false always and it
// would require a compat flag to remove. Just keep it as it's harmless.
bool getKeepalive() { return false; }

// The cache mode determines how HTTP cache is used with the request.
jsg::Optional<kj::StringPtr> getCache(jsg::Lock& js);
Expand Down Expand Up @@ -932,8 +905,6 @@ class Request final: public Body {
// TODO(conform): These are standard properties that we do not implement (see descriptions
// above).
// JSG_READONLY_PROTOTYPE_PROPERTY(duplex, getDuplex);
// JSG_READONLY_PROTOTYPE_PROPERTY(mode, getMode);
// JSG_READONLY_PROTOTYPE_PROPERTY(credentials, getCredentials);
JSG_READONLY_PROTOTYPE_PROPERTY(integrity, getIntegrity);
JSG_READONLY_PROTOTYPE_PROPERTY(keepalive, getKeepalive);
if(flags.getCacheOptionEnabled()) {
Expand Down Expand Up @@ -976,8 +947,6 @@ class Request final: public Body {
// TODO(conform): These are standard properties that we do not implement (see descriptions
// above).
// JSG_READONLY_INSTANCE_PROPERTY(duplex, getDuplex);
// JSG_READONLY_INSTANCE_PROPERTY(mode, getMode);
// JSG_READONLY_INSTANCE_PROPERTY(credentials, getCredentials);
JSG_READONLY_INSTANCE_PROPERTY(integrity, getIntegrity);
JSG_READONLY_INSTANCE_PROPERTY(keepalive, getKeepalive);

Expand Down