Skip to content

Commit 5db6d64

Browse files
committed
fix(@angular/ssr): ensure server-side navigation triggers a redirect
When a navigation occurs on the server-side, such as using `router.navigate`, and the final URL is different from the initial URL that was requested, the server should respond with a redirect. Previously, the initial URL was being read from `router.lastSuccessfulNavigation.initialUrl`, which could be incorrect in scenarios involving server-side navigations, causing the comparison with the final URL to fail and preventing the redirect. This change ensures that the initial URL requested by the browser is used for the comparison, correctly triggering a redirect when necessary. Closes #31482 (cherry picked from commit 9c3a689)
1 parent 3a124ac commit 5db6d64

File tree

3 files changed

+57
-28
lines changed

3 files changed

+57
-28
lines changed

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { APP_BASE_HREF, PlatformLocation } from '@angular/common';
9+
import { LocationStrategy } from '@angular/common';
1010
import {
1111
ApplicationRef,
1212
type PlatformRef,
@@ -21,7 +21,7 @@ import {
2121
platformServer,
2222
ɵrenderInternal as renderInternal,
2323
} from '@angular/platform-server';
24-
import { ActivatedRoute, Router } from '@angular/router';
24+
import { ActivatedRoute, Router, UrlSerializer } from '@angular/router';
2525
import { Console } from '../console';
2626
import { joinUrlParts, stripIndexHtmlFromURL } from './url';
2727

@@ -60,12 +60,12 @@ export async function renderAngular(
6060
serverContext: string,
6161
): Promise<{ hasNavigationError: boolean; redirectTo?: string; content: () => Promise<string> }> {
6262
// A request to `http://www.example.com/page/index.html` will render the Angular route corresponding to `http://www.example.com/page`.
63-
const urlToRender = stripIndexHtmlFromURL(url).toString();
63+
const urlToRender = stripIndexHtmlFromURL(url);
6464
const platformRef = platformServer([
6565
{
6666
provide: INITIAL_CONFIG,
6767
useValue: {
68-
url: urlToRender,
68+
url: urlToRender.href,
6969
document: html,
7070
},
7171
},
@@ -110,15 +110,13 @@ export async function renderAngular(
110110
} else if (lastSuccessfulNavigation?.finalUrl) {
111111
hasNavigationError = false;
112112

113-
const { finalUrl, initialUrl } = lastSuccessfulNavigation;
114-
const finalUrlStringified = finalUrl.toString();
113+
const urlSerializer = envInjector.get(UrlSerializer);
114+
const locationStrategy = envInjector.get(LocationStrategy);
115+
const finalUrlSerialized = urlSerializer.serialize(lastSuccessfulNavigation.finalUrl);
116+
const finalExternalUrl = joinUrlParts(locationStrategy.getBaseHref(), finalUrlSerialized);
115117

116-
if (initialUrl.toString() !== finalUrlStringified) {
117-
const baseHref =
118-
envInjector.get(APP_BASE_HREF, null, { optional: true }) ??
119-
envInjector.get(PlatformLocation).getBaseHrefFromDOM();
120-
121-
redirectTo = joinUrlParts(baseHref, finalUrlStringified);
118+
if (urlToRender.href !== new URL(finalExternalUrl, urlToRender.origin).href) {
119+
redirectTo = finalExternalUrl;
122120
}
123121
}
124122

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

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,6 @@ import { RenderMode } from '../src/routes/route-config';
1919
import { setAngularAppTestingManifest } from './testing-utils';
2020

2121
function createEntryPoint(locale: string) {
22-
@Component({
23-
standalone: true,
24-
selector: `app-ssr-${locale}`,
25-
template: `SSR works ${locale.toUpperCase()}`,
26-
})
27-
class SSRComponent {}
28-
29-
@Component({
30-
standalone: true,
31-
selector: `app-ssg-${locale}`,
32-
template: `SSG works ${locale.toUpperCase()}`,
33-
})
34-
class SSGComponent {}
35-
3622
return async () => {
3723
@Component({
3824
standalone: true,

packages/angular/ssr/test/app_spec.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
import '@angular/compiler';
1212
/* eslint-enable import/no-unassigned-import */
1313

14-
import { Component } from '@angular/core';
14+
import { Component, inject } from '@angular/core';
15+
import { CanActivateFn, Router } from '@angular/router';
1516
import { AngularServerApp } from '../src/app';
1617
import { RenderMode } from '../src/routes/route-config';
1718
import { setAngularAppTestingManifest } from './testing-utils';
@@ -27,14 +28,46 @@ describe('AngularServerApp', () => {
2728
})
2829
class HomeComponent {}
2930

31+
@Component({
32+
selector: 'app-redirect',
33+
})
34+
class RedirectComponent {
35+
constructor() {
36+
void inject(Router).navigate([], {
37+
queryParams: { filter: 'test' },
38+
});
39+
}
40+
}
41+
42+
const queryParamAdderGuard: CanActivateFn = (_route, state) => {
43+
const urlTree = inject(Router).parseUrl(state.url);
44+
45+
if (urlTree.queryParamMap.has('filter')) {
46+
return true;
47+
}
48+
49+
urlTree.queryParams = {
50+
filter: 'test',
51+
};
52+
53+
return urlTree;
54+
};
55+
3056
setAngularAppTestingManifest(
3157
[
3258
{ path: 'home', component: HomeComponent },
3359
{ path: 'home-csr', component: HomeComponent },
3460
{ path: 'home-ssg', component: HomeComponent },
3561
{ path: 'page-with-headers', component: HomeComponent },
3662
{ path: 'page-with-status', component: HomeComponent },
63+
3764
{ path: 'redirect', redirectTo: 'home' },
65+
{ path: 'redirect-via-navigate', component: RedirectComponent },
66+
{
67+
path: 'redirect-via-guard',
68+
canActivate: [queryParamAdderGuard],
69+
component: HomeComponent,
70+
},
3871
{ path: 'redirect/relative', redirectTo: 'home' },
3972
{ path: 'redirect/:param/relative', redirectTo: 'home' },
4073
{ path: 'redirect/absolute', redirectTo: '/home' },
@@ -260,11 +293,23 @@ describe('AngularServerApp', () => {
260293
});
261294

262295
describe('SSR pages', () => {
263-
it('returns a 302 status and redirects to the correct location when redirectTo is a function', async () => {
296+
it('returns a 302 status and redirects to the correct location when `redirectTo` is a function', async () => {
264297
const response = await app.handle(new Request('http://localhost/redirect-to-function'));
265298
expect(response?.headers.get('location')).toBe('/home');
266299
expect(response?.status).toBe(302);
267300
});
301+
302+
it('returns a 302 status and redirects to the correct location when `router.navigate` is used', async () => {
303+
const response = await app.handle(new Request('http://localhost/redirect-via-navigate'));
304+
expect(response?.headers.get('location')).toBe('/redirect-via-navigate?filter=test');
305+
expect(response?.status).toBe(302);
306+
});
307+
308+
it('returns a 302 status and redirects to the correct location when `urlTree` is updated in a guard', async () => {
309+
const response = await app.handle(new Request('http://localhost/redirect-via-guard'));
310+
expect(response?.headers.get('location')).toBe('/redirect-via-guard?filter=test');
311+
expect(response?.status).toBe(302);
312+
});
268313
});
269314
});
270315
});

0 commit comments

Comments
 (0)