fix: address PR review feedback

- Fix unreachable secure logic in getRenderingKey by removing default resolver.
- Correct JSDocs for host-based cache keys and getSafeHost fallback behavior.
- Fix case-sensitivity in TestEngineRunner request mock.
- Resolve type errors in default optimization options.

Author: l3tchupkt

core-libs/setup/ssr/express-utils/express-request-safe-host.ts

@@ -7,10 +7,15 @@
 import { Request } from 'express';
 
 /**
- * Returns a safe host from the request object.
+ * Returns a trusted host from the request object.
  *
- * It checks the `X-Forwarded-Host` header and validates it against the `allowedHosts` list.
- * If the header is missing, invalid, or not in the allowlist, it falls back to the `Host` header.
+ * It first checks the `X-Forwarded-Host` header and returns it only if it is valid
+ * and matches the `allowedHosts` list. If `X-Forwarded-Host` is missing, invalid,
+ * or not allowlisted, it then checks the `Host` header and returns it only if it is
+ * also valid and allowlisted.
+ *
+ * If neither header passes validation and allowlist checks, this function returns
+ * `undefined`, and callers should treat that as "no trusted host".
  *
  * @param req - Express Request object
  * @param allowedHosts - Optional list of allowed hosts

core-libs/setup/ssr/optimized-engine/ssr-optimization-options.ts

@@ -216,7 +216,7 @@ export interface SsrOptimizationOptions {
    * List of allowed hosts for host-based cache keys.
    *
    * If `useHostInCacheKey` is set to true, only hosts in this list will be used in the cache key.
-   * If the host is not in this list, it will fallback to the value of the `Host` header.
+   * If neither `X-Forwarded-Host` nor `Host` matching this list is found, the cache key will not include a host prefix.
    */
   allowedHosts?: string[];
 }
@@ -247,8 +247,12 @@ type DefaultSsrOptimizationOptions = Omit<
   Required<SsrOptimizationOptions>,
   | 'debug' // debug is deprecated and not used anymore
   | 'ttl' // ttl is required but its default value is `undefined`
+  | 'renderKeyResolver' // renderKeyResolver is optional and defaults to undefined to allow for secure default logic in the engine
+  | 'allowedHosts' // allowedHosts is optional and defaults to undefined
 > & {
   ttl: number | undefined; // needed, otherwise we could not set the value `ttl: undefined` value (due to the Required<...>)
+  renderKeyResolver: ((req: Request) => string) | undefined;
+  allowedHosts: string[] | undefined;
 } & DeepRequired<
     // all nested properties of `ssrFeatureToggles` are required too
     Pick<
@@ -274,9 +278,10 @@ export const defaultSsrOptimizationOptions: DefaultSsrOptimizationOptions = {
   ),
   logger: new DefaultExpressServerLogger(),
   shouldCacheRenderingResult: ({ entry: { err } }) => !err,
-  renderKeyResolver: getDefaultRenderKey,
+  renderKeyResolver: undefined,
   ssrFeatureToggles: {
     limitCacheByMemory: true,
   },
   useHostInCacheKey: false,
+  allowedHosts: undefined,
 };