Skip to content

Commit 194b641

Browse files
authored
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.
1 parent 70c2143 commit 194b641

4 files changed

Lines changed: 88 additions & 2 deletions

File tree

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

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,21 @@ 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+
let lastIndex = parts.length - 1;
105+
while (lastIndex >= 0 && parts[lastIndex] === '') {
106+
lastIndex--;
107+
}
108+
109+
for (let i = 0; i <= lastIndex; i++) {
110+
const part = parts[i];
103111
if (part === '') {
104112
// Skip any empty parts
105113
continue;
@@ -108,6 +116,22 @@ export function joinUrlParts(...parts: string[]): string {
108116
let start = 0;
109117
let end = part.length;
110118

119+
if (i === lastIndex) {
120+
const queryIndex = part.indexOf('?');
121+
const hashIndex = part.indexOf('#');
122+
const suffixIndex =
123+
queryIndex === -1
124+
? hashIndex
125+
: hashIndex === -1
126+
? queryIndex
127+
: Math.min(queryIndex, hashIndex);
128+
129+
if (suffixIndex !== -1) {
130+
suffix = part.slice(suffixIndex);
131+
end = suffixIndex;
132+
}
133+
}
134+
111135
// Use "Pointers" to avoid intermediate slices
112136
while (start < end && part[start] === '/') {
113137
start++;
@@ -122,7 +146,7 @@ export function joinUrlParts(...parts: string[]): string {
122146
}
123147
}
124148

125-
return addLeadingSlash(normalizedParts.join('/'));
149+
return addLeadingSlash(normalizedParts.join('/')) + suffix;
126150
}
127151

128152
/**

‎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: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,21 @@ 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+
expect(joinUrlParts('/login/?next=/a/', '')).toBe('/login?next=/a/');
127+
expect(joinUrlParts('/base/', '/login/?next=/a/', '', '')).toBe('/base/login?next=/a/');
128+
expect(joinUrlParts('/login/#section/', '')).toBe('/login#section/');
129+
});
115130
});
116131

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

0 commit comments

Comments
 (0)