Skip to content

Commit aecb077

Browse files
committed
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.
1 parent 490adcf commit aecb077

2 files changed

Lines changed: 14 additions & 5 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,15 @@
77
import { Request } from 'express';
88

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

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ export interface SsrOptimizationOptions {
216216
* List of allowed hosts for host-based cache keys.
217217
*
218218
* If `useHostInCacheKey` is set to true, only hosts in this list will be used in the cache key.
219-
* If the host is not in this list, it will fallback to the value of the `Host` header.
219+
* If neither `X-Forwarded-Host` nor `Host` matching this list is found, the cache key will not include a host prefix.
220220
*/
221221
allowedHosts?: string[];
222222
}
@@ -247,8 +247,11 @@ type DefaultSsrOptimizationOptions = Omit<
247247
Required<SsrOptimizationOptions>,
248248
| 'debug' // debug is deprecated and not used anymore
249249
| 'ttl' // ttl is required but its default value is `undefined`
250+
| 'renderKeyResolver' // renderKeyResolver is optional and defaults to undefined to allow for secure default logic in the engine
250251
> & {
251252
ttl: number | undefined; // needed, otherwise we could not set the value `ttl: undefined` value (due to the Required<...>)
253+
renderKeyResolver: ((req: Request) => string) | undefined;
254+
allowedHosts: string[] | undefined;
252255
} & DeepRequired<
253256
// all nested properties of `ssrFeatureToggles` are required too
254257
Pick<
@@ -274,9 +277,10 @@ export const defaultSsrOptimizationOptions: DefaultSsrOptimizationOptions = {
274277
),
275278
logger: new DefaultExpressServerLogger(),
276279
shouldCacheRenderingResult: ({ entry: { err } }) => !err,
277-
renderKeyResolver: getDefaultRenderKey,
280+
renderKeyResolver: undefined,
278281
ssrFeatureToggles: {
279282
limitCacheByMemory: true,
280283
},
281284
useHostInCacheKey: false,
285+
allowedHosts: undefined,
282286
};

0 commit comments

Comments
 (0)