Skip to content

Commit a77eadd

Browse files
committed
fix(@angular/ssr): preserve trailing slashes in query and fragment of redirect URLs
In redirect URLs, trailing slashes in query parameters or hash fragments should be preserved, while the path portion before the query or fragment continues to be normalized. Fixes #34511
1 parent 70c2143 commit a77eadd

4 files changed

Lines changed: 80 additions & 2 deletions

File tree

‎packages/angular/ssr/src/utils/url.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,16 @@ export function addTrailingSlash(url: string): string {
9393
* ```js
9494
* joinUrlParts('path/', '/to/resource'); // '/path/to/resource'
9595
* joinUrlParts('/path/', 'to/resource'); // '/path/to/resource'
96+
* joinUrlParts('/base/', '/login/?next=/a/'); // '/base/login?next=/a/'
9697
* joinUrlParts('', ''); // '/'
9798
* ```
9899
*/
99100
export function joinUrlParts(...parts: string[]): string {
100101
const normalizedParts: string[] = [];
102+
let suffix = '';
101103

102-
for (const part of parts) {
104+
for (let i = 0, len = parts.length; i < len; i++) {
105+
const part = parts[i];
103106
if (part === '') {
104107
// Skip any empty parts
105108
continue;
@@ -108,6 +111,22 @@ export function joinUrlParts(...parts: string[]): string {
108111
let start = 0;
109112
let end = part.length;
110113

114+
if (i === len - 1) {
115+
const queryIndex = part.indexOf('?');
116+
const hashIndex = part.indexOf('#');
117+
const suffixIndex =
118+
queryIndex === -1
119+
? hashIndex
120+
: hashIndex === -1
121+
? queryIndex
122+
: Math.min(queryIndex, hashIndex);
123+
124+
if (suffixIndex !== -1) {
125+
suffix = part.slice(suffixIndex);
126+
end = suffixIndex;
127+
}
128+
}
129+
111130
// Use "Pointers" to avoid intermediate slices
112131
while (start < end && part[start] === '/') {
113132
start++;
@@ -122,7 +141,7 @@ export function joinUrlParts(...parts: string[]): string {
122141
}
123142
}
124143

125-
return addLeadingSlash(normalizedParts.join('/'));
144+
return addLeadingSlash(normalizedParts.join('/')) + suffix;
126145
}
127146

128147
/**

‎packages/angular/ssr/test/app_spec.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ describe('AngularServerApp', () => {
9696
path: 'redirect-query-placeholder',
9797
redirectTo: '/home?q=:term&id=:missing&static=value',
9898
},
99+
{
100+
path: 'redirect-query-trailing-slash',
101+
redirectTo: '/home?next=/a/',
102+
},
103+
{
104+
path: 'redirect-fragment-trailing-slash',
105+
redirectTo: '/home#section/',
106+
},
99107
{
100108
path: 'redirect-to-function',
101109
redirectTo: () => 'home',
@@ -282,6 +290,20 @@ describe('AngularServerApp', () => {
282290
expect(response?.status).toBe(302);
283291
});
284292

293+
it('should preserve trailing slashes in query parameters and fragments of redirect targets', async () => {
294+
const queryResponse = await app.handle(
295+
new Request('http://localhost/redirect-query-trailing-slash'),
296+
);
297+
expect(queryResponse?.headers.get('location')).toBe('/home?next=/a/');
298+
expect(queryResponse?.status).toBe(302);
299+
300+
const fragmentResponse = await app.handle(
301+
new Request('http://localhost/redirect-fragment-trailing-slash'),
302+
);
303+
expect(fragmentResponse?.headers.get('location')).toBe('/home#section/');
304+
expect(fragmentResponse?.status).toBe(302);
305+
});
306+
285307
it('should handle request abortion gracefully', async () => {
286308
const controller = new AbortController();
287309
const request = new Request('http://localhost/home', { signal: controller.signal });

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,31 @@ describe('extractRoutesAndCreateRouteTree', () => {
812812
]);
813813
});
814814

815+
it('should preserve trailing slashes in query parameters and fragments of absolute redirect targets', async () => {
816+
setAngularAppTestingManifest(
817+
[
818+
{ path: 'query-slash', redirectTo: '/login?next=/a/' },
819+
{ path: 'fragment-slash', redirectTo: '/login#section/' },
820+
{ path: 'path-and-query-slash', redirectTo: '/login/?next=/a/' },
821+
{ path: 'login', component: DummyComponent },
822+
],
823+
[{ path: '**', renderMode: RenderMode.Server }],
824+
);
825+
826+
const { routeTree, errors } = await extractRoutesAndCreateRouteTree({ url });
827+
expect(errors).toHaveSize(0);
828+
expect(routeTree.toObject()).toEqual([
829+
{ route: '/query-slash', renderMode: RenderMode.Server, redirectTo: '/login?next=/a/' },
830+
{ route: '/fragment-slash', renderMode: RenderMode.Server, redirectTo: '/login#section/' },
831+
{
832+
route: '/path-and-query-slash',
833+
renderMode: RenderMode.Server,
834+
redirectTo: '/login?next=/a/',
835+
},
836+
{ route: '/login', renderMode: RenderMode.Server },
837+
]);
838+
});
839+
815840
it('should not resolve parameterized routes for SSG when `invokeGetPrerenderParams` is false', async () => {
816841
setAngularAppTestingManifest(
817842
[

‎packages/angular/ssr/test/utils/url_spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,18 @@ describe('URL Utils', () => {
112112
it('should handle parts containing only slashes', () => {
113113
expect(joinUrlParts('//', '///')).toBe('/');
114114
});
115+
116+
it('should preserve trailing slashes in query parameters and fragments', () => {
117+
expect(joinUrlParts('/', '/login?next=/a/')).toBe('/login?next=/a/');
118+
expect(joinUrlParts('/', '/login#section/')).toBe('/login#section/');
119+
expect(joinUrlParts('/base/', '/foo/?next=/a/#section/')).toBe('/base/foo?next=/a/#section/');
120+
expect(joinUrlParts('/base/', '/login/#section?next=/a/')).toBe(
121+
'/base/login#section?next=/a/',
122+
);
123+
expect(joinUrlParts('/base/', '/?next=/a/')).toBe('/base?next=/a/');
124+
expect(joinUrlParts('/', '/?next=/a/')).toBe('/?next=/a/');
125+
expect(joinUrlParts('', '///evil.com?a=/')).toBe('/evil.com?a=/');
126+
});
115127
});
116128

117129
describe('stripIndexHtmlFromURL', () => {

0 commit comments

Comments
 (0)