Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,6 @@ server/
deps.json

deno.lock

.cursorindexingignore
.specstory/
142 changes: 142 additions & 0 deletions blocks/matcher.cache-page.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import { assert, assertEquals } from "@std/assert";
import matcherBlock, { type MatcherModule } from "./matcher.ts";
import {
DECO_PAGE_CACHE_ALLOW_HEADER,
DECO_PAGE_CACHE_CONTROL_HEADER,
} from "../utils/http.ts";

// Minimal HttpContext stub with just what matcher.adapt needs
const makeHttpCtx = (resolverPath: string, headers: Headers) =>
({
resolveChain: [{ type: "resolvable", value: resolverPath }],
context: {
state: {
response: { headers },
flags: [] as Array<
{ name: string; value: boolean; isSegment: boolean }
>,
},
},
request: new Request("http://local/"),
}) as any;

// A matcher module that always returns true (so we can see when page caching forces false)
const TRUE_MATCHER: MatcherModule = {
default: () => true,
};

Deno.test("page cache ON: allows device matcher; blocks others by default", async () => {
const headers = new Headers();
headers.set(
DECO_PAGE_CACHE_CONTROL_HEADER,
"public, s-maxage=60, max-age=10",
);

// Allowed: device
{
const ctx = makeHttpCtx("/site/matchers/device.tsx", headers);
const adapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/device.tsx");
const fn = await (adapted as any)({}, ctx);
const result = fn({ device: {} as any, siteId: 1, request: ctx.request });
assert(result, "device matcher should be allowed when page-cache is ON");
// flag recorded and true
const flag = ctx.context.state.flags[0];
assertEquals(flag?.name, "/site/matchers/device.tsx");
assertEquals(flag?.value, true);
}

// Blocked: any non device/time matcher
{
const ctx = makeHttpCtx("/site/matchers/url.tsx", headers);
const adapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/url.tsx");
const fn = await (adapted as any)({}, ctx);
const result = fn({ device: {} as any, siteId: 1, request: ctx.request });
assertEquals(
result,
false,
"non device/time matchers must be disabled when page-cache is ON",
);
const flag = ctx.context.state.flags[0];
assertEquals(flag?.name, "/site/matchers/url.tsx");
assertEquals(flag?.value, false);
}
});

Deno.test("page cache ON: allows time matchers (date/cron)", async () => {
const headers = new Headers();
headers.set(
DECO_PAGE_CACHE_CONTROL_HEADER,
"public, s-maxage=60, max-age=10",
);

const dateCtx = makeHttpCtx("/site/matchers/date.ts", headers);
const dateAdapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/date.ts");
const dateFn = await (dateAdapted as any)({}, dateCtx);
assert(
dateFn({ device: {} as any, siteId: 1, request: dateCtx.request }),
"date matcher should be allowed",
);
assertEquals(dateCtx.context.state.flags[0]?.value, true);

const cronCtx = makeHttpCtx("/site/matchers/cron.ts", headers);
const cronAdapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/cron.ts");
const cronFn = await (cronAdapted as any)({}, cronCtx);
assert(
cronFn({ device: {} as any, siteId: 1, request: cronCtx.request }),
"cron matcher should be allowed",
);
assertEquals(cronCtx.context.state.flags[0]?.value, true);
});

Deno.test("page cache ON: honor allow-list header", async () => {
const headers = new Headers();
headers.set(
DECO_PAGE_CACHE_CONTROL_HEADER,
"public, s-maxage=60, max-age=10",
);
headers.set(DECO_PAGE_CACHE_ALLOW_HEADER, "device"); // time not allowed now

const ctx = makeHttpCtx("/site/matchers/date.ts", headers);
const adapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/date.ts");
const fn = await (adapted as any)({}, ctx);
const result = fn({ device: {} as any, siteId: 1, request: ctx.request });
assertEquals(
result,
false,
"time matcher should be disabled when 'device' is the only allowed group",
);
});

Deno.test("page cache OFF: evaluate matcher normally", async () => {
const headers = new Headers(); // no page cache header
const ctx = makeHttpCtx("/site/matchers/url.tsx", headers);
const adapted = matcherBlock.adapt!(TRUE_MATCHER, "site/matchers/url.tsx");
const fn = await (adapted as any)({}, ctx);
const result = fn({ device: {} as any, siteId: 1, request: ctx.request });
assert(
result,
"when page-cache is OFF, non device/time matchers are evaluated normally",
);
});

