fix(platform-server): throw a better error message for relative URLs (#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 #15349

PR Close #15357
This commit is contained in:
Alex Rickabaugh 2017-03-21 10:29:13 -07:00 committed by Miško Hevery
parent fbccd5cd38
commit 15a082c74e
2 changed files with 30 additions and 5 deletions

View File

@ -15,6 +15,14 @@ import {Observable} from 'rxjs/Observable';
import {Observer} from 'rxjs/Observer';
import {Subscription} from 'rxjs/Subscription';
const isAbsoluteUrl = /^[a-zA-Z\-\+.]+:\/\//;
function validateRequestUrl(url: string): void {
if (!isAbsoluteUrl.test(url)) {
throw new Error(`URLs requested via Http on the server must be absolute. URL: ${url}`);
}
}
@Injectable()
export class ServerXhr implements BrowserXhr {
build(): XMLHttpRequest { return new xhr2.XMLHttpRequest(); }
@ -30,6 +38,7 @@ export class ZoneMacroTaskConnection implements Connection {
lastConnection: Connection;
constructor(public request: Request, backend: XHRBackend) {
validateRequestUrl(request.url);
this.response = new Observable((observer: Observer<Response>) => {
let task: Task = null;
let scheduled: boolean = false;

View File

@ -426,10 +426,10 @@ export function main() {
NgZone.assertInAngularZone();
mock.connections.subscribe((mc: MockConnection) => {
NgZone.assertInAngularZone();
expect(mc.request.url).toBe('/testing');
expect(mc.request.url).toBe('http://localhost/testing');
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
});
http.get('/testing').subscribe(resp => {
http.get('http://localhost/testing').subscribe(resp => {
NgZone.assertInAngularZone();
expect(resp.text()).toBe('success!');
});
@ -449,7 +449,9 @@ export function main() {
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
});
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
http.get('http://localhost/testing').subscribe(resp => {
expect(resp.text()).toBe('success!');
});
});
});
}));
@ -466,7 +468,9 @@ export function main() {
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
});
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
http.get('http://localhost/testing').subscribe(resp => {
expect(resp.text()).toBe('success!');
});
});
});
}));
@ -483,10 +487,22 @@ export function main() {
expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy();
mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200})));
});
http.get('/testing').subscribe(resp => { expect(resp.text()).toBe('success!'); });
http.get('http://localhost/testing').subscribe(resp => {
expect(resp.text()).toBe('success!');
});
});
});
}));
it('throws when given a relative URL', async(() => {
const platform = platformDynamicServer(
[{provide: INITIAL_CONFIG, useValue: {document: '<app></app>'}}]);
platform.bootstrapModule(ExampleModule).then(ref => {
const http = ref.injector.get(Http);
expect(() => http.get('/testing'))
.toThrowError(
'URLs requested via Http on the server must be absolute. URL: /testing');
});
}));
});
});
}