From 15a082c74ed152ecac4c0ec989667e710bfcbb4e Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Tue, 21 Mar 2017 10:29:13 -0700
Subject: [PATCH] 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
---
 packages/platform-server/src/http.ts          |  9 +++++++
 .../platform-server/test/integration_spec.ts  | 26 +++++++++++++++----
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts
index 1cd67c7d45..49b07490de 100644
--- a/packages/platform-server/src/http.ts
+++ b/packages/platform-server/src/http.ts
@@ -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;
diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts
index 4ea5b4f925..1bf08703b4 100644
--- a/packages/platform-server/test/integration_spec.ts
+++ b/packages/platform-server/test/integration_spec.ts
@@ -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');
+           });
+         }));
     });
   });
 }