// Sticky-session: ensure that when page cache is ON and a matcher is not allowed,
// we do not set sticky cookies.
Deno.test("page cache ON: sticky-session matcher does not set cookie when disabled", async () => {
const headers = new Headers();
headers.set(
DECO_PAGE_CACHE_CONTROL_HEADER,
"public, s-maxage=60, max-age=10",
);
const STICKY_TRUE_MATCHER: MatcherModule = {
sticky: "session",
default: () => true,
sessionKey: () => "k",
} as any;
const ctx = makeHttpCtx("/site/matchers/url.tsx", headers);
const adapted = matcherBlock.adapt!(STICKY_TRUE_MATCHER, "site/matchers/url.tsx");
const fn = await (adapted as any)({}, ctx);
const result = fn({ device: {} as any, siteId: 1, request: ctx.request });
assertEquals(result, false);
// should not set cookie when disabled by page caching
assertEquals(headers.has("set-cookie"), false);
});
58 changes: 45 additions & 13 deletions blocks/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import { getCookies, Murmurhash3, setCookie } from "../deps.ts";
import type { Block, BlockModule, InstanceOf } from "../engine/block.ts";
import type { Device } from "../utils/userAgent.ts";
import type { RequestState } from "./utils.tsx";
import {
DECO_PAGE_CACHE_ALLOW_HEADER,
DECO_PAGE_CACHE_CONTROL_HEADER,
} from "../utils/http.ts";

export type Matcher = InstanceOf<typeof matcherBlock, "#/root/matchers">;

Expand Down Expand Up @@ -147,6 +151,7 @@ const matcherBlock: Block<
return (ctx: MatchContext) => {
let uniqueId = "";
let isSegment = true;
let matcherResolver = "";

// from last to first and stop in the first resolvable
// the rationale behind is: whenever you enter a resolvable it means that it can be referenced by other resolvables and this value should not change.
Expand All @@ -160,6 +165,7 @@ const matcherBlock: Block<
// stop on first resolvable
if (type === "resolvable") {
isSegment = uniqueId === value;
matcherResolver = `${value}`;
break;
}
}
Expand All @@ -168,9 +174,32 @@ const matcherBlock: Block<
);

let result = isEnabled;
// If page Cache-Control is enabled, ignore all matchers that are not device/time based.
const pageCachingOn = respHeaders.has(DECO_PAGE_CACHE_CONTROL_HEADER);
if (pageCachingOn) {
const allow = respHeaders.get(DECO_PAGE_CACHE_ALLOW_HEADER) ??
"device,time";
const allowDevice = allow.includes("device");
const allowTime = allow.includes("time");
const isDeviceMatcher = /\/matchers\/device\.tsx?$/.test(
matcherResolver,
);
const isTimeMatcher = /\/matchers\/(date|cron)\.tsx?$/.test(
matcherResolver,
) || /\/matchers\/(date|cron)\.ts$/.test(matcherResolver);
const allowed = (allowDevice && isDeviceMatcher) ||
(allowTime && isTimeMatcher);
if (!allowed) {
result = false;
}
}
Comment on lines +177 to +195
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n 'DECO_PAGE_CACHE_ALLOW_HEADER' --type ts -A2 -B2

Repository: deco-cx/deco

Length of output: 1793


🏁 Script executed:

rg -n 'set.*DECO_PAGE_CACHE_ALLOW_HEADER|\.set\([^,]*x-deco-page-cache-allow' --type ts -A1 -B1

Repository: deco-cx/deco

Length of output: 251


🏁 Script executed:

# Also check for any documentation or comments about header format
rg -n 'device.*time|allow.*header' --type ts -i -A2 -B2 | head -50

Repository: deco-cx/deco

Length of output: 2453


Consider trimming individual values when parsing the allow-list header.

While the documented format is "device,time" (without spaces), the current .includes() checks are susceptible to malformed headers. For example, if a header is set to "device, time" (with space after comma), allow.includes("time") would fail. Use .split(",").map(s => s.trim()) to make the parsing more robust.

🤖 Prompt for AI Agents
In blocks/matcher.ts around lines 177 to 195, the parsing of the
DECO_PAGE_CACHE_ALLOW_HEADER uses allow.includes(...) which breaks on values
with surrounding whitespace (e.g., "device, time"); split the header on commas,
trim each entry, and check membership against that normalized list (e.g., const
parts = (allow ?? "").split(",").map(s => s.trim()).filter(Boolean); then use
parts.includes("device") and parts.includes("time") to set
allowDevice/allowTime) so malformed spacing no longer prevents matching.

