Skip to content

Commit 53f59db

Browse files
committed
fix(@angular/ssr): improve header validation logic
Updates the `validateRequest` function to correctly handle the `allowedHost=true` option.
1 parent 76f09aa commit 53f59db

File tree

6 files changed

+130
-18
lines changed

6 files changed

+130
-18
lines changed

goldens/public-api/angular/ssr/index.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export class AngularAppEngine {
1414
constructor(options?: AngularAppEngineOptions);
1515
handle(request: Request, requestContext?: unknown): Promise<Response | null>;
1616
static ɵallowStaticRouteRender: boolean;
17+
static ɵdisableAllowedHostsCheck: boolean;
1718
static ɵhooks: Hooks;
1819
}
1920

packages/angular/build/src/tools/vite/middlewares/ssr-middleware.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ export async function createAngularSsrExternalMiddleware(
9090
'@angular/ssr/node' as string
9191
)) as typeof import('@angular/ssr/node', { with: { 'resolution-mode': 'import' } });
9292

93+
// Disable host check if the server is listening on all interfaces or if allowed hosts is true meaning allow all hosts.
94+
const { host, allowedHosts } = server.config.server;
95+
const disableAllowedHostsCheck = allowedHosts === true || host === '0.0.0.0';
96+
9397
return function angularSsrExternalMiddleware(
9498
req: Connect.IncomingMessage,
9599
res: ServerResponse,
@@ -123,6 +127,7 @@ export async function createAngularSsrExternalMiddleware(
123127
}
124128

125129
if (cachedAngularAppEngine !== AngularAppEngine) {
130+
AngularAppEngine.ɵdisableAllowedHostsCheck = disableAllowedHostsCheck;
126131
AngularAppEngine.ɵallowStaticRouteRender = true;
127132
AngularAppEngine.ɵhooks.on('html:transform:pre', async ({ html, url }) => {
128133
const processedHtml = await server.transformIndexHtml(url.pathname, html);

packages/angular/ssr/src/app-engine.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ export class AngularAppEngine {
4242
*/
4343
static ɵallowStaticRouteRender = false;
4444

45+
/**
46+
* A flag to enable or disable the allowed hosts check.
47+
*
48+
* Typically used during development to avoid the allowed hosts check.
49+
*
50+
* @private
51+
*/
52+
static ɵdisableAllowedHostsCheck = false;
53+
4554
/**
4655
* Hooks for extending or modifying the behavior of the server application.
4756
* These hooks are used by the Angular CLI when running the development server and
@@ -106,23 +115,33 @@ export class AngularAppEngine {
106115
*/
107116
async handle(request: Request, requestContext?: unknown): Promise<Response | null> {
108117
const allowedHost = this.allowedHosts;
118+
const disableAllowedHostsCheck = AngularAppEngine.ɵdisableAllowedHostsCheck;
109119

110120
try {
111-
validateRequest(request, allowedHost);
121+
validateRequest(request, allowedHost, disableAllowedHostsCheck);
112122
} catch (error) {
113123
return this.handleValidationError(error as Error, request);
114124
}
115125

116126
// Clone request with patched headers to prevent unallowed host header access.
117-
const { request: securedRequest, onError: onHeaderValidationError } =
118-
cloneRequestAndPatchHeaders(request, allowedHost);
127+
const { request: securedRequest, onError: onHeaderValidationError } = disableAllowedHostsCheck
128+
? { request, onError: null }
129+
: cloneRequestAndPatchHeaders(request, allowedHost);
119130

120131
const serverApp = await this.getAngularServerAppForRequest(securedRequest);
121132
if (serverApp) {
122-
return Promise.race([
123-
onHeaderValidationError.then((error) => this.handleValidationError(error, securedRequest)),
124-
serverApp.handle(securedRequest, requestContext),
125-
]);
133+
const promises: Promise<Response | null>[] = [];
134+
if (onHeaderValidationError) {
135+
promises.push(
136+
onHeaderValidationError.then((error) =>
137+
this.handleValidationError(error, securedRequest),
138+
),
139+
);
140+
}
141+
142+
promises.push(serverApp.handle(securedRequest, requestContext));
143+
144+
return Promise.race(promises);
126145
}
127146

128147
if (this.supportedLocales.length > 1) {

packages/angular/ssr/src/utils/validation.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,19 @@ export function getFirstHeaderValue(
5656
*
5757
* @param request - The incoming `Request` object to validate.
5858
* @param allowedHosts - A set of allowed hostnames.
59+
* @param disableHostCheck - Whether to disable the host check.
5960
* @throws Error if any of the validated headers contain invalid values.
6061
*/
61-
export function validateRequest(request: Request, allowedHosts: ReadonlySet<string>): void {
62+
export function validateRequest(
63+
request: Request,
64+
allowedHosts: ReadonlySet<string>,
65+
disableHostCheck: boolean,
66+
): void {
6267
validateHeaders(request);
63-
validateUrl(new URL(request.url), allowedHosts);
68+
69+
if (!disableHostCheck) {
70+
validateUrl(new URL(request.url), allowedHosts);
71+
}
6472
}
6573

6674
/**

packages/angular/ssr/test/app-engine_spec.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,4 +426,61 @@ describe('AngularAppEngine', () => {
426426
});
427427
});
428428
});
429+
430+
describe('Disable host check', () => {
431+
let consoleErrorSpy: jasmine.Spy;
432+
433+
beforeAll(() => {
434+
setAngularAppEngineManifest({
435+
allowedHosts: ['example.com'],
436+
entryPoints: {
437+
'': async () => {
438+
setAngularAppTestingManifest(
439+
[{ path: 'home', component: TestHomeComponent }],
440+
[{ path: '**', renderMode: RenderMode.Server }],
441+
);
442+
443+
return {
444+
ɵgetOrCreateAngularServerApp: getOrCreateAngularServerApp,
445+
ɵdestroyAngularServerApp: destroyAngularServerApp,
446+
};
447+
},
448+
},
449+
basePath: '/',
450+
supportedLocales: { 'en-US': '' },
451+
});
452+
453+
appEngine = new AngularAppEngine();
454+
455+
AngularAppEngine.ɵdisableAllowedHostsCheck = true;
456+
});
457+
458+
afterAll(() => {
459+
AngularAppEngine.ɵdisableAllowedHostsCheck = false;
460+
});
461+
462+
beforeEach(() => {
463+
consoleErrorSpy = spyOn(console, 'error');
464+
});
465+
466+
it('should allow requests to disallowed hosts', async () => {
467+
const request = new Request('https://evil.com/home');
468+
const response = await appEngine.handle(request);
469+
expect(response).toBeDefined();
470+
expect(response?.status).toBe(200);
471+
expect(await response?.text()).toContain('Home works');
472+
expect(consoleErrorSpy).not.toHaveBeenCalled();
473+
});
474+
475+
it('should allow requests with disallowed host header', async () => {
476+
const request = new Request('https://example.com/home', {
477+
headers: { 'host': 'evil.com' },
478+
});
479+
const response = await appEngine.handle(request);
480+
expect(response).toBeDefined();
481+
expect(response?.status).toBe(200);
482+
expect(await response?.text()).toContain('Home works');
483+
expect(consoleErrorSpy).not.toHaveBeenCalled();
484+
});
485+
});
429486
});

packages/angular/ssr/test/utils/validation_spec.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,19 @@ describe('Validation Utils', () => {
7777
},
7878
});
7979

80-
expect(() => validateRequest(req, allowedHosts)).not.toThrow();
80+
expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
81+
});
82+
83+
it('should pass for valid request when disableHostCheck is true', () => {
84+
const req = new Request('http://evil.com');
85+
86+
expect(() => validateRequest(req, allowedHosts, true)).not.toThrow();
8187
});
8288

8389
it('should throw if URL hostname is invalid', () => {
8490
const req = new Request('http://evil.com');
8591

86-
expect(() => validateRequest(req, allowedHosts)).toThrowError(
92+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
8793
/URL with hostname "evil.com" is not allowed/,
8894
);
8995
});
@@ -93,7 +99,7 @@ describe('Validation Utils', () => {
9399
headers: { 'x-forwarded-port': 'abc' },
94100
});
95101

96-
expect(() => validateRequest(req, allowedHosts)).toThrowError(
102+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
97103
'Header "x-forwarded-port" must be a numeric value.',
98104
);
99105
});
@@ -102,16 +108,32 @@ describe('Validation Utils', () => {
102108
const req = new Request('http://example.com', {
103109
headers: { 'x-forwarded-proto': 'ftp' },
104110
});
105-
expect(() => validateRequest(req, allowedHosts)).toThrowError(
111+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
106112
'Header "x-forwarded-proto" must be either "http" or "https".',
107113
);
108114
});
109115

116+
it('should pass for valid x-forwarded-proto (case-insensitive)', () => {
117+
const req = new Request('http://example.com', {
118+
headers: { 'x-forwarded-proto': 'HTTP' },
119+
});
120+
expect(() => validateRequest(req, allowedHosts, false)).not.toThrow();
121+
});
122+
110123
it('should throw if host contains path separators', () => {
111124
const req = new Request('http://example.com', {
112125
headers: { 'host': 'example.com/bad' },
113126
});
114-
expect(() => validateRequest(req, allowedHosts)).toThrowError(
127+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
128+
'Header "host" contains characters that are not allowed.',
129+
);
130+
});
131+
132+
it('should throw if host contains invalid characters', () => {
133+
const req = new Request('http://example.com', {
134+
headers: { 'host': 'example.com?query=1' },
135+
});
136+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
115137
'Header "host" contains characters that are not allowed.',
116138
);
117139
});
@@ -120,7 +142,7 @@ describe('Validation Utils', () => {
120142
const req = new Request('http://example.com', {
121143
headers: { 'x-forwarded-host': 'example.com/bad' },
122144
});
123-
expect(() => validateRequest(req, allowedHosts)).toThrowError(
145+
expect(() => validateRequest(req, allowedHosts, false)).toThrowError(
124146
'Header "x-forwarded-host" contains characters that are not allowed.',
125147
);
126148
});
@@ -135,7 +157,7 @@ describe('Validation Utils', () => {
135157
},
136158
});
137159

138-
expect(() => validateRequest(request, allowedHosts))
160+
expect(() => validateRequest(request, allowedHosts, false))
139161
.withContext(`Prefix: "${prefix}"`)
140162
.toThrowError(
141163
'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
@@ -168,7 +190,7 @@ describe('Validation Utils', () => {
168190
},
169191
});
170192

171-
expect(() => validateRequest(request, allowedHosts))
193+
expect(() => validateRequest(request, allowedHosts, false))
172194
.withContext(`Prefix: "${prefix}"`)
173195
.toThrowError(
174196
'Header "x-forwarded-prefix" must not start with multiple "/" or "\\" or contain ".", ".." path segments.',
@@ -186,7 +208,7 @@ describe('Validation Utils', () => {
186208
},
187209
});
188210

189-
expect(() => validateRequest(request, allowedHosts))
211+
expect(() => validateRequest(request, allowedHosts, false))
190212
.withContext(`Prefix: "${prefix}"`)
191213
.not.toThrow();
192214
}

0 commit comments

Comments
 (0)