fix(vtex): key the legacy PDP cache by pageHref and by the props that change it

vtex/loaders/intelligentSearch/productList.ts

@@ -10,6 +10,7 @@ import {
 import {
   getSegmentCacheKeyWithoutUTM,
   getSegmentFromBag,
+  keyUrlOf,
   withSegmentCookie,
 } from "../../utils/segment.ts";
 import { withIsSimilarTo } from "../../utils/similars.ts";
@@ -122,6 +123,11 @@ export interface CommonProps {
    * @description Defines the simulation behavior.
    */
   simulationBehavior?: SimulationBehavior;
+  /**
+   * @hide true
+   * @description The URL of the page, used to override URL from request
+   */
+  pageHref?: string;
 }
 
 /**
@@ -219,7 +225,12 @@ const loader = async (
   const props = expandedProps.props ??
     (expandedProps as unknown as Props["props"]);
   const { vcsDeprecated } = ctx;
-  const { url } = req;
+  // `pageHref` over `req.url`, same as the sibling productListingPage loader: it
+  // is what lets a caller that is not the page itself — a nested loader, a
+  // /live/invoke from the mobile app — land on the entry the storefront's own
+  // render produced. It also decides the base of every product URL in the
+  // payload below, so a shared entry must not be written with the caller's URL.
+  const url = keyUrlOf(props.pageHref, req).href;
   const segment = getSegmentFromBag(ctx);
   const locale = segment?.payload?.cultureInfo ??
     ctx.defaultSegment?.cultureInfo ?? "pt-BR";
@@ -325,18 +336,46 @@ export const cacheKey = (
   const props = expandedProps.props ??
     (expandedProps as unknown as Props["props"]);
 
-  const url = new URL(req.url);
+  const url = keyUrlOf(props.pageHref, req);
+
+  // A facets list is decided entirely by its props — fromProps reads nothing
+  // from the URL for it — so its key must not read the URL either. When the two
+  // disagree the key is finer than the fetch: the same payload gets one entry
+  // per `?q=`, and the caller's own path (a PDP, /deco/render, /live/invoke)
+  // splits it again. Narrow it here and the entry is shared by construction.
+  const keyedEntirelyByProps = isFacetsList(props);
+  const keyParams = keyedEntirelyByProps
+    ? new URLSearchParams()
+    : url.searchParams;
 
   const searchTerm = url.searchParams.get("q");
   const cachedSearchTerms = ctx.cachedSearchTerms ?? [];
   if (
-    // Avoid cache on loader call over call and on search pages
+    // Avoid cache on search pages whose term is not whitelisted. This stays in
+    // force for props-keyed lists too: an arbitrary term must never mint an entry.
     (!isQueryList(props) && searchTerm &&
-      !cachedSearchTerms.includes(searchTerm.toLowerCase())) || ctx.isInvoke
+      !cachedSearchTerms.includes(searchTerm.toLowerCase()))
   ) {
     return null;
   }
 
+  // Loader-over-loader and /live/invoke are exactly the calls this cache is worth
+  // the most to — the PDP backfill pays three of them — so the blanket
+  // `ctx.isInvoke` veto only survives where the URL still decides the search.
+  if (ctx.isInvoke && !keyedEntirelyByProps) {
+    return null;
+  }
+
+  // A facets list carries a free-text `query` of its own, and the whitelist above
+  // never sees it — `keyParams` is empty by design. Ungated on purpose: a
+  // top-level POST /live/invoke arrives with `ctx.isInvoke` false, so gating this
+  // on it would leave the public endpoint minting an entry, and an IS search, per
+  // arbitrary phrase.
+  const propsQuery = isFacetsList(props) ? props.query : undefined;
+  if (propsQuery && !cachedSearchTerms.includes(propsQuery.toLowerCase())) {
+    return null;
+  }
+
   if (
     url.search.includes("filter.")
   ) {
@@ -347,8 +386,12 @@ export const cacheKey = (
     ? getSegmentCacheKeyWithoutUTM(ctx)
     : getSegmentFromBag(ctx)?.token;
   const params = new URLSearchParams([
-    ...getSearchParams(props, url.searchParams, ctx),
+    ...getSearchParams(props, keyParams, ctx),
     ["segment", segmentCacheKey],
+    // Attaches isSimilarTo to every product and is not reachable from the URL.
+    // With the loader cache keyed by module, this key is the only thing keeping
+    // two callers of this loader apart.
+    ["similars", (props.similars ?? false).toString()],
   ]);
 
   if (

vtex/loaders/intelligentSearch/productListingPage.ts

@@ -17,6 +17,7 @@ import {
 import {
   getSegmentCacheKeyWithoutUTM,
   getSegmentFromBag,
+  keyUrlOf,
   withSegmentCookie,
 } from "../../utils/segment.ts";
 import { pageTypesFromUrl } from "../../utils/intelligentSearch.ts";
@@ -159,6 +160,20 @@ export interface Props {
    */
   simulationBehavior?: SimulationBehavior;
 }
