Visitar URL original
fix(platform-server): avoid stripping unicode whitespace during url r… · angular/angular@5aa6d97 · GitHub
Skip to content

Commit 5aa6d97

Browse files
alan-agius4leonsenft
authored andcommitted
fix(platform-server): avoid stripping unicode whitespace during url resolution
Avoid trimming urlStr with String.prototype.trim() in resolveUrl to ensure URL parsing and resolution align with the WHATWG URL standard.
1 parent 73d8bbd commit 5aa6d97

3 files changed

Lines changed: 30 additions & 4 deletions

File tree

‎packages/platform-server/src/url.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ export function resolveUrl(
5757
return originUrl || null;
5858
}
5959

60-
urlStr = urlStr.trim();
61-
6260
// Fast-path: if the URL is a valid, standard absolute URL, parse and return it immediately.
6361
let resolved: URL | undefined;
6462
try {

‎packages/platform-server/test/integration_spec.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,6 +1583,17 @@ class HiddenModule {}
15831583
});
15841584
});
15851585

1586+
it('should resolve non-breaking space prefixed URLs as relative paths on the same origin', async () => {
1587+
ref.injector.get(NgZone).run(() => {
1588+
http.get('\u00A0//attacker.example/collect').subscribe((body) => {
1589+
expect(body).toEqual('success!');
1590+
});
1591+
mock
1592+
.expectOne('http://localhost:4000/%C2%A0//attacker.example/collect')
1593+
.flush('success!');
1594+
});
1595+
});
1596+
15861597
it('should reject backslash bypass SSRF attempts in relative requests and throw a suspicious origin error', async () => {
15871598
const badUrls = [
15881599
'/\\attacker.com',
@@ -1597,7 +1608,7 @@ class HiddenModule {}
15971608
next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`),
15981609
error: (err) => {
15991610
expect(err.message).toBe(
1600-
`NG05703: URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
1611+
`NG05703: URL ${badUrl} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
16011612
);
16021613
},
16031614
});
@@ -1620,7 +1631,7 @@ class HiddenModule {}
16201631
next: () => fail(`Expected request for ${badUrl} to fail, but it succeeded.`),
16211632
error: (err) => {
16221633
expect(err.message).toBe(
1623-
`NG05703: URL ${badUrl.trim()} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
1634+
`NG05703: URL ${badUrl} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
16241635
);
16251636
},
16261637
});

‎packages/platform-server/test/url_spec.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,22 @@ describe('resolveUrl', () => {
7474
`NG05703: URL ${url} changed origin unexpectedly. This is suspicious and may indicate a security bypass attempt.`,
7575
);
7676
});
77+
78+
it('should not trim unicode whitespace into protocol-relative URLs', () => {
79+
const urls = ['\u00A0//attacker.example/collect', '\uFEFF//attacker.example/collect'];
80+
81+
for (const urlStr of urls) {
82+
const urlWithProtocolRelative = resolveUrl(urlStr, 'http://test.com', {
83+
allowProtocolRelative: true,
84+
});
85+
expect(urlWithProtocolRelative.origin).toBe('http://test.com');
86+
expect(urlWithProtocolRelative.pathname).toContain('//attacker.example/collect');
87+
88+
const urlWithoutProtocolRelative = resolveUrl(urlStr, 'http://test.com');
89+
expect(urlWithoutProtocolRelative.origin).toBe('http://test.com');
90+
expect(urlWithoutProtocolRelative.pathname).toContain('//attacker.example/collect');
91+
}
92+
});
7793
});
7894

7995
describe('without origin', () => {
@@ -82,6 +98,7 @@ describe('resolveUrl', () => {
8298
expect(resolveUrl('deep/path')).toBeNull();
8399
expect(resolveUrl('/\\attacker.com/deep/path')).toBeNull();
84100
expect(resolveUrl('\\\\attacker.com/deep/path')).toBeNull();
101+
expect(resolveUrl('\u00A0//attacker.com/deep/path')).toBeNull();
85102
});
86103

87104
it('should parse valid absolute URLs', () => {

0 commit comments

Comments
 (0)