Skip to content

Commit 15a082c

Browse files
alxhubmhevery
authored andcommitted
fix(platform-server): throw a better error message for relative URLs (angular#15357)
Unlike in the browser, on the server there is no concept of a document origin. Thus, it is illegal to make requests for relative URLs against Http on platform-server. Currently this fails with a vague error: Error: Uncaught (in promise): Error at resolvePromise This change adds explicit validation and a friendlier error message: Error: URLs requested via Http on the server must be absolute. URL: /testing Another option considered was to track the concept of an origin for the platform and automatically prepend it to relative URLs. This would cause automatic "local RPCs" to be made, though, which would be an unexpected and undesirable default behavior. Fixes angular#15349 PR Close angular#15357
1 parent fbccd5c commit 15a082c

2 files changed

Lines changed: 30 additions & 5 deletions

File tree

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ import {Observable} from 'rxjs/Observable';
1515
import {Observer} from 'rxjs/Observer';
1616
import {Subscription} from 'rxjs/Subscription';
1717

18+
const isAbsoluteUrl = /^[a-zA-Z\-\+.]+:\/\//;
19+
20+
function validateRequestUrl(url: string): void {
21+
if (!isAbsoluteUrl.test(url)) {
22+
throw new Error(`URLs requested via Http on the server must be absolute. URL: ${url}`);
23+
}
24+
}
25+
1826
@Injectable()
1927
export class ServerXhr implements BrowserXhr {
2028
build(): XMLHttpRequest { return new xhr2.XMLHttpRequest(); }
@@ -30,6 +38,7 @@ export class ZoneMacroTaskConnection implements Connection {
3038
lastConnection: Connection;
3139

3240
constructor(public request: Request, backend: XHRBackend) {
41+
validateRequestUrl(request.url);
3342
this.response = new Observable((observer: Observer<Response>) => {
3443
let task: Task = null;
3544
let scheduled: boolean = false;

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -426,10 +426,10 @@ export function main() {
426426
NgZone.assertInAngularZone();
427427
mock.connections.subscribe((mc: MockConnection) => {
428428
NgZone.assertInAngularZone();
429-
expect(mc.request.url).toBe('/testing');
429+
expect(mc.request.url).toBe('http://localhost/testing');
430430
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
431431
});
432-
http.get('/testing').subscribe(resp => {
432+
http.get('http://localhost/testing').subscribe(resp => {
433433
NgZone.assertInAngularZone();
434434
expect(resp.text()).toBe('success!');
435435
});
@@ -449,7 +449,9 @@ export function main() {
449449
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
450450
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
451451
});
452-
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
452+
http.get('http://localhost/testing').subscribe(resp => {
453+
expect(resp.text()).toBe('success!');
454+
});
453455
});
454456
});
455457
}));
@@ -466,7 +468,9 @@ export function main() {
466468
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
467469
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
468470
});
469-
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
471+
http.get('http://localhost/testing').subscribe(resp => {
472+
expect(resp.text()).toBe('success!');
473+
});
470474
});
471475
});
472476
}));
@@ -483,10 +487,22 @@ export function main() {
483487
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
484488
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
485489
});
486-
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
490+
http.get('http://localhost/testing').subscribe(resp => {
491+
expect(resp.text()).toBe('success!');
492+
});
487493
});
488494
});
489495
}));
496+
it('throws when given a relative URL', async(() => {
497+
const platform = platformDynamicServer(
498+
[{provide: INITIAL_CONFIG, useValue: {document: '<app></app>'}}]);
499+
platform.bootstrapModule(ExampleModule).then(ref => {
500+
const http = ref.injector.get(Http);
501+
expect(() => http.get('/testing'))
502+
.toThrowError(
503+
'URLs requested via Http on the server must be absolute. URL: /testing');
504+
});
505+
}));
490506
});
491507
});
492508
}

0 commit comments

Comments
 (0)