+/**
+ * The page the fetch will actually ask for. Shared with `cacheKey`: the URL is
+ * 1-based (and configurable through `pageOffset`) while the API is 0-based, so a
+ * key that took the raw `?page` would hand page 0 the entry of page 1 whenever
+ * the two happen to normalise onto the same number.
+ */
+const pageOf = (props: Props, url: URL) => {
+  const offset = props.pageOffset ?? 1;
+  const fromUrl = url.searchParams.get("page");
+
+  return props.page ??
+    Math.min(fromUrl ? Number(fromUrl) - offset : 0, VTEX_MAX_PAGES - offset);
+};
+
 const searchArgsOf = (props: Props, url: URL, ctx: AppContext) => {
   const hideUnavailableItems = props.hideUnavailableItems ??
     ctx.advancedConfigs?.hideUnavailableItems;
@@ -168,14 +183,7 @@ const searchArgsOf = (props: Props, url: URL, ctx: AppContext) => {
   const countFromSearchParams = url.searchParams.get("PS");
   const count = Number(countFromSearchParams ?? props.count ?? 12);
   const query = props.query ?? url.searchParams.get("q") ?? "";
-  const currentPageoffset = props.pageOffset ?? 1;
-  const page = props.page ??
-    Math.min(
-      url.searchParams.get("page")
-        ? Number(url.searchParams.get("page")) - currentPageoffset
-        : 0,
-      VTEX_MAX_PAGES - currentPageoffset,
-    );
+  const page = pageOf(props, url);
   const sort = (url.searchParams.get("sort")) ??
     LEGACY_TO_IS[url.searchParams.get("O") ?? ""] ??
     props.sort ??
@@ -278,7 +286,9 @@ const loader = async (
 ): Promise<ProductListingPage | null> => {
   const { vcsDeprecated } = ctx;
   const { url: baseUrl } = req;
-  const url = new URL(props.pageHref || baseUrl);
+  const url = new URL(
+    props.pageHref && URL.canParse(props.pageHref) ? props.pageHref : baseUrl,
+  );
   const segment = getSegmentFromBag(ctx);
   const currentPageoffset = props.pageOffset ?? 1;
   const { selectedFacets: baseSelectedFacets, page, ...args } = searchArgsOf(
@@ -439,7 +449,7 @@ const loader = async (
 };
 export const cache = "stale-while-revalidate";
 export const cacheKey = (props: Props, req: Request, ctx: AppContext) => {
-  const url = new URL(props.pageHref || req.url);
+  const url = keyUrlOf(props.pageHref, req);
   const searchTerm = url.searchParams.get("q");
   const cachedSearchTerms = ctx.cachedSearchTerms ?? [];
   if (searchTerm && !cachedSearchTerms.includes(searchTerm.toLowerCase())) {
@@ -463,7 +473,7 @@ export const cacheKey = (props: Props, req: Request, ctx: AppContext) => {
   const params = new URLSearchParams([
     ["query", props.query ?? ""],
     ["count", (props.count || url.searchParams.get("count") || 12).toString()],
-    ["page", (props.page ?? url.searchParams.get("page") ?? 1).toString()],
+    ["page", pageOf(props, url).toString()],
     ["sort", props.sort ?? url.searchParams.get("sort") ?? ""],
     ["fuzzy", props.fuzzy ?? url.searchParams.get("fuzzy") ?? ""],
     [
@@ -486,15 +496,39 @@ export const cacheKey = (props: Props, req: Request, ctx: AppContext) => {
       url.searchParams.get("simulationBehavior") || props.simulationBehavior ||
       "default",
     ],
+    // Both change the payload — priceFacets adds the price facets (see the
+    // fselected branch), similars attaches isSimilarTo to every product — and
+    // neither is reachable from the URL. Since the loader cache is now keyed by
+    // the module, two callers of this same loader are separated by nothing but
+    // this key, so every prop that changes the result has to be in it.
+    ["priceFacets", (props.priceFacets ?? false).toString()],
+    ["similars", (props.similars ?? false).toString()],
+    // Renames the last pageType from the product's collection, so it changes the
+    // breadcrumb this payload carries. One PLP block in this storefront turns it
+    // on and the others leave it off.
+    ["useCollectionName", (props.useCollectionName ?? false).toString()],
   ]);
   url.searchParams.forEach((value, key) => {
     if (!ALLOWED_PARAMS.has(key.toLowerCase()) && !isFilterParam(key)) {
       return;
     }
+    // Case is preserved on purpose, even though the test above is
+    // case-insensitive. The loader reads `PS` and `O` uppercase and `q`
+    // lowercase, all case-sensitive, and none of the three is echoed by the
+    // props block above — so folding `?PS=24` onto `?ps=24` would hand one page
+    // size the other's entry. `?PAGE=1` keeping its own entry is a wasted hit;
+    // folding these would be wrong data.
     params.append(key, value);
   });
   params.sort();
-  url.search = params.toString();
+
+  // Props echoed above repeat through the URL — `?page=1` yields `page=1&page=1`
+  // where its absence yields `page=1` — which is two keys for one state. Only
+  // identical adjacent pairs collapse, so a genuine repeat with different values
+  // (filter.colors=azul&filter.colors=verde) survives: a dedup, not a `set`.
+  const pairs = params.toString().split("&");
+  url.search = pairs.filter((pair, i) => pair !== pairs[i - 1]).join("&");
+
   return url.href;
 };
 export default loader;

vtex/loaders/legacy/productDetailsPage.ts

@@ -6,6 +6,7 @@ import { toSegmentParams } from "../../utils/legacy.ts";
 import {
   getSegmentCacheKeyWithoutUTM,
   getSegmentFromBag,
+  keyUrlOf,
   withSegmentCookie,
 } from "../../utils/segment.ts";
 import { withIsSimilarTo } from "../../utils/similars.ts";
@@ -26,6 +27,14 @@ export interface Props {
    * @description Index of product pages with the `skuId` parameter
    */
   indexingSkus?: boolean;
+  /**
+   * @hide true
+   * @description Canonical page URL to key the cache by. Same contract, and the
+   * same `@hide`, as intelligentSearch/productListingPage: without it a call
+   * through /live/invoke keys by its own URL and can never share the entry the
+   * page render produced.
+   */
+  pageHref?: string;
   /**
    * @title Advanced Configuration
    * @description Further change loader behaviour
@@ -117,21 +126,41 @@ async function loader(
 export const cache = "stale-while-revalidate";
 
 export const cacheKey = (props: Props, req: Request, ctx: AppContext) => {
-  const url = new URL(req.url);
+  const reqUrl = new URL(req.url);
 
-  if (url.searchParams.has("ft")) {
+  if (reqUrl.searchParams.has("ft")) {
     return null;
   }
 
+  // props.pageHref first, like productListingPage does: the page render and a
+  // /live/invoke call for the same product have different request URLs, so
+  // keying by req.url alone gives each caller its own entry for identical data.
+  const url = keyUrlOf(props.pageHref, req);
+
   const segment = ctx.advancedConfigs?.removeUTMFromCacheKey
     ? getSegmentCacheKeyWithoutUTM(ctx)
     : getSegmentFromBag(ctx)?.token;
-  const skuId = url.searchParams.get("skuId") ?? "";
+  const skuId = reqUrl.searchParams.get("skuId") ?? "";
 
+  // Every prop that changes the payload belongs here. `similars` toggles
+  // withIsSimilarTo below, so leaving it out let whoever asked first decide
+  // whether isSimilarTo existed for everyone else until the entry expired.
   const params = new URLSearchParams([
     ["slug", props.slug],
     ["segment", segment ?? ""],
     ["skuId", skuId],
+    ["similars", String(props.similars ?? false)],
+    ["indexingSkus", String(props.indexingSkus ?? false)],
+    [
+      "preferDescription",
+      String(props.advancedConfigs?.preferDescription ?? false),
+    ],
+    [
+      // Stringified, not joined: `["a,b"]` and `["a","b"]` join to the same
+      // string and this list decides which attributes the payload carries.
+      "originalAttrs",
+      JSON.stringify(props.advancedConfigs?.includeOriginalAttributes ?? []),
+    ],
   ]);
 
   params.sort();

vtex/utils/segment.ts

@@ -87,6 +87,39 @@ export const setOrderFormIdInBag = (
   orderFormId: Promise<string | undefined>,
 ) => ctx?.bag?.set(ORDER_FORM_ID, orderFormId);
 
+/**
+ * btoa only accepts latin1 and THROWS on anything above it. The segment reaches
+ * it from an attacker-controlled cookie: a `\uXXXX` escape in the cookie's JSON
+ * survives atob + JSON.parse as a real code point above 0xFF and reaches btoa
+ * from there. (The `?sc=` query param is fenced off at the source instead — see
+ * buildSegmentFromRequest — because it also feeds the VTEXSC cookie.)
+ *
+ * That matters because `serialize` runs in the app middleware, on every request,
+ * and `getSegmentCacheKeyWithoutUTM` runs inside `cacheKey`, which the runtime
+ * calls outside its try — so a throw in either is a 500.
+ *
+ * Escaping back to ASCII keeps btoa in range without touching the value: the
+ * round-trip through atob + JSON.parse yields the exact same string.
+ */
+const toBase64 = (value: unknown) =>
+  btoa(
+    JSON.stringify(value).replace(
+      /[\u0080-\uffff]/g,
+      (c) => `\\u${c.charCodeAt(0).toString(16).padStart(4, "0")}`,
+    ),
+  );
+
+/**
+ * The URL the cache key is built from.
+ *
+ * `pageHref` is a prop, so it arrives from a public POST /live/invoke like any
+ * other, and `cacheKey` runs outside the runtime's try — a malformed value in
+ * `new URL` is a 500 the caller hands itself. An unparseable one falls back to
+ * the request, which is what the loader did before the prop existed.
+ */
+export const keyUrlOf = (pageHref: string | undefined, req: Request) =>
+  new URL(pageHref && URL.canParse(pageHref) ? pageHref : req.url);
+
 /**
  * Creates a stable cache key from segment that only includes business-critical fields.
  * Excludes marketing/tracking parameters (UTM, UTMI) to prevent cache fragmentation.
@@ -101,21 +134,43 @@ export const getSegmentCacheKeyWithoutUTM = (ctx: AppContext): string => {
   }
 
   // Only include fields that affect pricing, inventory, or content
+  //
+  // `channel` is deliberately absent. This storefront serves the website on
+  // trade policy 1 and the mobile app on policy 5, and the two mirror each
+  // other, so keying by the channel split one entry in two for byte-identical
+  // payloads — and the app reaches these loaders over /live/invoke carrying its
+  // own vtex_segment, so every shared path paid the fetch twice.
   const cacheRelevantSegment = {
     campaigns: segment.campaigns, // VTEX campaigns (can affect pricing)
-    channel: segment.channel, // Sales channel (affects inventory/pricing)
     priceTables: segment.priceTables, // Price tables (affects pricing)
     regionId: segment.regionId, // Region (can affect pricing/inventory)
     currencyCode: segment.currencyCode, // Currency
     cultureInfo: segment.cultureInfo, // Locale/language
     countryCode: segment.countryCode, // Country
-    channelPrivacy: segment.channelPrivacy, // Privacy settings
+    // Privacy settings. The default arrives spelled out — VTEX mints
+    // `"public"` while DEFAULT_SEGMENT omits the field — so only `"private"`
+    // earns an entry of its own.
+    channelPrivacy: segment.channelPrivacy === "public"
+      ? undefined
+      : segment.channelPrivacy,
     // EXCLUDED: utm_campaign, utm_source, utm_medium (marketing only)
     // EXCLUDED: utmi_campaign, utmi_page, utmi_part (VTEX tracking only)
   };
 
-  // Stable serialization for consistent cache keys
-  return btoa(JSON.stringify(cacheRelevantSegment));
+  // Absent, null and "" are one state, so they have to serialize as one.
+  // DEFAULT_SEGMENT carries no campaigns, priceTables, regionId or
+  // channelPrivacy, so the anonymous bag omits them entirely, while a shopper
+  // carrying a VTEX-minted vtex_segment sends them as null — same truth, two
+  // keys. The mobile app always carries that cookie, so every entry it shares
+  // with the anonymous SSR of the website depended on it. Same predicate the
+  // storefront applies on its own half of the key.
+  return toBase64(
+    Object.fromEntries(
+      Object.entries(cacheRelevantSegment).filter(([, value]) =>
+        Boolean(value)
+      ),
+    ),
+  );
 };
 /**
  * Stable serialization.
@@ -160,7 +215,7 @@ const serialize = ({
     cultureInfo,
     channelPrivacy,
   };
-  return btoa(JSON.stringify(seg));
+  return toBase64(seg);
 };
 
 const parse = (cookie: string) => {
@@ -190,8 +245,12 @@ export const buildSegmentFromRequest = (req: Request): Partial<Segment> => {
     }
   }
 
+  // A sales channel is a VTEX integer id. Take it only in that shape: the raw
+  // query value ends up in the VTEXSC cookie below, and setCookie rejects
+  // anything outside US-ASCII by throwing — in the app middleware, which runs on
+  // every request, so `?sc=<emoji>` on any URL would 500 the whole page.
   const sc = url.searchParams.get("sc");
-  if (sc) {
+  if (sc && /^\d+$/.test(sc)) {
     partialSegment.channel = sc;
   }