From 7301e70ddd3e7eab8d44cfa5b9c492b2a84c3864 Mon Sep 17 00:00:00 2001 From: Adam Date: Thu, 28 May 2020 21:33:15 -0500 Subject: [PATCH] fix(platform-server): correctly handle absolute relative URLs (#37341) Previously, we would simply prepend any relative URL with the HREF for the current route (pulled from document.location). However, this does not correctly account for the leading slash URLs that would otherwise be parsed correctly in the browser, or the presence of a base HREF in the DOM. Therefore, we use the built-in URL implementation for NodeJS, which implements the WHATWG standard that's used in the browser. We also pull the base HREF from the DOM, falling back on the full HREF as the browser would, to form the correct request URL. Fixes #37314 PR Close #37341 --- packages/platform-server/src/http.ts | 27 +++++----- packages/platform-server/src/location.ts | 1 + .../platform-server/test/integration_spec.ts | 52 +++++++++++++++++++ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts index 682da84fa0..d165be953b 100644 --- a/packages/platform-server/src/http.ts +++ b/packages/platform-server/src/http.ts @@ -10,13 +10,12 @@ const xhr2: any = require('xhr2'); import {Injectable, Injector, Provider} from '@angular/core'; -import {DOCUMENT} from '@angular/common'; +import {PlatformLocation} from '@angular/common'; import {HttpEvent, HttpRequest, HttpHandler, HttpBackend, XhrFactory, ɵHttpInterceptingHandler as HttpInterceptingHandler} from '@angular/common/http'; import {Observable, Observer, Subscription} from 'rxjs'; // @see https://www.w3.org/Protocols/HTTP/1.1/draft-ietf-http-v11-spec-01#URI-syntax const isAbsoluteUrl = /^[a-zA-Z\-\+.]+:\/\//; -const FORWARD_SLASH = '/'; @Injectable() export class ServerXhr implements XhrFactory { @@ -105,20 +104,18 @@ export abstract class ZoneMacroTaskWrapper { export class ZoneClientBackend extends ZoneMacroTaskWrapper, HttpEvent> implements HttpBackend { - constructor(private backend: HttpBackend, private doc: Document) { + constructor(private backend: HttpBackend, private platformLocation: PlatformLocation) { super(); } handle(request: HttpRequest): Observable> { - const href = this.doc.location.href; - if (!isAbsoluteUrl.test(request.url) && href) { - const urlParts = Array.from(request.url); - if (request.url[0] === FORWARD_SLASH && href[href.length - 1] === FORWARD_SLASH) { - urlParts.shift(); - } else if (request.url[0] !== FORWARD_SLASH && href[href.length - 1] !== FORWARD_SLASH) { - urlParts.splice(0, 0, FORWARD_SLASH); - } - return this.wrap(request.clone({url: href + urlParts.join('')})); + const {href, protocol, hostname} = this.platformLocation; + if (!isAbsoluteUrl.test(request.url) && href !== '/') { + const baseHref = this.platformLocation.getBaseHrefFromDOM() || href; + const urlPrefix = `${protocol}//${hostname}`; + const baseUrl = new URL(baseHref, urlPrefix); + const url = new URL(request.url, baseUrl); + return this.wrap(request.clone({url: url.toString()})); } return this.wrap(request); } @@ -129,15 +126,15 @@ export class ZoneClientBackend extends } export function zoneWrappedInterceptingHandler( - backend: HttpBackend, injector: Injector, doc: Document) { + backend: HttpBackend, injector: Injector, platformLocation: PlatformLocation) { const realBackend: HttpBackend = new HttpInterceptingHandler(backend, injector); - return new ZoneClientBackend(realBackend, doc); + return new ZoneClientBackend(realBackend, platformLocation); } export const SERVER_HTTP_PROVIDERS: Provider[] = [ {provide: XhrFactory, useClass: ServerXhr}, { provide: HttpHandler, useFactory: zoneWrappedInterceptingHandler, - deps: [HttpBackend, Injector, DOCUMENT] + deps: [HttpBackend, Injector, PlatformLocation] } ]; diff --git a/packages/platform-server/src/location.ts b/packages/platform-server/src/location.ts index 18c3c8da87..60f42c793b 100644 --- a/packages/platform-server/src/location.ts +++ b/packages/platform-server/src/location.ts @@ -51,6 +51,7 @@ export class ServerPlatformLocation implements PlatformLocation { this.pathname = parsedUrl.pathname; this.search = parsedUrl.search; this.hash = parsedUrl.hash; + this.href = _doc.location.href; } } diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index d0fbfa0f3c..db3d334e55 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -841,6 +841,58 @@ describe('platform-server integration', () => { }); }); + it('can make relative HttpClient requests no slashes longer url', async () => { + const platform = platformDynamicServer([{ + provide: INITIAL_CONFIG, + useValue: {document: '', url: 'http://localhost/path/page'} + }]); + const ref = await platform.bootstrapModule(HttpClientExampleModule); + const mock = ref.injector.get(HttpTestingController) as HttpTestingController; + const http = ref.injector.get(HttpClient); + ref.injector.get(NgZone).run(() => { + http.get('testing').subscribe((body: string) => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://localhost/path/testing').flush('success!'); + }); + }); + + it('can make relative HttpClient requests slashes longer url', async () => { + const platform = platformDynamicServer([{ + provide: INITIAL_CONFIG, + useValue: {document: '', url: 'http://localhost/path/page'} + }]); + const ref = await platform.bootstrapModule(HttpClientExampleModule); + const mock = ref.injector.get(HttpTestingController) as HttpTestingController; + const http = ref.injector.get(HttpClient); + ref.injector.get(NgZone).run(() => { + http.get('/testing').subscribe((body: string) => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://localhost/testing').flush('success!'); + }); + }); + + it('can make relative HttpClient requests slashes longer url with base href', async () => { + const platform = platformDynamicServer([{ + provide: INITIAL_CONFIG, + useValue: + {document: '', url: 'http://localhost/path/page'} + }]); + const ref = await platform.bootstrapModule(HttpClientExampleModule); + const mock = ref.injector.get(HttpTestingController) as HttpTestingController; + const http = ref.injector.get(HttpClient); + ref.injector.get(NgZone).run(() => { + http.get('/testing').subscribe((body: string) => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://other/testing').flush('success!'); + }); + }); + it('requests are macrotasks', async(() => { const platform = platformDynamicServer( [{provide: INITIAL_CONFIG, useValue: {document: ''}}]);