From d97383226d3a75186ad6e7485363307dbe414a73 Mon Sep 17 00:00:00 2001 From: Dan Lapid Date: Wed, 22 Apr 2026 23:45:52 +0100 Subject: [PATCH] Revert "CACHE-13504: remove gate from request cf" --- src/workerd/api/http.c++ | 47 +++++++++++-------- src/workerd/api/tests/BUILD.bazel | 1 + .../api/tests/cf-cache-control-test.js | 1 + .../api/tests/cf-cache-control-test.wd-test | 4 +- 4 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++ index 1b86fe02681..3e707b0e9c3 100644 --- a/src/workerd/api/http.c++ +++ b/src/workerd/api/http.c++ @@ -687,7 +687,10 @@ kj::Maybe Request::serializeCfBlobJson(jsg::Lock& js) { bool hasCacheMode = (cacheMode != CacheMode::NONE); bool needsSynthesizedCacheControl = false; - if (!hasCacheMode) { + // TODO(cleanup): Remove the workerdExperimental gate once validated in production. + bool experimentalCacheControl = FeatureFlags::get(js).getWorkerdExperimental(); + + if (!hasCacheMode && experimentalCacheControl) { // Check if cf has cacheTtl but no cacheControl — we'll need to synthesize cacheControl. KJ_IF_SOME(cfObj, cf.get(js)) { if (!cfObj.has(js, "cacheControl") && cfObj.has(js, "cacheTtl")) { @@ -736,7 +739,8 @@ kj::Maybe Request::serializeCfBlobJson(jsg::Lock& js) { // Synthesize cacheControl from cacheTtl or cacheMode when cacheControl is not explicitly set. // This dual-writes both fields so downstream can migrate to cacheControl incrementally. - if (!obj.has(js, "cacheControl")) { + // TODO(cleanup): Remove the workerdExperimental gate once validated in production. + if (experimentalCacheControl && !obj.has(js, "cacheControl")) { if (hasCacheMode) { // Synthesize from the cache request option. switch (cacheMode) { @@ -787,25 +791,28 @@ void RequestInitializerDict::validate(jsg::Lock& js) { // cacheControl provides explicit Cache-Control header override and cannot be combined with // cacheTtl (which sets a simplified TTL) or the cache option (which maps to cacheTtl internally). // cacheTtlByStatus is allowed alongside cacheControl since they serve different purposes. - KJ_IF_SOME(cfRef, cf) { - auto cfObj = jsg::JsObject(cfRef.getHandle(js)); - if (cfObj.has(js, "cacheControl")) { - auto cacheControlVal = cfObj.get(js, "cacheControl"); - if (!cacheControlVal.isUndefined()) { - // cacheControl + cacheTtl → throw - if (cfObj.has(js, "cacheTtl")) { - auto cacheTtlVal = cfObj.get(js, "cacheTtl"); - JSG_REQUIRE(cacheTtlVal.isUndefined(), TypeError, - "The 'cacheControl' and 'cacheTtl' options on cf are mutually exclusive. " - "Use 'cacheControl' for explicit Cache-Control header directives, " - "or 'cacheTtl' for a simplified TTL, but not both."); + // TODO(cleanup): Remove the workerdExperimental gate once validated in production. + if (FeatureFlags::get(js).getWorkerdExperimental()) { + KJ_IF_SOME(cfRef, cf) { + auto cfObj = jsg::JsObject(cfRef.getHandle(js)); + if (cfObj.has(js, "cacheControl")) { + auto cacheControlVal = cfObj.get(js, "cacheControl"); + if (!cacheControlVal.isUndefined()) { + // cacheControl + cacheTtl → throw + if (cfObj.has(js, "cacheTtl")) { + auto cacheTtlVal = cfObj.get(js, "cacheTtl"); + JSG_REQUIRE(cacheTtlVal.isUndefined(), TypeError, + "The 'cacheControl' and 'cacheTtl' options on cf are mutually exclusive. " + "Use 'cacheControl' for explicit Cache-Control header directives, " + "or 'cacheTtl' for a simplified TTL, but not both."); + } + // cacheControl + cache option (no-store/no-cache) → throw + // The cache request option maps to cacheTtl internally, so they conflict. + JSG_REQUIRE(cache == kj::none, TypeError, + "The 'cacheControl' option on cf cannot be used together with the 'cache' " + "request option. The 'cache' option ('no-store'/'no-cache') maps to cache TTL " + "behavior internally, which conflicts with explicit Cache-Control directives."); } - // cacheControl + cache option (no-store/no-cache) → throw - // The cache request option maps to cacheTtl internally, so they conflict. - JSG_REQUIRE(cache == kj::none, TypeError, - "The 'cacheControl' option on cf cannot be used together with the 'cache' " - "request option. The 'cache' option ('no-store'/'no-cache') maps to cache TTL " - "behavior internally, which conflicts with explicit Cache-Control directives."); } } } diff --git a/src/workerd/api/tests/BUILD.bazel b/src/workerd/api/tests/BUILD.bazel index f7ac3a434a1..bfb39548b90 100644 --- a/src/workerd/api/tests/BUILD.bazel +++ b/src/workerd/api/tests/BUILD.bazel @@ -114,6 +114,7 @@ wd_test( wd_test( src = "cf-cache-control-test.wd-test", + args = ["--experimental"], data = ["cf-cache-control-test.js"], ) diff --git a/src/workerd/api/tests/cf-cache-control-test.js b/src/workerd/api/tests/cf-cache-control-test.js index e0d9168fe84..7d68c3c9081 100644 --- a/src/workerd/api/tests/cf-cache-control-test.js +++ b/src/workerd/api/tests/cf-cache-control-test.js @@ -3,6 +3,7 @@ // https://opensource.org/licenses/Apache-2.0 // Tests for cf.cacheControl mutual exclusion, synthesis, and additional cache settings. +// These require the workerd_experimental compat flag. import assert from 'node:assert'; diff --git a/src/workerd/api/tests/cf-cache-control-test.wd-test b/src/workerd/api/tests/cf-cache-control-test.wd-test index 00dd57384d8..55c3cce9ef4 100644 --- a/src/workerd/api/tests/cf-cache-control-test.wd-test +++ b/src/workerd/api/tests/cf-cache-control-test.wd-test @@ -10,7 +10,7 @@ const unitTests :Workerd.Config = ( bindings = [ ( name = "CACHE_ENABLED", json = "false" ), ], - compatibilityFlags = ["nodejs_compat", "cache_option_disabled"], + compatibilityFlags = ["nodejs_compat", "cache_option_disabled", "experimental"], ) ), ( name = "cf-cache-control-test-cache-enabled", @@ -21,7 +21,7 @@ const unitTests :Workerd.Config = ( bindings = [ ( name = "CACHE_ENABLED", json = "true" ), ], - compatibilityFlags = ["nodejs_compat", "cache_option_enabled"], + compatibilityFlags = ["nodejs_compat", "cache_option_enabled", "experimental"], ) ), ],