// if it is not sticky then we can run the matcher function
if (!shouldStickyOnSession) {
result ??= matcherFunc(ctx);
if (pageCachingOn && result === false) {
// matcher disabled by page caching, do not evaluate
} else {
result ??= matcherFunc(ctx);
}
} else {
hasher.hash(uniqueId);
const _sessionKey = matcherModule.sessionKey
Expand All @@ -182,18 +211,21 @@ const matcherBlock: Block<
const isMatchFromCookie = cookieValue.boolean(
getCookies(ctx.request.headers)[cookieName],
);
result ??= isMatchFromCookie ?? matcherFunc(ctx);
if (result !== isMatchFromCookie) {
const date = new Date();
date.setTime(date.getTime() + (30 * 24 * 60 * 60 * 1000)); // 1 month
setCookie(respHeaders, {
name: cookieName,
value: cookieValue.build(uniqueId, result),
path: "/",
sameSite: "Lax",
expires: date,
});
respHeaders.append("vary", "cookie");
const skipStickyOps = pageCachingOn && result === false;
if (!skipStickyOps) {
result ??= isMatchFromCookie ?? matcherFunc(ctx);
if (result !== isMatchFromCookie) {
const date = new Date();
date.setTime(date.getTime() + (30 * 24 * 60 * 60 * 1000)); // 1 month
setCookie(respHeaders, {
name: cookieName,
value: cookieValue.build(uniqueId, result),
path: "/",
sameSite: "Lax",
expires: date,
});
respHeaders.append("vary", "cookie");
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions engine/manifest/manifestGen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ export async function* defaultWalker(
includeDirs: false,
includeFiles: true,
exts: ["tsx", "jsx", "ts", "js"],
// Avoid scanning VCS, build artifacts and dependencies
skip: [
/[\\/]\.git([\\/]|$)/,
/[\\/]node_modules([\\/]|$)/,
/[\\/]_fresh([\\/]|$)/,
/[\\/]dist([\\/]|$)/,
],
});
}
export const decoManifestBuilder = async (
Expand Down
12 changes: 12 additions & 0 deletions runtime/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,18 @@ export const middlewareFor = <TAppManifest extends AppManifest = AppManifest>(
}
}

// Apply page-level Cache-Control if configured and safe to cache
// Local constants to avoid cross-package import cycles
const DECO_PAGE_CACHE_CONTROL_HEADER = "x-deco-page-cache-control";
const isGet = ctx.req.raw.method === "GET";
const pageCC = ctx.var.response.headers.get(
DECO_PAGE_CACHE_CONTROL_HEADER,
);
const shouldCachePage = pageCC && isGet && ctx?.var?.vary?.shouldCache;
if (shouldCachePage) {
newHeaders.set("cache-control", pageCC as string);
}

// for some reason hono deletes content-type when response is not fresh.
// which means that sometimes it will fail as headers are immutable.
// so I'm first setting it to undefined and just then set the entire response again
Expand Down
7 changes: 3 additions & 4 deletions runtime/routes/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,9 @@ export const handler = createHandler(async (
if (shouldCache && shouldCacheFromVary) {
// Stale cache on CDN, but make the browser fetch every single time.
// We can test if caching on the browser helps too.
response.headers.set(
"cache-control",
DECO_RENDER_CACHE_CONTROL,
);
const DECO_PAGE_CACHE_CONTROL_HEADER = "x-deco-page-cache-control";
const pageCC = ctx.var.response.headers.get(DECO_PAGE_CACHE_CONTROL_HEADER);
response.headers.set("cache-control", pageCC ?? DECO_RENDER_CACHE_CONTROL);
} else {
response.headers.set(
"cache-control",
Expand Down
41 changes: 41 additions & 0 deletions utils/http.cache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {
DEFAULT_CACHE_CONTROL,
formatCacheControl,
normalizeCacheControlHeader,
parseCacheControl,
} from "./http.ts";
import { assertEquals, assertStringIncludes } from "@std/assert";

Deno.test("normalizeCacheControlHeader: true uses DEFAULT_CACHE_CONTROL", () => {
const header = normalizeCacheControlHeader(true)!;
const expected = formatCacheControl(DEFAULT_CACHE_CONTROL);
assertEquals(header, expected);
});

Deno.test("normalizeCacheControlHeader: valid string is preserved (normalized)", () => {
const input =
"public, s-maxage=120, max-age=10, stale-while-revalidate=3600, stale-if-error=86400";
const header = normalizeCacheControlHeader(input)!;
// Order is not guaranteed; verify presence of key segments
assertStringIncludes(header, "public");
assertStringIncludes(header, "s-maxage=120");
assertStringIncludes(header, "max-age=10");
// Round-trip parsing should preserve numeric values
const parsed = parseCacheControl(new Headers({ "cache-control": header }));
assertEquals(parsed["public"], true);
assertEquals(parsed["s-maxage"], 120);
assertEquals(parsed["max-age"], 10);
});

Deno.test("normalizeCacheControlHeader: invalid string falls back to default", () => {
const header = normalizeCacheControlHeader("totally-invalid-directive=foo")!;
const expected = formatCacheControl(DEFAULT_CACHE_CONTROL);
assertEquals(header, expected);
});

Deno.test("normalizeCacheControlHeader: falsy => undefined (disabled)", () => {
const h1 = normalizeCacheControlHeader(undefined);
const h2 = normalizeCacheControlHeader(false);
assertEquals(h1, undefined);
assertEquals(h2, undefined);
});
35 changes: 35 additions & 0 deletions utils/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,41 @@ export const defaultHeaders = {
["x-powered-by"]: `deco@${denoJSON.version}`,
};

/**
* Internal header used to signal that the current page should emit Cache-Control.
* The value must be a valid Cache-Control string; when present, middleware will apply it if safe.
*/
export const DECO_PAGE_CACHE_CONTROL_HEADER = "x-deco-page-cache-control";
/**
* Internal header used to list which matcher groups are allowed to vary when page cache-control is enabled.
* Example value: "device,time"
*/
export const DECO_PAGE_CACHE_ALLOW_HEADER = "x-deco-page-cache-allow";

/**
* Normalize a cache-control configuration into a valid header string.
* - true => DEFAULT_CACHE_CONTROL
* - string => parsed/validated and re-formatted; on invalid, falls back to DEFAULT_CACHE_CONTROL
* - falsy/undefined => undefined (disabled)
*/
export const normalizeCacheControlHeader = (
value?: boolean | string,
): string | undefined => {
if (!value) return undefined;
if (value === true) {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
try {
const h = new Headers({ "cache-control": value });
const parsed = parseCacheControl(h);
// If nothing parsed, treat as invalid
const normalized = formatCacheControl(parsed);
return normalized || formatCacheControl(DEFAULT_CACHE_CONTROL);
} catch {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
};
Comment on lines +163 to +179
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fallback when cache-control numbers are invalid.

Line 176 ends up returning strings like max-age=NaN whenever the supplied directive has a non-numeric value (e.g. max-age=foo). In those cases we wanted to fall back to DEFAULT_CACHE_CONTROL, but the current logic formats the NaN instead. That produces an invalid Cache-Control header and breaks downstream caching. Please detect non-finite numeric tokens before formatting and fall back to the default when they appear.

Apply this diff:

   try {
     const h = new Headers({ "cache-control": value });
     const parsed = parseCacheControl(h);
+    const hasInvalidNumbers = Object.values(parsed).some((entry) =>
+      typeof entry === "number" && !Number.isFinite(entry)
+    );
+    if (hasInvalidNumbers) {
+      return formatCacheControl(DEFAULT_CACHE_CONTROL);
+    }
     // If nothing parsed, treat as invalid
     const normalized = formatCacheControl(parsed);
     return normalized || formatCacheControl(DEFAULT_CACHE_CONTROL);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const normalizeCacheControlHeader = (
value?: boolean | string,
): string | undefined => {
if (!value) return undefined;
if (value === true) {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
try {
const h = new Headers({ "cache-control": value });
const parsed = parseCacheControl(h);
// If nothing parsed, treat as invalid
const normalized = formatCacheControl(parsed);
return normalized || formatCacheControl(DEFAULT_CACHE_CONTROL);
} catch {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
};
export const normalizeCacheControlHeader = (
value?: boolean | string,
): string | undefined => {
if (!value) return undefined;
if (value === true) {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
try {
const h = new Headers({ "cache-control": value });
const parsed = parseCacheControl(h);
const hasInvalidNumbers = Object.values(parsed).some((entry) =>
typeof entry === "number" && !Number.isFinite(entry)
);
if (hasInvalidNumbers) {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
// If nothing parsed, treat as invalid
const normalized = formatCacheControl(parsed);
return normalized || formatCacheControl(DEFAULT_CACHE_CONTROL);
} catch {
return formatCacheControl(DEFAULT_CACHE_CONTROL);
}
};
🤖 Prompt for AI Agents
In utils/http.ts around lines 163 to 179, the normalization currently formats
parsed numeric directives (e.g. max-age) even when they parse to NaN (producing
"max-age=NaN"); detect non-finite numeric tokens on the parsed result before
calling formatCacheControl and if any numeric directive is not a finite number,
return formatCacheControl(DEFAULT_CACHE_CONTROL) instead. Concretely: after
parseCacheControl(h) iterate the parsed directives that should be numeric
(max-age, s-maxage, stale-while-revalidate, stale-if-error, etc.), coerce their
values to Number and use Number.isFinite(...) to validate; if any fail, treat as
invalid and fall back to DEFAULT_CACHE_CONTROL, otherwise proceed to format and
return the normalized header.


export function setCSPHeaders(
request: Request,
response: Response,
Expand Down
6 changes: 6 additions & 0 deletions utils/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@ export { tryOrDefault } from "./object.ts";
export type { DotNestedKeys } from "./object.ts";
export { createServerTimings } from "./timings.ts";
export type { Device } from "./userAgent.ts";

export {
DECO_PAGE_CACHE_ALLOW_HEADER,
DECO_PAGE_CACHE_CONTROL_HEADER,
normalizeCacheControlHeader,
} from "./http.ts";