Skip to content

Commit 7301e70

Browse files
CaerusKaruatscott
CaerusKaru
authored and
atscott
committed
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
1 parent 7edb026 commit 7301e70

3 files changed

Lines changed: 65 additions & 15 deletions

File tree

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@
1010
const xhr2: any = require('xhr2');
1111

1212
import {Injectable, Injector, Provider} from '@angular/core';
13-
import {DOCUMENT} from '@angular/common';
13+
import {PlatformLocation} from '@angular/common';
1414
import {HttpEvent, HttpRequest, HttpHandler, HttpBackend, XhrFactory, ɵHttpInterceptingHandler as HttpInterceptingHandler} from '@angular/common/http';
1515
import {Observable, Observer, Subscription} from 'rxjs';
1616

1717
// @see https://www.w3.org/Protocols/HTTP/1.1/draft-ietf-http-v11-spec-01#URI-syntax
1818
const isAbsoluteUrl = /^[a-zA-Z\-\+.]+:\/\//;
19-
const FORWARD_SLASH = '/';
2019

2120
@Injectable()
2221
export class ServerXhr implements XhrFactory {
@@ -105,20 +104,18 @@ export abstract class ZoneMacroTaskWrapper<S, R> {
105104

106105
export class ZoneClientBackend extends
107106
ZoneMacroTaskWrapper<HttpRequest<any>, HttpEvent<any>> implements HttpBackend {
108-
constructor(private backend: HttpBackend, private doc: Document) {
107+
constructor(private backend: HttpBackend, private platformLocation: PlatformLocation) {
109108
super();
110109
}
111110

112111
handle(request: HttpRequest<any>): Observable<HttpEvent<any>> {
113-
const href = this.doc.location.href;
114-
if (!isAbsoluteUrl.test(request.url) && href) {
115-
const urlParts = Array.from(request.url);
116-
if (request.url[0] === FORWARD_SLASH && href[href.length - 1] === FORWARD_SLASH) {
117-
urlParts.shift();
118-
} else if (request.url[0] !== FORWARD_SLASH && href[href.length - 1] !== FORWARD_SLASH) {
119-
urlParts.splice(0, 0, FORWARD_SLASH);
120-
}
121-
return this.wrap(request.clone({url: href + urlParts.join('')}));
112+
const {href, protocol, hostname} = this.platformLocation;
113+
if (!isAbsoluteUrl.test(request.url) && href !== '/') {
114+
const baseHref = this.platformLocation.getBaseHrefFromDOM() || href;
115+
const urlPrefix = `${protocol}//${hostname}`;
116+
const baseUrl = new URL(baseHref, urlPrefix);
117+
const url = new URL(request.url, baseUrl);
118+
return this.wrap(request.clone({url: url.toString()}));
122119
}
123120
return this.wrap(request);
124121
}
@@ -129,15 +126,15 @@ export class ZoneClientBackend extends
129126
}
130127

131128
export function zoneWrappedInterceptingHandler(
132-
backend: HttpBackend, injector: Injector, doc: Document) {
129+
backend: HttpBackend, injector: Injector, platformLocation: PlatformLocation) {
133130
const realBackend: HttpBackend = new HttpInterceptingHandler(backend, injector);
134-
return new ZoneClientBackend(realBackend, doc);
131+
return new ZoneClientBackend(realBackend, platformLocation);
135132
}
136133

137134
export const SERVER_HTTP_PROVIDERS: Provider[] = [
138135
{provide: XhrFactory, useClass: ServerXhr}, {
139136
provide: HttpHandler,
140137
useFactory: zoneWrappedInterceptingHandler,
141-
deps: [HttpBackend, Injector, DOCUMENT]
138+
deps: [HttpBackend, Injector, PlatformLocation]
142139
}
143140
];

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export class ServerPlatformLocation implements PlatformLocation {
5151
this.pathname = parsedUrl.pathname;
5252
this.search = parsedUrl.search;
5353
this.hash = parsedUrl.hash;
54+
this.href = _doc.location.href;
5455
}
5556
}
5657

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,58 @@ describe('platform-server integration', () => {
841841
});
842842
});
843843

844+
it('can make relative HttpClient requests no slashes longer url', async () => {
845+
const platform = platformDynamicServer([{
846+
provide: INITIAL_CONFIG,
847+
useValue: {document: '<app></app>', url: 'http://localhost/path/page'}
848+
}]);
849+
const ref = await platform.bootstrapModule(HttpClientExampleModule);
850+
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
851+
const http = ref.injector.get(HttpClient);
852+
ref.injector.get(NgZone).run(() => {
853+
http.get<string>('testing').subscribe((body: string) => {
854+
NgZone.assertInAngularZone();
855+
expect(body).toEqual('success!');
856+
});
857+
mock.expectOne('http://localhost/path/testing').flush('success!');
858+
});
859+
});
860+
861+
it('can make relative HttpClient requests slashes longer url', async () => {
862+
const platform = platformDynamicServer([{
863+
provide: INITIAL_CONFIG,
864+
useValue: {document: '<app></app>', url: 'http://localhost/path/page'}
865+
}]);
866+
const ref = await platform.bootstrapModule(HttpClientExampleModule);
867+
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
868+
const http = ref.injector.get(HttpClient);
869+
ref.injector.get(NgZone).run(() => {
870+
http.get<string>('/testing').subscribe((body: string) => {
871+
NgZone.assertInAngularZone();
872+
expect(body).toEqual('success!');
873+
});
874+
mock.expectOne('http://localhost/testing').flush('success!');
875+
});
876+
});
877+
878+
it('can make relative HttpClient requests slashes longer url with base href', async () => {
879+
const platform = platformDynamicServer([{
880+
provide: INITIAL_CONFIG,
881+
useValue:
882+
{document: '<base href="http://other"><app></app>', url: 'http://localhost/path/page'}
883+
}]);
884+
const ref = await platform.bootstrapModule(HttpClientExampleModule);
885+
const mock = ref.injector.get(HttpTestingController) as HttpTestingController;
886+
const http = ref.injector.get(HttpClient);
887+
ref.injector.get(NgZone).run(() => {
888+
http.get<string>('/testing').subscribe((body: string) => {
889+
NgZone.assertInAngularZone();
890+
expect(body).toEqual('success!');
891+
});
892+
mock.expectOne('http://other/testing').flush('success!');
893+
});
894+
});
895+
844896
it('requests are macrotasks', async(() => {
845897
const platform = platformDynamicServer(
846898
[{provide: INITIAL_CONFIG, useValue: {document: '<app></app>'}}]);

0 commit comments

Comments
 (0)