Skip to content

Commit 2d11e8e

Browse files
committed
fix(@angular/ssr): return 302 when redirectTo is a function
Ensure the server returns a 302 status code and sets the correct 'Location' header when the redirectTo value is a function, aligning with expected HTTP redirect behavior.
1 parent cf31d19 commit 2d11e8e

File tree

5 files changed

+110
-34
lines changed

5 files changed

+110
-34
lines changed

packages/angular/ssr/src/app.ts

+28-11
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,7 @@ export class AngularServerApp {
174174

175175
const { redirectTo, status, renderMode } = matchedRoute;
176176
if (redirectTo !== undefined) {
177-
return new Response(null, {
178-
// Note: The status code is validated during route extraction.
179-
// 302 Found is used by default for redirections
180-
// See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
181-
status: status ?? 302,
182-
headers: {
183-
'Location': buildPathWithParams(redirectTo, url.pathname),
184-
},
185-
});
177+
return createRedirectResponse(buildPathWithParams(redirectTo, url.pathname), status);
186178
}
187179

188180
if (renderMode === RenderMode.Prerender) {
@@ -324,20 +316,28 @@ export class AngularServerApp {
324316
let html = await assets.getIndexServerHtml().text();
325317
html = await this.runTransformsOnHtml(html, url, preload);
326318

327-
const { content } = await renderAngular(
319+
const result = await renderAngular(
328320
html,
329321
this.boostrap,
330322
url,
331323
platformProviders,
332324
SERVER_CONTEXT_VALUE[renderMode],
333325
);
334326

327+
if (result.hasNavigationError) {
328+
return null;
329+
}
330+
331+
if (result.redirectTo) {
332+
return createRedirectResponse(result.redirectTo, status);
333+
}
334+
335335
const { inlineCriticalCssProcessor, criticalCssLRUCache, textDecoder } = this;
336336

337337
// Use a stream to send the response before finishing rendering and inling critical CSS, improving performance via header flushing.
338338
const stream = new ReadableStream({
339339
async start(controller) {
340-
const renderedHtml = await content();
340+
const renderedHtml = await result.content();
341341

342342
if (!inlineCriticalCssProcessor) {
343343
controller.enqueue(textDecoder.encode(renderedHtml));
@@ -484,3 +484,20 @@ function appendPreloadHintsToHtml(html: string, preload: readonly string[]): str
484484
html.slice(bodyCloseIdx),
485485
].join('\n');
486486
}
487+
488+
/**
489+
* Creates an HTTP redirect response with a specified location and status code.
490+
*
491+
* @param location - The URL to which the response should redirect.
492+
* @param status - The HTTP status code for the redirection. Defaults to 302 (Found).
493+
* See: https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static#status
494+
* @returns A `Response` object representing the HTTP redirect.
495+
*/
496+
function createRedirectResponse(location: string, status = 302): Response {
497+
return new Response(null, {
498+
status,
499+
headers: {
500+
'Location': location,
501+
},
502+
});
503+
}

packages/angular/ssr/src/routes/ng-routes.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -160,15 +160,20 @@ async function* handleRoute(options: {
160160
invokeGetPrerenderParams,
161161
includePrerenderFallbackRoutes,
162162
);
163-
} else if (typeof redirectTo === 'string') {
163+
} else if (redirectTo !== undefined) {
164164
if (metadata.status && !VALID_REDIRECT_RESPONSE_CODES.has(metadata.status)) {
165165
yield {
166166
error:
167167
`The '${metadata.status}' status code is not a valid redirect response code. ` +
168168
`Please use one of the following redirect response codes: ${[...VALID_REDIRECT_RESPONSE_CODES.values()].join(', ')}.`,
169169
};
170+
} else if (typeof redirectTo === 'string') {
171+
yield {
172+
...metadata,
173+
redirectTo: resolveRedirectTo(metadata.route, redirectTo),
174+
};
170175
} else {
171-
yield { ...metadata, redirectTo: resolveRedirectTo(metadata.route, redirectTo) };
176+
yield metadata;
172177
}
173178
} else {
174179
yield metadata;

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

+47-16
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import { APP_BASE_HREF, PlatformLocation } from '@angular/common';
910
import {
1011
ApplicationRef,
1112
type PlatformRef,
@@ -19,8 +20,9 @@ import {
1920
platformServer,
2021
ɵrenderInternal as renderInternal,
2122
} from '@angular/platform-server';
23+
import { Router } from '@angular/router';
2224
import { Console } from '../console';
23-
import { stripIndexHtmlFromURL } from './url';
25+
import { joinUrlParts, stripIndexHtmlFromURL } from './url';
2426

2527
/**
2628
* Represents the bootstrap mechanism for an Angular application.
@@ -35,28 +37,25 @@ export type AngularBootstrap = Type<unknown> | (() => Promise<ApplicationRef>);
3537
* Renders an Angular application or module to an HTML string.
3638
*
3739
* This function determines whether the provided `bootstrap` value is an Angular module
38-
* or a bootstrap function and calls the appropriate rendering method (`renderModule` or
39-
* `renderApplication`) based on that determination.
40+
* or a bootstrap function and invokes the appropriate rendering method (`renderModule` or `renderApplication`).
4041
*
41-
* @param html - The HTML string to be used as the initial document content.
42-
* @param bootstrap - Either an Angular module type or a function that returns a promise
43-
* resolving to an `ApplicationRef`.
44-
* @param url - The URL of the application. This is used for server-side rendering to
45-
* correctly handle route-based rendering.
46-
* @param platformProviders - An array of platform providers to be used during the
47-
* rendering process.
48-
* @param serverContext - A string representing the server context, used to provide additional
49-
* context or metadata during server-side rendering.
50-
* @returns A promise resolving to an object containing a `content` method, which returns a
51-
* promise that resolves to the rendered HTML string.
42+
* @param html - The initial HTML document content.
43+
* @param bootstrap - An Angular module type or a function returning a promise that resolves to an `ApplicationRef`.
44+
* @param url - The application URL, used for route-based rendering in SSR.
45+
* @param platformProviders - An array of platform providers for the rendering process.
46+
* @param serverContext - A string representing the server context, providing additional metadata for SSR.
47+
* @returns A promise resolving to an object containing:
48+
* - `hasNavigationError`: Indicates if a navigation error occurred.
49+
* - `redirectTo`: (Optional) The redirect URL if a navigation redirect occurred.
50+
* - `content`: A function returning a promise that resolves to the rendered HTML string.
5251
*/
5352
export async function renderAngular(
5453
html: string,
5554
bootstrap: AngularBootstrap,
5655
url: URL,
5756
platformProviders: StaticProvider[],
5857
serverContext: string,
59-
): Promise<{ content: () => Promise<string> }> {
58+
): Promise<{ hasNavigationError: boolean; redirectTo?: string; content: () => Promise<string> }> {
6059
// A request to `http://www.example.com/page/index.html` will render the Angular route corresponding to `http://www.example.com/page`.
6160
const urlToRender = stripIndexHtmlFromURL(url).toString();
6261
const platformRef = platformServer([
@@ -82,6 +81,9 @@ export async function renderAngular(
8281
...platformProviders,
8382
]);
8483

84+
let redirectTo: string | undefined;
85+
let hasNavigationError = true;
86+
8587
try {
8688
let applicationRef: ApplicationRef;
8789
if (isNgModule(bootstrap)) {
@@ -94,7 +96,29 @@ export async function renderAngular(
9496
// Block until application is stable.
9597
await applicationRef.whenStable();
9698

99+
// TODO(alanagius): Find a way to avoid rendering here especially for redirects as any output will be discarded.
100+
const envInjector = applicationRef.injector;
101+
const router = envInjector.get(Router);
102+
const lastSuccessfulNavigation = router.lastSuccessfulNavigation;
103+
104+
if (lastSuccessfulNavigation?.finalUrl) {
105+
hasNavigationError = false;
106+
107+
const { finalUrl, initialUrl } = lastSuccessfulNavigation;
108+
const finalUrlStringified = finalUrl.toString();
109+
110+
if (initialUrl.toString() !== finalUrlStringified) {
111+
const baseHref =
112+
envInjector.get(APP_BASE_HREF, null, { optional: true }) ??
113+
envInjector.get(PlatformLocation).getBaseHrefFromDOM();
114+
115+
redirectTo = joinUrlParts(baseHref, finalUrlStringified);
116+
}
117+
}
118+
97119
return {
120+
hasNavigationError,
121+
redirectTo,
98122
content: () =>
99123
new Promise<string>((resolve, reject) => {
100124
// Defer rendering to the next event loop iteration to avoid blocking, as most operations in `renderInternal` are synchronous.
@@ -110,6 +134,10 @@ export async function renderAngular(
110134
await asyncDestroyPlatform(platformRef);
111135

112136
throw error;
137+
} finally {
138+
if (hasNavigationError || redirectTo) {
139+
void asyncDestroyPlatform(platformRef);
140+
}
113141
}
114142
}
115143

@@ -134,7 +162,10 @@ export function isNgModule(value: AngularBootstrap): value is Type<unknown> {
134162
function asyncDestroyPlatform(platformRef: PlatformRef): Promise<void> {
135163
return new Promise((resolve) => {
136164
setTimeout(() => {
137-
platformRef.destroy();
165+
if (!platformRef.destroyed) {
166+
platformRef.destroy();
167+
}
168+
138169
resolve();
139170
}, 0);
140171
});

packages/angular/ssr/test/app_spec.ts

+17-4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ describe('AngularServerApp', () => {
3838
{ path: 'redirect/relative', redirectTo: 'home' },
3939
{ path: 'redirect/:param/relative', redirectTo: 'home' },
4040
{ path: 'redirect/absolute', redirectTo: '/home' },
41+
{
42+
path: 'redirect-to-function',
43+
redirectTo: () => 'home',
44+
pathMatch: 'full',
45+
},
4146
],
4247
[
4348
{
@@ -106,25 +111,25 @@ describe('AngularServerApp', () => {
106111

107112
it('should correctly handle top level redirects', async () => {
108113
const response = await app.handle(new Request('http://localhost/redirect'));
109-
expect(response?.headers.get('location')).toContain('/home');
114+
expect(response?.headers.get('location')).toBe('/home');
110115
expect(response?.status).toBe(302);
111116
});
112117

113118
it('should correctly handle relative nested redirects', async () => {
114119
const response = await app.handle(new Request('http://localhost/redirect/relative'));
115-
expect(response?.headers.get('location')).toContain('/redirect/home');
120+
expect(response?.headers.get('location')).toBe('/redirect/home');
116121
expect(response?.status).toBe(302);
117122
});
118123

119124
it('should correctly handle relative nested redirects with parameter', async () => {
120125
const response = await app.handle(new Request('http://localhost/redirect/param/relative'));
121-
expect(response?.headers.get('location')).toContain('/redirect/param/home');
126+
expect(response?.headers.get('location')).toBe('/redirect/param/home');
122127
expect(response?.status).toBe(302);
123128
});
124129

125130
it('should correctly handle absolute nested redirects', async () => {
126131
const response = await app.handle(new Request('http://localhost/redirect/absolute'));
127-
expect(response?.headers.get('location')).toContain('/home');
132+
expect(response?.headers.get('location')).toBe('/home');
128133
expect(response?.status).toBe(302);
129134
});
130135

@@ -253,5 +258,13 @@ describe('AngularServerApp', () => {
253258
expect(response).toBeNull();
254259
});
255260
});
261+
262+
describe('SSR pages', () => {
263+
it('returns a 302 status and redirects to the correct location when redirectTo is a function', async () => {
264+
const response = await app.handle(new Request('http://localhost/redirect-to-function'));
265+
expect(response?.headers.get('location')).toBe('/home');
266+
expect(response?.status).toBe(302);
267+
});
268+
});
256269
});
257270
});

packages/angular/ssr/test/routes/ng-routes_spec.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ describe('extractRoutesAndCreateRouteTree', () => {
5353
});
5454

5555
describe('route configuration validation', () => {
56+
it('should error when a redirect uses an invalid status code', async () => {
57+
setAngularAppTestingManifest(
58+
[{ path: '', redirectTo: () => 'home', pathMatch: 'full' }],
59+
[{ path: '', renderMode: RenderMode.Server, status: 404 }],
60+
);
61+
62+
const { errors } = await extractRoutesAndCreateRouteTree({ url });
63+
expect(errors[0]).toContain(`The '404' status code is not a valid redirect response code.`);
64+
});
65+
5666
it(`should error when a route starts with a '/'`, async () => {
5767
setAngularAppTestingManifest(
5868
[{ path: 'home', component: DummyComponent }],
@@ -88,7 +98,7 @@ describe('extractRoutesAndCreateRouteTree', () => {
8898
);
8999
});
90100

91-
it(`should not error when a catch-all route didn't match any Angular route.`, async () => {
101+
it(`should not error when a catch-all route didn't match any Angular route`, async () => {
92102
setAngularAppTestingManifest(
93103
[{ path: 'home', component: DummyComponent }],
94104
[

0 commit comments

Comments
 (0)