From 0c66e2424c84f92765c727a98f8d2199f4b1a809 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Mon, 23 Oct 2023 12:58:03 +0000 Subject: [PATCH] fix(platform-server): resolve relative requests URL (#52326) Prior to this commit relative HTTP requests were not being resolved to absolute even thought the behaviour is documented in https://angular.io/guide/universal#using-absolute-urls-for-http-data-requests-on-the-server. This caused relative HTTP requests to fail when done on the server because of missing request context. This change is also required to eventually support HTTP requests handled during prerendering (SSG). Closes #51626 PR Close #52326 --- .../projects/ngmodule/server.ts | 7 +++- .../http-transfer-state.component.ts | 2 +- .../projects/standalone/server.ts | 6 ++- .../http-transfer-state.component.ts | 2 +- packages/common/http/public_api.ts | 3 ++ packages/common/http/src/private_export.ts | 9 +++++ packages/platform-server/src/http.ts | 30 +++++++++++++- .../platform-server/test/integration_spec.ts | 39 ++++++++++++++++++- 8 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 packages/common/http/src/private_export.ts diff --git a/integration/platform-server/projects/ngmodule/server.ts b/integration/platform-server/projects/ngmodule/server.ts index e908b269c8e7b..6201fb1d19670 100644 --- a/integration/platform-server/projects/ngmodule/server.ts +++ b/integration/platform-server/projects/ngmodule/server.ts @@ -38,15 +38,18 @@ app.get('/api-2', (req, res) => { // All regular routes use the Universal engine app.get('*', (req, res) => { + const { protocol, originalUrl, baseUrl, headers } = req; + renderModule(AppServerModule, { document: indexHtml, - url: req.url, - extraProviders: [{provide: APP_BASE_HREF, useValue: req.baseUrl}], + url: `${protocol}://${headers.host}${originalUrl}`, + extraProviders: [{provide: APP_BASE_HREF, useValue: baseUrl}], }).then((response: string) => { res.send(response); }); }); + app.listen(4206, () => { console.log('Server listening on port 4206!'); }); diff --git a/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy/http-transfer-state.component.ts b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy/http-transfer-state.component.ts index a4a04a74b2b4e..76b1e62ce4f7c 100644 --- a/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy/http-transfer-state.component.ts +++ b/integration/platform-server/projects/ngmodule/src/app/http-transferstate-lazy/http-transfer-state.component.ts @@ -29,7 +29,7 @@ export class TransferStateComponent implements OnInit { ngOnInit(): void { // Test that HTTP cache works when HTTP call is made in a lifecycle hook. - this.httpClient.get('http://localhost:4206/api-2').subscribe((response) => { + this.httpClient.get('/api-2').subscribe((response) => { this.responseTwo = response.data; }); } diff --git a/integration/platform-server/projects/standalone/server.ts b/integration/platform-server/projects/standalone/server.ts index ac1b37df48fb9..572d0c3d67b89 100644 --- a/integration/platform-server/projects/standalone/server.ts +++ b/integration/platform-server/projects/standalone/server.ts @@ -38,10 +38,12 @@ app.get('/api-2', (req, res) => { // All regular routes use the Universal engine app.get('*', (req, res) => { + const { protocol, originalUrl, baseUrl, headers } = req; + renderApplication(bootstrap, { document: indexHtml, - url: req.url, - platformProviders: [{provide: APP_BASE_HREF, useValue: req.baseUrl}], + url: `${protocol}://${headers.host}${originalUrl}`, + platformProviders: [{provide: APP_BASE_HREF, useValue: baseUrl}], }).then((response: string) => { res.send(response); }); diff --git a/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts b/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts index dfaa84a5138f7..eda820d63ccc9 100644 --- a/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts +++ b/integration/platform-server/projects/standalone/src/app/http-transferstate-lazy/http-transfer-state.component.ts @@ -31,7 +31,7 @@ export class TransferStateComponent implements OnInit { ngOnInit(): void { // Test that HTTP cache works when HTTP call is made in a lifecycle hook. - this.httpClient.get('http://localhost:4206/api-2').subscribe((response) => { + this.httpClient.get('/api-2').subscribe((response) => { this.responseTwo = response.data; }); } diff --git a/packages/common/http/public_api.ts b/packages/common/http/public_api.ts index 4a0a4c30ca52d..277680b844510 100644 --- a/packages/common/http/public_api.ts +++ b/packages/common/http/public_api.ts @@ -21,3 +21,6 @@ export {HttpDownloadProgressEvent, HttpErrorResponse, HttpEvent, HttpEventType, export {HttpTransferCacheOptions, withHttpTransferCache as ɵwithHttpTransferCache} from './src/transfer_cache'; export {HttpXhrBackend} from './src/xhr'; export {HttpXsrfTokenExtractor} from './src/xsrf'; + +// Private exports +export * from './src/private_export'; diff --git a/packages/common/http/src/private_export.ts b/packages/common/http/src/private_export.ts new file mode 100644 index 0000000000000..d5b0ff7cb9f87 --- /dev/null +++ b/packages/common/http/src/private_export.ts @@ -0,0 +1,9 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +export {HTTP_ROOT_INTERCEPTOR_FNS as ɵHTTP_ROOT_INTERCEPTOR_FNS} from './interceptor'; diff --git a/packages/platform-server/src/http.ts b/packages/platform-server/src/http.ts index 80098962d9846..6dc14998b5b01 100644 --- a/packages/platform-server/src/http.ts +++ b/packages/platform-server/src/http.ts @@ -6,8 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {XhrFactory} from '@angular/common'; -import {Injectable, Provider} from '@angular/core'; +import {PlatformLocation, XhrFactory} from '@angular/common'; +import {HttpEvent, HttpHandlerFn, HttpRequest, ɵHTTP_ROOT_INTERCEPTOR_FNS as HTTP_ROOT_INTERCEPTOR_FNS} from '@angular/common/http'; +import {inject, Injectable, Provider} from '@angular/core'; +import {Observable} from 'rxjs'; @Injectable() export class ServerXhr implements XhrFactory { @@ -34,7 +36,31 @@ export class ServerXhr implements XhrFactory { } } +function relativeUrlsTransformerInterceptorFn( + request: HttpRequest, next: HttpHandlerFn): Observable> { + const platformLocation = inject(PlatformLocation); + const {href, protocol, hostname, port} = platformLocation; + if (!protocol.startsWith('http')) { + return next(request); + } + + let urlPrefix = `${protocol}//${hostname}`; + if (port) { + urlPrefix += `:${port}`; + } + + const baseHref = platformLocation.getBaseHrefFromDOM() || href; + const baseUrl = new URL(baseHref, urlPrefix); + const newUrl = new URL(request.url, baseUrl).toString(); + + return next(request.clone({url: newUrl})); +} export const SERVER_HTTP_PROVIDERS: Provider[] = [ {provide: XhrFactory, useClass: ServerXhr}, + { + provide: HTTP_ROOT_INTERCEPTOR_FNS, + useValue: relativeUrlsTransformerInterceptorFn, + multi: true, + }, ]; diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index b81487889b976..34b750d736efc 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -11,7 +11,7 @@ import {animate, AnimationBuilder, state, style, transition, trigger} from '@ang import {DOCUMENT, isPlatformServer, PlatformLocation, ɵgetDOM as getDOM} from '@angular/common'; import {HTTP_INTERCEPTORS, HttpClient, HttpClientModule, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest} from '@angular/common/http'; import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; -import {ApplicationConfig, ApplicationRef, Component, destroyPlatform, EnvironmentProviders, HostListener, Inject, inject as coreInject, Injectable, Input, makeStateKey, mergeApplicationConfig, NgModule, NgZone, PLATFORM_ID, Provider, TransferState, Type, ViewEncapsulation, ɵwhenStable as whenStable} from '@angular/core'; +import {ApplicationConfig, ApplicationRef, Component, destroyPlatform, EnvironmentProviders, HostListener, Inject, inject as coreInject, Injectable, Input, makeStateKey, mergeApplicationConfig, NgModule, NgModuleRef, NgZone, PLATFORM_ID, Provider, TransferState, Type, ViewEncapsulation, ɵwhenStable as whenStable} from '@angular/core'; import {SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core/src/hydration/utils'; import {InitialRenderPendingTasks} from '@angular/core/src/initial_render_pending_tasks'; import {TestBed} from '@angular/core/testing'; @@ -1147,6 +1147,43 @@ describe('platform-server integration', () => { }); }); }); + + describe(`given 'url' is provided in 'INITIAL_CONFIG'`, () => { + let mock: HttpTestingController; + let ref: NgModuleRef; + let http: HttpClient; + + beforeEach(async () => { + const platform = platformServer([{ + provide: INITIAL_CONFIG, + useValue: {document: '', url: 'http://localhost:4000/foo'} + }]); + + ref = await platform.bootstrapModule(HttpInterceptorExampleModule); + mock = ref.injector.get(HttpTestingController); + http = ref.injector.get(HttpClient); + }); + + it('should resolve relative request URLs to absolute', async () => { + ref.injector.get(NgZone).run(() => { + http.get('/testing').subscribe(body => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://localhost:4000/testing').flush('success!'); + }); + }); + + it(`should not replace the baseUrl of a request when it's absolute`, async () => { + ref.injector.get(NgZone).run(() => { + http.get('http://localhost/testing').subscribe(body => { + NgZone.assertInAngularZone(); + expect(body).toEqual('success!'); + }); + mock.expectOne('http://localhost/testing').flush('success!'); + }); + }); + }); }); }); })();