From 8aa6b14867b6f67ee6ade9b62cea9f23c0bf3ca4 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Sun, 16 Jun 2019 20:17:49 +0200 Subject: [PATCH] Unify base path in HttpService (#38237) (#39049) * unify modifyUrl on client and server * create BasePath as a separate entity on server * use BasePath class in http server * use BasePath a separate entity on client * use BasePath class on Http service on the client * switch client code to the new api * improve setver http service mocks * address comments #1 * address comments #2 * update docs * add comment why we define own typings --- ...-plugin-public.httpservicebase.basepath.md | 15 ++ ...ugin-public.httpservicebase.getbasepath.md | 15 -- .../kibana-plugin-public.httpservicebase.md | 4 +- ...-public.httpservicebase.prependbasepath.md | 22 --- ...n-public.httpservicebase.removebasepath.md | 22 --- .../kibana-plugin-server.coresetup.http.md | 3 +- .../server/kibana-plugin-server.coresetup.md | 2 +- .../nav_links/nav_links_service.test.ts | 4 +- .../chrome/nav_links/nav_links_service.ts | 2 +- .../public/http/base_path_service.test.ts | 91 ++++++++++++ src/core/public/http/base_path_service.ts | 71 ++++++++++ src/core/public/http/http_service.mock.ts | 36 +++-- src/core/public/http/http_service.test.ts | 68 +-------- src/core/public/http/http_setup.ts | 38 +---- src/core/public/http/types.ts | 8 +- .../public/plugins/plugins_service.test.ts | 6 +- src/core/public/plugins/plugins_service.ts | 2 +- src/core/public/public.api.md | 12 +- .../ui_settings_service.test.ts.snap | 8 +- src/core/public/utils/index.ts | 1 - src/core/public/utils/modify_url.test.ts | 59 -------- src/core/public/utils/modify_url.ts | 95 ------------- .../server/http/base_path_service.test.ts | 132 ++++++++++++++++++ src/core/server/http/base_path_service.ts | 76 ++++++++++ src/core/server/http/http_server.test.ts | 81 ----------- src/core/server/http/http_server.ts | 63 +++------ src/core/server/http/http_service.mock.ts | 24 +++- .../integration_tests/http_service.test.ts | 6 +- src/core/server/index.ts | 3 +- src/core/server/plugins/plugin_context.ts | 3 +- src/core/server/server.api.md | 3 +- src/core/utils/url.ts | 23 +-- .../server/http/setup_base_path_provider.js | 4 +- .../ui/public/chrome/api/base_path.test.ts | 30 ++-- src/legacy/ui/public/chrome/api/base_path.ts | 6 +- .../public/legacy_compat/angular_config.tsx | 2 +- .../notify/app_redirect/app_redirect.js | 2 +- src/legacy/ui/public/url/kibana_parsed_url.ts | 2 +- 38 files changed, 522 insertions(+), 522 deletions(-) create mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md delete mode 100644 docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md create mode 100644 src/core/public/http/base_path_service.test.ts create mode 100644 src/core/public/http/base_path_service.ts delete mode 100644 src/core/public/utils/modify_url.test.ts delete mode 100644 src/core/public/utils/modify_url.ts create mode 100644 src/core/server/http/base_path_service.test.ts create mode 100644 src/core/server/http/base_path_service.ts diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md new file mode 100644 index 0000000000000..6794199dc9dbd --- /dev/null +++ b/docs/development/core/public/kibana-plugin-public.httpservicebase.basepath.md @@ -0,0 +1,15 @@ + + +[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [basePath](./kibana-plugin-public.httpservicebase.basepath.md) + +## HttpServiceBase.basePath property + +Signature: + +```typescript +basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; +``` diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md deleted file mode 100644 index 918f0514b3c4b..0000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.getbasepath.md +++ /dev/null @@ -1,15 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [getBasePath](./kibana-plugin-public.httpservicebase.getbasepath.md) - -## HttpServiceBase.getBasePath() method - -Signature: - -```typescript -getBasePath(): string; -``` -Returns: - -`string` - diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.md index 2287855e20122..aa08108e2cdb0 100644 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.md +++ b/docs/development/core/public/kibana-plugin-public.httpservicebase.md @@ -15,6 +15,7 @@ export interface HttpServiceBase | Property | Type | Description | | --- | --- | --- | +| [basePath](./kibana-plugin-public.httpservicebase.basepath.md) | {
get: () => string;
prepend: (url: string) => string;
remove: (url: string) => string;
} | | | [delete](./kibana-plugin-public.httpservicebase.delete.md) | HttpHandler | | | [fetch](./kibana-plugin-public.httpservicebase.fetch.md) | HttpHandler | | | [get](./kibana-plugin-public.httpservicebase.get.md) | HttpHandler | | @@ -29,11 +30,8 @@ export interface HttpServiceBase | Method | Description | | --- | --- | | [addLoadingCount(count$)](./kibana-plugin-public.httpservicebase.addloadingcount.md) | | -| [getBasePath()](./kibana-plugin-public.httpservicebase.getbasepath.md) | | | [getLoadingCount$()](./kibana-plugin-public.httpservicebase.getloadingcount$.md) | | | [intercept(interceptor)](./kibana-plugin-public.httpservicebase.intercept.md) | | -| [prependBasePath(path)](./kibana-plugin-public.httpservicebase.prependbasepath.md) | | | [removeAllInterceptors()](./kibana-plugin-public.httpservicebase.removeallinterceptors.md) | | -| [removeBasePath(path)](./kibana-plugin-public.httpservicebase.removebasepath.md) | | | [stop()](./kibana-plugin-public.httpservicebase.stop.md) | | diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md deleted file mode 100644 index a4ae95eb1c307..0000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.prependbasepath.md +++ /dev/null @@ -1,22 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [prependBasePath](./kibana-plugin-public.httpservicebase.prependbasepath.md) - -## HttpServiceBase.prependBasePath() method - -Signature: - -```typescript -prependBasePath(path: string): string; -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| path | string | | - -Returns: - -`string` - diff --git a/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md b/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md deleted file mode 100644 index 90fefb1532a68..0000000000000 --- a/docs/development/core/public/kibana-plugin-public.httpservicebase.removebasepath.md +++ /dev/null @@ -1,22 +0,0 @@ - - -[Home](./index.md) > [kibana-plugin-public](./kibana-plugin-public.md) > [HttpServiceBase](./kibana-plugin-public.httpservicebase.md) > [removeBasePath](./kibana-plugin-public.httpservicebase.removebasepath.md) - -## HttpServiceBase.removeBasePath() method - -Signature: - -```typescript -removeBasePath(path: string): string; -``` - -## Parameters - -| Parameter | Type | Description | -| --- | --- | --- | -| path | string | | - -Returns: - -`string` - diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.http.md b/docs/development/core/server/kibana-plugin-server.coresetup.http.md index d6b87006da533..cba8a5832058d 100644 --- a/docs/development/core/server/kibana-plugin-server.coresetup.http.md +++ b/docs/development/core/server/kibana-plugin-server.coresetup.http.md @@ -11,8 +11,7 @@ http: { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; createNewServer: HttpServiceSetup['createNewServer']; }; ``` diff --git a/docs/development/core/server/kibana-plugin-server.coresetup.md b/docs/development/core/server/kibana-plugin-server.coresetup.md index aea7349d3880e..3757338b80d4d 100644 --- a/docs/development/core/server/kibana-plugin-server.coresetup.md +++ b/docs/development/core/server/kibana-plugin-server.coresetup.md @@ -17,5 +17,5 @@ export interface CoreSetup | Property | Type | Description | | --- | --- | --- | | [elasticsearch](./kibana-plugin-server.coresetup.elasticsearch.md) | {
adminClient$: Observable<ClusterClient>;
dataClient$: Observable<ClusterClient>;
} | | -| [http](./kibana-plugin-server.coresetup.http.md) | {
registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
registerAuth: HttpServiceSetup['registerAuth'];
registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
getBasePathFor: HttpServiceSetup['getBasePathFor'];
setBasePathFor: HttpServiceSetup['setBasePathFor'];
createNewServer: HttpServiceSetup['createNewServer'];
} | | +| [http](./kibana-plugin-server.coresetup.http.md) | {
registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
registerAuth: HttpServiceSetup['registerAuth'];
registerOnPostAuth: HttpServiceSetup['registerOnPostAuth'];
basePath: HttpServiceSetup['basePath'];
createNewServer: HttpServiceSetup['createNewServer'];
} | | diff --git a/src/core/public/chrome/nav_links/nav_links_service.test.ts b/src/core/public/chrome/nav_links/nav_links_service.test.ts index d6ccdf84c0f2c..fe74ae3a7d9a2 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.test.ts +++ b/src/core/public/chrome/nav_links/nav_links_service.test.ts @@ -29,7 +29,9 @@ const mockAppService = { } as any; const mockHttp = { - prependBasePath: (url: string) => `wow${url}`, + basePath: { + prepend: (url: string) => `wow${url}`, + }, } as any; describe('NavLinksService', () => { diff --git a/src/core/public/chrome/nav_links/nav_links_service.ts b/src/core/public/chrome/nav_links/nav_links_service.ts index e8ea30ff6f61d..60f5e4574752e 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.ts +++ b/src/core/public/chrome/nav_links/nav_links_service.ts @@ -42,7 +42,7 @@ export class NavLinksService { new NavLinkWrapper({ ...app, // Either rootRoute or appUrl must be defined. - baseUrl: relativeToAbsolute(http.prependBasePath((app.rootRoute || app.appUrl)!)), + baseUrl: relativeToAbsolute(http.basePath.prepend((app.rootRoute || app.appUrl)!)), }), ] as [string, NavLinkWrapper] ) diff --git a/src/core/public/http/base_path_service.test.ts b/src/core/public/http/base_path_service.test.ts new file mode 100644 index 0000000000000..65403c906e614 --- /dev/null +++ b/src/core/public/http/base_path_service.test.ts @@ -0,0 +1,91 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { BasePath } from './base_path_service'; + +describe('BasePath', () => { + describe('#get()', () => { + it('returns an empty string if no basePath not provided', () => { + expect(new BasePath().get()).toBe(''); + }); + + it('returns basePath value if provided', () => { + expect(new BasePath('/foo').get()).toBe('/foo'); + }); + + describe('#prepend()', () => { + it('adds the base path to the path if it is relative and starts with a slash', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('/a/b')).toBe('/foo/bar/a/b'); + }); + + it('leaves the query string and hash of path unchanged', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('/a/b?x=y#c/d/e')).toBe('/foo/bar/a/b?x=y#c/d/e'); + }); + + it('returns the path unchanged if it does not start with a slash', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('a/b')).toBe('a/b'); + }); + + it('returns the path unchanged it it has a hostname', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b'); + }); + }); + + describe('#remove()', () => { + it('removes the basePath if relative path starts with it', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar/a/b')).toBe('/a/b'); + }); + + it('leaves query string and hash intact', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234'); + }); + + it('ignores urls with hostnames', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('http://localhost:5601/foo/bar/a/b')).toBe( + 'http://localhost:5601/foo/bar/a/b' + ); + }); + + it('returns slash if path is just basePath', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar')).toBe('/'); + }); + + it('returns full path if basePath is not its own segment', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/barhop')).toBe('/foo/barhop'); + }); + }); + }); +}); diff --git a/src/core/public/http/base_path_service.ts b/src/core/public/http/base_path_service.ts new file mode 100644 index 0000000000000..6352327c41625 --- /dev/null +++ b/src/core/public/http/base_path_service.ts @@ -0,0 +1,71 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { modifyUrl } from '../../utils'; + +export class BasePath { + constructor(private readonly basePath: string = '') {} + + public get = () => { + return this.basePath; + }; + + public prepend = (path: string): string => { + if (!this.basePath) return path; + return modifyUrl(path, parts => { + if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) { + parts.pathname = `${this.basePath}${parts.pathname}`; + } + }); + }; + + public remove = (path: string): string => { + if (!this.basePath) { + return path; + } + + if (path === this.basePath) { + return '/'; + } + + if (path.startsWith(`${this.basePath}/`)) { + return path.slice(this.basePath.length); + } + + return path; + }; +} diff --git a/src/core/public/http/http_service.mock.ts b/src/core/public/http/http_service.mock.ts index 3f5611ba7f7df..4ca925a19af81 100644 --- a/src/core/public/http/http_service.mock.ts +++ b/src/core/public/http/http_service.mock.ts @@ -18,9 +18,13 @@ */ import { HttpService } from './http_service'; -import { HttpSetup, HttpStart } from './types'; +import { HttpSetup } from './types'; -const createServiceMock = () => ({ +type ServiceSetupMockType = jest.Mocked & { + basePath: jest.Mocked; +}; + +const createServiceMock = (): ServiceSetupMockType => ({ fetch: jest.fn(), get: jest.fn(), head: jest.fn(), @@ -29,9 +33,11 @@ const createServiceMock = () => ({ patch: jest.fn(), delete: jest.fn(), options: jest.fn(), - getBasePath: jest.fn(), - prependBasePath: jest.fn(), - removeBasePath: jest.fn(), + basePath: { + get: jest.fn(), + prepend: jest.fn(), + remove: jest.fn(), + }, addLoadingCount: jest.fn(), getLoadingCount$: jest.fn(), stop: jest.fn(), @@ -39,13 +45,19 @@ const createServiceMock = () => ({ removeAllInterceptors: jest.fn(), }); -const createSetupContractMock = (): jest.Mocked => createServiceMock(); -const createStartContractMock = (): jest.Mocked => createServiceMock(); -const createMock = (): jest.Mocked> => ({ - setup: jest.fn().mockReturnValue(createSetupContractMock()), - start: jest.fn().mockReturnValue(createStartContractMock()), - stop: jest.fn(), -}); +const createSetupContractMock = () => createServiceMock(); +const createStartContractMock = () => createServiceMock(); + +const createMock = () => { + const mocked: jest.Mocked> = { + setup: jest.fn(), + start: jest.fn(), + stop: jest.fn(), + }; + mocked.setup.mockReturnValue(createSetupContractMock()); + mocked.start.mockReturnValue(createSetupContractMock()); + return mocked; +}; export const httpServiceMock = { create: createMock, diff --git a/src/core/public/http/http_service.test.ts b/src/core/public/http/http_service.test.ts index d40152157ec93..68fb08bfb38e3 100644 --- a/src/core/public/http/http_service.test.ts +++ b/src/core/public/http/http_service.test.ts @@ -29,79 +29,19 @@ const setupFakeBasePath: SetupTap = injectedMetadata => { injectedMetadata.getBasePath.mockReturnValue('/foo/bar'); }; -describe('getBasePath', () => { +describe('basePath.get()', () => { it('returns an empty string if no basePath is injected', () => { const { http } = setup(injectedMetadata => { - injectedMetadata.getBasePath.mockReturnValue(''); + injectedMetadata.getBasePath.mockReturnValue(undefined as any); }); - expect(http.getBasePath()).toBe(''); + expect(http.basePath.get()).toBe(''); }); it('returns the injected basePath', () => { const { http } = setup(setupFakeBasePath); - expect(http.getBasePath()).toBe('/foo/bar'); - }); -}); - -describe('prependBasePath', () => { - it('adds the base path to the path if it is relative and starts with a slash', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('/a/b')).toBe('/foo/bar/a/b'); - }); - - it('leaves the query string and hash of path unchanged', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('/a/b?x=y#c/d/e')).toBe('/foo/bar/a/b?x=y#c/d/e'); - }); - - it('returns the path unchanged if it does not start with a slash', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('a/b')).toBe('a/b'); - }); - - it('returns the path unchanged it it has a hostname', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.prependBasePath('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b'); - }); -}); - -describe('removeBasePath', () => { - it('removes the basePath if relative path starts with it', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar/a/b')).toBe('/a/b'); - }); - - it('leaves query string and hash intact', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234'); - }); - - it('ignores urls with hostnames', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('http://localhost:5601/foo/bar/a/b')).toBe( - 'http://localhost:5601/foo/bar/a/b' - ); - }); - - it('returns slash if path is just basePath', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/bar')).toBe('/'); - }); - - it('returns full path if basePath is not its own segment', () => { - const { http } = setup(setupFakeBasePath); - - expect(http.removeBasePath('/foo/barhop')).toBe('/foo/barhop'); + expect(http.basePath.get()).toBe('/foo/bar'); }); }); diff --git a/src/core/public/http/http_setup.ts b/src/core/public/http/http_setup.ts index c12ec4be5d3a3..0e270b3e2610e 100644 --- a/src/core/public/http/http_setup.ts +++ b/src/core/public/http/http_setup.ts @@ -31,11 +31,11 @@ import { merge } from 'lodash'; import { format } from 'url'; import { InjectedMetadataSetup } from '../injected_metadata'; import { FatalErrorsSetup } from '../fatal_errors'; -import { modifyUrl } from '../utils'; import { HttpFetchOptions, HttpServiceBase, HttpInterceptor, HttpResponse } from './types'; import { HttpInterceptController } from './http_intercept_controller'; import { HttpFetchError } from './http_fetch_error'; import { HttpInterceptHaltError } from './http_intercept_halt_error'; +import { BasePath } from './base_path_service'; const JSON_CONTENT = /^(application\/(json|x-javascript)|text\/(x-)?javascript|x-json)(;.*)?$/; const NDJSON_CONTENT = /^(application\/ndjson)(;.*)?$/; @@ -48,7 +48,7 @@ export const setup = ( const stop$ = new Subject(); const interceptors = new Set(); const kibanaVersion = injectedMetadata.getKibanaVersion(); - const basePath = injectedMetadata.getBasePath() || ''; + const basePath = new BasePath(injectedMetadata.getBasePath()); function intercept(interceptor: HttpInterceptor) { interceptors.add(interceptor); @@ -60,14 +60,6 @@ export const setup = ( interceptors.clear(); } - function prependBasePath(path: string): string { - return modifyUrl(path, parts => { - if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) { - parts.pathname = `${basePath}${parts.pathname}`; - } - }); - } - function createRequest(path: string, options?: HttpFetchOptions) { const { query, prependBasePath: shouldPrependBasePath, ...fetchOptions } = merge( { @@ -82,7 +74,7 @@ export const setup = ( options || {} ); const url = format({ - pathname: shouldPrependBasePath ? prependBasePath(path) : path, + pathname: shouldPrependBasePath ? basePath.prepend(path) : path, query, }); @@ -255,26 +247,6 @@ export const setup = ( loadingCount$.complete(); } - function getBasePath() { - return basePath; - } - - function removeBasePath(path: string): string { - if (!basePath) { - return path; - } - - if (path === basePath) { - return '/'; - } - - if (path.startsWith(`${basePath}/`)) { - return path.slice(basePath.length); - } - - return path; - } - function addLoadingCount(count$: Observable) { count$ .pipe( @@ -314,9 +286,7 @@ export const setup = ( return { stop, - getBasePath, - prependBasePath, - removeBasePath, + basePath, intercept, removeAllInterceptors, fetch, diff --git a/src/core/public/http/types.ts b/src/core/public/http/types.ts index 9f28d03e4e5af..79527e528ac40 100644 --- a/src/core/public/http/types.ts +++ b/src/core/public/http/types.ts @@ -26,9 +26,11 @@ import { HttpFetchError } from './http_fetch_error'; /** @public */ export interface HttpServiceBase { stop(): void; - getBasePath(): string; - prependBasePath(path: string): string; - removeBasePath(path: string): string; + basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; intercept(interceptor: HttpInterceptor): () => void; removeAllInterceptors(): void; fetch: HttpHandler; diff --git a/src/core/public/plugins/plugins_service.test.ts b/src/core/public/plugins/plugins_service.test.ts index 84d4f33fdb153..84f01eda00ca9 100644 --- a/src/core/public/plugins/plugins_service.test.ts +++ b/src/core/public/plugins/plugins_service.test.ts @@ -167,9 +167,9 @@ test('`PluginsService.setup` calls loadPluginBundles with http and plugins', asy await pluginsService.setup(mockSetupDeps); expect(mockLoadPluginBundle).toHaveBeenCalledTimes(3); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginA'); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginB'); - expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.prependBasePath, 'pluginC'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginA'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginB'); + expect(mockLoadPluginBundle).toHaveBeenCalledWith(mockSetupDeps.http.basePath.prepend, 'pluginC'); }); test('`PluginsService.setup` initalizes plugins with CoreContext', async () => { diff --git a/src/core/public/plugins/plugins_service.ts b/src/core/public/plugins/plugins_service.ts index c3b337a8956ec..6d49a77ad08eb 100644 --- a/src/core/public/plugins/plugins_service.ts +++ b/src/core/public/plugins/plugins_service.ts @@ -70,7 +70,7 @@ export class PluginsService implements CoreService(); diff --git a/src/core/public/public.api.md b/src/core/public/public.api.md index 7cfbe61abff3a..ed5fbb26eba16 100644 --- a/src/core/public/public.api.md +++ b/src/core/public/public.api.md @@ -190,6 +190,12 @@ export interface HttpServiceBase { // (undocumented) addLoadingCount(count$: Observable): void; // (undocumented) + basePath: { + get: () => string; + prepend: (url: string) => string; + remove: (url: string) => string; + }; + // (undocumented) delete: HttpHandler; // Warning: (ae-forgotten-export) The symbol "HttpHandler" needs to be exported by the entry point index.d.ts // @@ -198,8 +204,6 @@ export interface HttpServiceBase { // (undocumented) get: HttpHandler; // (undocumented) - getBasePath(): string; - // (undocumented) getLoadingCount$(): Observable; // (undocumented) head: HttpHandler; @@ -212,14 +216,10 @@ export interface HttpServiceBase { // (undocumented) post: HttpHandler; // (undocumented) - prependBasePath(path: string): string; - // (undocumented) put: HttpHandler; // (undocumented) removeAllInterceptors(): void; // (undocumented) - removeBasePath(path: string): string; - // (undocumented) stop(): void; } diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap index edbdef3f05099..f607c924a9e68 100644 --- a/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap +++ b/src/core/public/ui_settings/__snapshots__/ui_settings_service.test.ts.snap @@ -20,20 +20,22 @@ exports[`#setup constructs UiSettingsClient and UiSettingsApi: UiSettingsApi arg }, ], }, + "basePath": Object { + "get": [MockFunction], + "prepend": [MockFunction], + "remove": [MockFunction], + }, "delete": [MockFunction], "fetch": [MockFunction], "get": [MockFunction], - "getBasePath": [MockFunction], "getLoadingCount$": [MockFunction], "head": [MockFunction], "intercept": [MockFunction], "options": [MockFunction], "patch": [MockFunction], "post": [MockFunction], - "prependBasePath": [MockFunction], "put": [MockFunction], "removeAllInterceptors": [MockFunction], - "removeBasePath": [MockFunction], "stop": [MockFunction], }, ], diff --git a/src/core/public/utils/index.ts b/src/core/public/utils/index.ts index 786e12e4de688..300fb1e43e29a 100644 --- a/src/core/public/utils/index.ts +++ b/src/core/public/utils/index.ts @@ -17,5 +17,4 @@ * under the License. */ -export { modifyUrl } from './modify_url'; export { shareWeakReplay } from './share_weak_replay'; diff --git a/src/core/public/utils/modify_url.test.ts b/src/core/public/utils/modify_url.test.ts deleted file mode 100644 index 3b8091b7c3074..0000000000000 --- a/src/core/public/utils/modify_url.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { modifyUrl } from './modify_url'; - -it('supports returning a new url spec', () => { - expect(modifyUrl('http://localhost', () => ({}))).toBe(''); -}); - -it('supports modifying the passed object', () => { - expect( - modifyUrl('http://localhost', parsed => { - parsed.port = '9999'; - parsed.auth = 'foo:bar'; - }) - ).toBe('http://foo:bar@localhost:9999/'); -}); - -it('supports changing pathname', () => { - expect( - modifyUrl('http://localhost/some/path', parsed => { - parsed.pathname += '/subpath'; - }) - ).toBe('http://localhost/some/path/subpath'); -}); - -it('supports changing port', () => { - expect( - modifyUrl('http://localhost:5601', parsed => { - parsed.port = String(Number(parsed.port) + 1); - }) - ).toBe('http://localhost:5602/'); -}); - -it('supports changing protocol', () => { - expect( - modifyUrl('http://localhost', parsed => { - parsed.protocol = 'mail'; - parsed.slashes = false; - parsed.pathname = undefined; - }) - ).toBe('mail:localhost'); -}); diff --git a/src/core/public/utils/modify_url.ts b/src/core/public/utils/modify_url.ts deleted file mode 100644 index a441c7eed6a38..0000000000000 --- a/src/core/public/utils/modify_url.ts +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { format as formatUrl, parse as parseUrl } from 'url'; - -interface UrlParts { - protocol?: string; - slashes?: boolean; - auth?: string; - hostname?: string; - port?: string; - pathname?: string; - query: { [key: string]: string | string[] | undefined }; - hash?: string; -} - -/** - * Takes a URL and a function that takes the meaningful parts - * of the URL as a key-value object, modifies some or all of - * the parts, and returns the modified parts formatted again - * as a url. - * - * Url Parts sent: - * - protocol - * - slashes (does the url have the //) - * - auth - * - hostname (just the name of the host, no port or auth information) - * - port - * - pathname (the path after the hostname, no query or hash, starts - * with a slash if there was a path) - * - query (always an object, even when no query on original url) - * - hash - * - * Why? - * - The default url library in node produces several conflicting - * properties on the "parsed" output. Modifying any of these might - * lead to the modifications being ignored (depending on which - * property was modified) - * - It's not always clear wither to use path/pathname, host/hostname, - * so this tries to add helpful constraints - * - * @param url the url to parse - * @param block a function that will modify the parsed url, or return a new one - */ -export function modifyUrl(url: string, block: (parts: UrlParts) => Partial | void) { - const parsed = parseUrl(url, true); - - // copy over the most specific version of each - // property. By default, the parsed url includes - // several conflicting properties (like path and - // pathname + search, or search and query) and keeping - // track of which property is actually used when they - // are formatted is harder than necessary - const meaningfulParts = { - protocol: parsed.protocol, - slashes: parsed.slashes, - auth: parsed.auth, - hostname: parsed.hostname, - port: parsed.port, - pathname: parsed.pathname, - query: parsed.query || {}, - hash: parsed.hash, - }; - - // the block modifies the meaningfulParts object, or returns a new one - const modifiedParts = block(meaningfulParts) || meaningfulParts; - - // format the modified/replaced meaningfulParts back into a url - return formatUrl({ - protocol: modifiedParts.protocol, - slashes: modifiedParts.slashes, - auth: modifiedParts.auth, - hostname: modifiedParts.hostname, - port: modifiedParts.port, - pathname: modifiedParts.pathname, - query: modifiedParts.query, - hash: modifiedParts.hash, - }); -} diff --git a/src/core/server/http/base_path_service.test.ts b/src/core/server/http/base_path_service.test.ts new file mode 100644 index 0000000000000..ffbbe158cb2d4 --- /dev/null +++ b/src/core/server/http/base_path_service.test.ts @@ -0,0 +1,132 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { BasePath } from './base_path_service'; +import { KibanaRequest } from './router'; +import { httpServerMock } from './http_server.mocks'; + +describe('BasePath', () => { + describe('#get()', () => { + it('returns base path associated with an incoming Legacy.Request request', () => { + const request = httpServerMock.createRawRequest(); + + const basePath = new BasePath(); + basePath.set(request, '/baz/'); + expect(basePath.get(request)).toBe('/baz/'); + }); + + it('returns base path associated with an incoming KibanaRequest', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath(); + + basePath.set(KibanaRequest.from(request, undefined), '/baz/'); + expect(basePath.get(KibanaRequest.from(request, undefined))).toBe('/baz/'); + }); + + it('operates with both Legacy.Request/KibanaRequest requests', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath(); + + basePath.set(request, '/baz/'); + expect(basePath.get(KibanaRequest.from(request, undefined))).toBe('/baz/'); + }); + + it('is based on server base path', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath('/foo/bar'); + + basePath.set(request, '/baz/'); + expect(basePath.get(request)).toBe('/foo/bar/baz/'); + }); + }); + + describe('#set()', () => { + it('#set() cannot be set twice for one request', () => { + const request = httpServerMock.createRawRequest(); + const basePath = new BasePath('/foo/bar'); + + const setPath = () => basePath.set(request, 'baz/'); + setPath(); + + expect(setPath).toThrowErrorMatchingInlineSnapshot( + `"Request basePath was previously set. Setting multiple times is not supported."` + ); + }); + }); + + describe('#prepend()', () => { + it('adds the base path to the path if it is relative and starts with a slash', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('/a/b')).toBe('/foo/bar/a/b'); + }); + + it('leaves the query string and hash of path unchanged', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('/a/b?x=y#c/d/e')).toBe('/foo/bar/a/b?x=y#c/d/e'); + }); + + it('returns the path unchanged if it does not start with a slash', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('a/b')).toBe('a/b'); + }); + + it('returns the path unchanged it it has a hostname', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.prepend('http://localhost:5601/a/b')).toBe('http://localhost:5601/a/b'); + }); + }); + + describe('#remove()', () => { + it('removes the basePath if relative path starts with it', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar/a/b')).toBe('/a/b'); + }); + + it('leaves query string and hash intact', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar/a/b?c=y#1234')).toBe('/a/b?c=y#1234'); + }); + + it('ignores urls with hostnames', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('http://localhost:5601/foo/bar/a/b')).toBe( + 'http://localhost:5601/foo/bar/a/b' + ); + }); + + it('returns slash if path is just basePath', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/bar')).toBe('/'); + }); + + it('returns full path if basePath is not its own segment', () => { + const basePath = new BasePath('/foo/bar'); + + expect(basePath.remove('/foo/barhop')).toBe('/foo/barhop'); + }); + }); +}); diff --git a/src/core/server/http/base_path_service.ts b/src/core/server/http/base_path_service.ts new file mode 100644 index 0000000000000..a6a868547bfb1 --- /dev/null +++ b/src/core/server/http/base_path_service.ts @@ -0,0 +1,76 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Request } from 'hapi'; +import { KibanaRequest, toRawRequest } from './router'; + +import { modifyUrl } from '../../utils'; + +const getIncomingMessage = (request: KibanaRequest | Request) => + request instanceof KibanaRequest ? toRawRequest(request).raw.req : request.raw.req; + +export class BasePath { + private readonly basePathCache = new WeakMap, string>(); + + constructor(private readonly serverBasePath?: string) {} + + public get = (request: KibanaRequest | Request) => { + const incomingMessage = getIncomingMessage(request); + + const requestScopePath = this.basePathCache.get(incomingMessage) || ''; + const serverBasePath = this.serverBasePath || ''; + return `${serverBasePath}${requestScopePath}`; + }; + + // should work only for KibanaRequest as soon as spaces migrate to NP + public set = (request: KibanaRequest | Request, requestSpecificBasePath: string) => { + const incomingMessage = getIncomingMessage(request); + + if (this.basePathCache.has(incomingMessage)) { + throw new Error( + 'Request basePath was previously set. Setting multiple times is not supported.' + ); + } + this.basePathCache.set(incomingMessage, requestSpecificBasePath); + }; + + public prepend = (path: string): string => { + if (!this.serverBasePath) return path; + return modifyUrl(path, parts => { + if (!parts.hostname && parts.pathname && parts.pathname.startsWith('/')) { + parts.pathname = `${this.serverBasePath}${parts.pathname}`; + } + }); + }; + + public remove = (path: string): string => { + if (!this.serverBasePath) { + return path; + } + + if (path === this.serverBasePath) { + return '/'; + } + + if (path.startsWith(`${this.serverBasePath}/`)) { + return path.slice(this.serverBasePath.length); + } + + return path; + }; +} diff --git a/src/core/server/http/http_server.test.ts b/src/core/server/http/http_server.test.ts index 603cfb06c7fa7..53bf5c2abc816 100644 --- a/src/core/server/http/http_server.test.ts +++ b/src/core/server/http/http_server.test.ts @@ -32,8 +32,6 @@ import { ByteSizeValue } from '@kbn/config-schema'; import { HttpConfig, Router } from '.'; import { loggingServiceMock } from '../logging/logging_service.mock'; import { HttpServer } from './http_server'; -import { KibanaRequest } from './router'; -import { httpServerMock } from './http_server.mocks'; const chance = new Chance(); @@ -602,85 +600,6 @@ test('throws an error if starts without set up', async () => { ); }); -test('#getBasePathFor() returns base path associated with an incoming request', async () => { - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(config); - - const path = '/base-path'; - registerOnPostAuth((req, t) => { - setBasePathFor(req, path); - return t.next(); - }); - - const router = new Router('/'); - router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) })); - registerRouter(router); - - await server.start(); - await supertest(innerServer.listener) - .get('/') - .expect(200) - .then(res => { - expect(res.body).toEqual({ key: path }); - }); -}); - -test('#getBasePathFor() is based on server base path', async () => { - const configWithBasePath = { - ...config, - basePath: '/bar', - }; - const { - getBasePathFor, - setBasePathFor, - registerRouter, - server: innerServer, - registerOnPostAuth, - } = await server.setup(configWithBasePath); - - const path = '/base-path'; - registerOnPostAuth((req, t) => { - setBasePathFor(req, path); - return t.next(); - }); - - const router = new Router('/'); - router.get({ path: '/', validate: false }, (req, res) => res.ok({ key: getBasePathFor(req) })); - registerRouter(router); - - await server.start(); - await supertest(innerServer.listener) - .get('/') - .expect(200) - .then(res => { - expect(res.body).toEqual({ key: `${configWithBasePath.basePath}${path}` }); - }); -}); - -test('#setBasePathFor() cannot be set twice for one request', async () => { - const kibanaRequestFactory = { - from() { - return KibanaRequest.from(httpServerMock.createRawRequest()); - }, - }; - jest.doMock('./router/request', () => ({ - KibanaRequest: jest.fn(() => kibanaRequestFactory), - })); - - const { setBasePathFor } = await server.setup(config); - const req = kibanaRequestFactory.from(); - const setPath = () => setBasePathFor(req, '/path'); - - setPath(); - expect(setPath).toThrowErrorMatchingInlineSnapshot( - `"Request basePath was previously set. Setting multiple times is not supported."` - ); -}); const cookieOptions = { name: 'sid', encryptionKey: 'something_at_least_32_characters', diff --git a/src/core/server/http/http_server.ts b/src/core/server/http/http_server.ts index 6f59eb11906ec..1cff8cd1d312e 100644 --- a/src/core/server/http/http_server.ts +++ b/src/core/server/http/http_server.ts @@ -19,23 +19,20 @@ import { Request, Server, ServerOptions } from 'hapi'; -import { modifyUrl } from '../../utils'; import { Logger } from '../logging'; import { HttpConfig } from './http_config'; import { createServer, getServerOptions } from './http_tools'; import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth'; import { adoptToHapiOnPostAuthFormat, OnPostAuthHandler } from './lifecycle/on_post_auth'; import { adoptToHapiOnPreAuthFormat, OnPreAuthHandler } from './lifecycle/on_pre_auth'; -import { Router, KibanaRequest, toRawRequest } from './router'; +import { Router, KibanaRequest } from './router'; import { SessionStorageCookieOptions, createCookieSessionStorageFactory, } from './cookie_session_storage'; import { SessionStorageFactory } from './session_storage'; import { AuthStateStorage } from './auth_state_storage'; - -const getIncomingMessage = (request: KibanaRequest | Request) => - request instanceof KibanaRequest ? toRawRequest(request).raw.req : request.raw.req; +import { BasePath } from './base_path_service'; export interface HttpServerSetup { server: Server; @@ -67,8 +64,12 @@ export interface HttpServerSetup { * (from the first registered to the last). */ registerOnPostAuth: (handler: OnPostAuthHandler) => void; - getBasePathFor: (request: KibanaRequest | Request) => string; - setBasePathFor: (request: KibanaRequest | Request, basePath: string) => void; + basePath: { + get: (request: KibanaRequest | Request) => string; + set: (request: KibanaRequest | Request, basePath: string) => void; + prepend: (url: string) => string; + remove: (url: string) => string; + }; auth: { get: AuthStateStorage['get']; isAuthenticated: AuthStateStorage['isAuthenticated']; @@ -80,7 +81,6 @@ export class HttpServer { private config?: HttpConfig; private registeredRouters = new Set(); private authRegistered = false; - private basePathCache = new WeakMap, string>(); private readonly authState: AuthStateStorage; @@ -101,33 +101,13 @@ export class HttpServer { this.registeredRouters.add(router); } - // passing hapi Request works for BWC. can be deleted once we remove legacy server. - private getBasePathFor(config: HttpConfig, request: KibanaRequest | Request) { - const incomingMessage = getIncomingMessage(request); - - const requestScopePath = this.basePathCache.get(incomingMessage) || ''; - const serverBasePath = config.basePath || ''; - return `${serverBasePath}${requestScopePath}`; - } - - // should work only for KibanaRequest as soon as spaces migrate to NP - private setBasePathFor(request: KibanaRequest | Request, basePath: string) { - const incomingMessage = getIncomingMessage(request); - - if (this.basePathCache.has(incomingMessage)) { - throw new Error( - 'Request basePath was previously set. Setting multiple times is not supported.' - ); - } - this.basePathCache.set(incomingMessage, basePath); - } - public setup(config: HttpConfig): HttpServerSetup { const serverOptions = getServerOptions(config); this.server = createServer(serverOptions); this.config = config; - this.setupBasePathRewrite(config); + const basePathService = new BasePath(config.basePath); + this.setupBasePathRewrite(config, basePathService); return { options: serverOptions, @@ -136,8 +116,7 @@ export class HttpServer { registerOnPostAuth: this.registerOnPostAuth.bind(this), registerAuth: (fn: AuthenticationHandler, cookieOptions: SessionStorageCookieOptions) => this.registerAuth(fn, cookieOptions, config.basePath), - getBasePathFor: this.getBasePathFor.bind(this, config), - setBasePathFor: this.setBasePathFor.bind(this), + basePath: basePathService, auth: { get: this.authState.get, isAuthenticated: this.authState.isAuthenticated, @@ -185,25 +164,19 @@ export class HttpServer { this.server = undefined; } - private setupBasePathRewrite(config: HttpConfig) { + private setupBasePathRewrite(config: HttpConfig, basePathService: BasePath) { if (config.basePath === undefined || !config.rewriteBasePath) { return; } - const basePath = config.basePath; this.registerOnPreAuth((request, toolkit) => { - const newURL = modifyUrl(request.url.href!, urlParts => { - if (urlParts.pathname != null && urlParts.pathname.startsWith(basePath)) { - urlParts.pathname = urlParts.pathname.replace(basePath, '') || '/'; - } else { - return {}; - } - }); - if (!newURL) { - return toolkit.rejected(new Error('not found'), { statusCode: 404 }); + const oldUrl = request.url.href!; + const newURL = basePathService.remove(oldUrl); + const shouldRedirect = newURL !== oldUrl; + if (shouldRedirect) { + return toolkit.redirected(newURL, { forward: true }); } - - return toolkit.redirected(newURL, { forward: true }); + return toolkit.rejected(new Error('not found'), { statusCode: 404 }); }); } diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index ff20385792d83..07040560f89cd 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -19,24 +19,34 @@ import { Server, ServerOptions } from 'hapi'; import { HttpService } from './http_service'; +import { HttpServerSetup } from './http_server'; +import { HttpServiceSetup } from './http_service'; +type ServiceSetupMockType = jest.Mocked & { + basePath: jest.Mocked; +}; const createSetupContractMock = () => { - const setupContract = { - options: {} as ServerOptions, + const setupContract: ServiceSetupMockType = { + options: ({} as unknown) as ServerOptions, + // we can mock some hapi server method when we need it + server: {} as Server, registerOnPreAuth: jest.fn(), registerAuth: jest.fn(), registerOnPostAuth: jest.fn(), registerRouter: jest.fn(), - getBasePathFor: jest.fn(), - setBasePathFor: jest.fn(), - // we can mock some hapi server method when we need it - server: {} as Server, + basePath: { + get: jest.fn(), + set: jest.fn(), + prepend: jest.fn(), + remove: jest.fn(), + }, auth: { get: jest.fn(), isAuthenticated: jest.fn(), }, - createNewServer: jest.fn().mockResolvedValue({}), + createNewServer: jest.fn(), }; + setupContract.createNewServer.mockResolvedValue({} as HttpServerSetup); return setupContract; }; diff --git a/src/core/server/http/integration_tests/http_service.test.ts b/src/core/server/http/integration_tests/http_service.test.ts index 100efc4e2a607..21207d0b90e25 100644 --- a/src/core/server/http/integration_tests/http_service.test.ts +++ b/src/core/server/http/integration_tests/http_service.test.ts @@ -238,7 +238,7 @@ describe('http service', () => { }); }); - describe('#getBasePathFor()/#setBasePathFor()', () => { + describe('#basePath()', () => { let root: ReturnType; beforeEach(async () => { root = kbnTestServer.createRoot(); @@ -249,7 +249,7 @@ describe('http service', () => { const reqBasePath = '/requests-specific-base-path'; const { http } = await root.setup(); http.registerOnPreAuth((req, t) => { - http.setBasePathFor(req, reqBasePath); + http.basePath.set(req, reqBasePath); return t.next(); }); @@ -260,7 +260,7 @@ describe('http service', () => { kbnServer.server.route({ method: 'GET', path: legacyUrl, - handler: kbnServer.newPlatform.setup.core.http.getBasePathFor, + handler: kbnServer.newPlatform.setup.core.http.basePath.get, }); await kbnTestServer.request.get(root, legacyUrl).expect(200, reqBasePath); diff --git a/src/core/server/index.ts b/src/core/server/index.ts index 4cebe24f39f15..f901b25e1ae87 100644 --- a/src/core/server/index.ts +++ b/src/core/server/index.ts @@ -91,8 +91,7 @@ export interface CoreSetup { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; createNewServer: HttpServiceSetup['createNewServer']; }; } diff --git a/src/core/server/plugins/plugin_context.ts b/src/core/server/plugins/plugin_context.ts index 9dfc2df1c2d20..10912ca608485 100644 --- a/src/core/server/plugins/plugin_context.ts +++ b/src/core/server/plugins/plugin_context.ts @@ -120,8 +120,7 @@ export function createPluginSetupContext( registerOnPreAuth: deps.http.registerOnPreAuth, registerAuth: deps.http.registerAuth, registerOnPostAuth: deps.http.registerOnPostAuth, - getBasePathFor: deps.http.getBasePathFor, - setBasePathFor: deps.http.setBasePathFor, + basePath: deps.http.basePath, createNewServer: deps.http.createNewServer, }, }; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index 43712d63abb53..c0e1ce9028706 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -87,8 +87,7 @@ export interface CoreSetup { registerOnPreAuth: HttpServiceSetup['registerOnPreAuth']; registerAuth: HttpServiceSetup['registerAuth']; registerOnPostAuth: HttpServiceSetup['registerOnPostAuth']; - getBasePathFor: HttpServiceSetup['getBasePathFor']; - setBasePathFor: HttpServiceSetup['setBasePathFor']; + basePath: HttpServiceSetup['basePath']; createNewServer: HttpServiceSetup['createNewServer']; }; } diff --git a/src/core/utils/url.ts b/src/core/utils/url.ts index 1f566b4897d10..67b379e729ca4 100644 --- a/src/core/utils/url.ts +++ b/src/core/utils/url.ts @@ -20,15 +20,20 @@ import { ParsedUrlQuery } from 'querystring'; import { format as formatUrl, parse as parseUrl, UrlObject } from 'url'; +/** + * We define our own typings because the current version of @types/node + * declares properties to be optional "hostname?: string". + * Although, parse call returns "hostname: null | string". + */ export interface URLMeaningfulParts { - auth: string | null; - hash: string | null; - hostname: string | null; - pathname: string | null; - protocol: string | null; - slashes: boolean | null; - port: string | null; - query: ParsedUrlQuery | {}; + auth?: string | null; + hash?: string | null; + hostname?: string | null; + pathname?: string | null; + protocol?: string | null; + slashes?: boolean | null; + port?: string | null; + query: ParsedUrlQuery; } /** @@ -62,7 +67,7 @@ export interface URLMeaningfulParts { */ export function modifyUrl( url: string, - urlModifier: (urlParts: URLMeaningfulParts) => Partial | undefined + urlModifier: (urlParts: URLMeaningfulParts) => Partial | void ) { const parsed = parseUrl(url, true) as URLMeaningfulParts; diff --git a/src/legacy/server/http/setup_base_path_provider.js b/src/legacy/server/http/setup_base_path_provider.js index 8cf6cc1fde512..c4873cb8da8b8 100644 --- a/src/legacy/server/http/setup_base_path_provider.js +++ b/src/legacy/server/http/setup_base_path_provider.js @@ -20,11 +20,11 @@ export function setupBasePathProvider(kbnServer) { kbnServer.server.decorate('request', 'setBasePath', function (basePath) { const request = this; - kbnServer.newPlatform.setup.core.http.setBasePathFor(request, basePath); + kbnServer.newPlatform.setup.core.http.basePath.set(request, basePath); }); kbnServer.server.decorate('request', 'getBasePath', function () { const request = this; - return kbnServer.newPlatform.setup.core.http.getBasePathFor(request); + return kbnServer.newPlatform.setup.core.http.basePath.get(request); }); } diff --git a/src/legacy/ui/public/chrome/api/base_path.test.ts b/src/legacy/ui/public/chrome/api/base_path.test.ts index 448872e87e458..812635ba36483 100644 --- a/src/legacy/ui/public/chrome/api/base_path.test.ts +++ b/src/legacy/ui/public/chrome/api/base_path.test.ts @@ -26,40 +26,40 @@ function initChrome() { return chrome; } -newPlatformHttp.getBasePath.mockImplementation(() => 'gotBasePath'); -newPlatformHttp.prependBasePath.mockImplementation(() => 'addedToPath'); -newPlatformHttp.removeBasePath.mockImplementation(() => 'removedFromPath'); +newPlatformHttp.basePath.get.mockImplementation(() => 'gotBasePath'); +newPlatformHttp.basePath.prepend.mockImplementation(() => 'addedToPath'); +newPlatformHttp.basePath.remove.mockImplementation(() => 'removedFromPath'); beforeEach(() => { jest.clearAllMocks(); }); describe('#getBasePath()', () => { - it('proxies to newPlatformHttp.getBasePath()', () => { + it('proxies to newPlatformHttp.basePath.get()', () => { const chrome = initChrome(); - expect(newPlatformHttp.prependBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.prepend).not.toHaveBeenCalled(); expect(chrome.getBasePath()).toBe('gotBasePath'); - expect(newPlatformHttp.getBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.getBasePath).toHaveBeenCalledWith(); + expect(newPlatformHttp.basePath.get).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.get).toHaveBeenCalledWith(); }); }); describe('#addBasePath()', () => { - it('proxies to newPlatformHttp.prependBasePath(path)', () => { + it('proxies to newPlatformHttp.basePath.prepend(path)', () => { const chrome = initChrome(); - expect(newPlatformHttp.prependBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.prepend).not.toHaveBeenCalled(); expect(chrome.addBasePath('foo/bar')).toBe('addedToPath'); - expect(newPlatformHttp.prependBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.prependBasePath).toHaveBeenCalledWith('foo/bar'); + expect(newPlatformHttp.basePath.prepend).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.prepend).toHaveBeenCalledWith('foo/bar'); }); }); describe('#removeBasePath', () => { - it('proxies to newPlatformBasePath.removeBasePath(path)', () => { + it('proxies to newPlatformBasePath.basePath.remove(path)', () => { const chrome = initChrome(); - expect(newPlatformHttp.removeBasePath).not.toHaveBeenCalled(); + expect(newPlatformHttp.basePath.remove).not.toHaveBeenCalled(); expect(chrome.removeBasePath('foo/bar')).toBe('removedFromPath'); - expect(newPlatformHttp.removeBasePath).toHaveBeenCalledTimes(1); - expect(newPlatformHttp.removeBasePath).toHaveBeenCalledWith('foo/bar'); + expect(newPlatformHttp.basePath.remove).toHaveBeenCalledTimes(1); + expect(newPlatformHttp.basePath.remove).toHaveBeenCalledWith('foo/bar'); }); }); diff --git a/src/legacy/ui/public/chrome/api/base_path.ts b/src/legacy/ui/public/chrome/api/base_path.ts index cb342627f09d5..da823425b7c22 100644 --- a/src/legacy/ui/public/chrome/api/base_path.ts +++ b/src/legacy/ui/public/chrome/api/base_path.ts @@ -22,7 +22,7 @@ import { npSetup } from 'ui/new_platform'; const newPlatformHttp = npSetup.core.http; export function initChromeBasePathApi(chrome: { [key: string]: any }) { - chrome.getBasePath = newPlatformHttp.getBasePath.bind(newPlatformHttp); - chrome.addBasePath = newPlatformHttp.prependBasePath.bind(newPlatformHttp); - chrome.removeBasePath = newPlatformHttp.removeBasePath.bind(newPlatformHttp); + chrome.getBasePath = newPlatformHttp.basePath.get; + chrome.addBasePath = newPlatformHttp.basePath.prepend; + chrome.removeBasePath = newPlatformHttp.basePath.remove; } diff --git a/src/legacy/ui/public/legacy_compat/angular_config.tsx b/src/legacy/ui/public/legacy_compat/angular_config.tsx index d4386a5d13115..1e22003b32833 100644 --- a/src/legacy/ui/public/legacy_compat/angular_config.tsx +++ b/src/legacy/ui/public/legacy_compat/angular_config.tsx @@ -79,7 +79,7 @@ export const configureAppAngularModule = (angularModule: IModule) => { const getEsUrl = (newPlatform: InternalCoreStart) => { const a = document.createElement('a'); - a.href = newPlatform.http.prependBasePath('/elasticsearch'); + a.href = newPlatform.http.basePath.prepend('/elasticsearch'); const protocolPort = /https/.test(a.protocol) ? 443 : 80; const port = a.port || protocolPort; return { diff --git a/src/legacy/ui/public/notify/app_redirect/app_redirect.js b/src/legacy/ui/public/notify/app_redirect/app_redirect.js index d3f66c1eb59ac..a92e5401e5e75 100644 --- a/src/legacy/ui/public/notify/app_redirect/app_redirect.js +++ b/src/legacy/ui/public/notify/app_redirect/app_redirect.js @@ -17,7 +17,7 @@ * under the License. */ -import { modifyUrl } from '../../../../../core/public/utils'; +import { modifyUrl } from '../../../../../core/utils'; import { toastNotifications } from '../toasts'; const APP_REDIRECT_MESSAGE_PARAM = 'app_redirect_message'; diff --git a/src/legacy/ui/public/url/kibana_parsed_url.ts b/src/legacy/ui/public/url/kibana_parsed_url.ts index d431c775d1d2e..93d2e17d6038f 100644 --- a/src/legacy/ui/public/url/kibana_parsed_url.ts +++ b/src/legacy/ui/public/url/kibana_parsed_url.ts @@ -19,7 +19,7 @@ import { parse } from 'url'; -import { modifyUrl } from '../../../../core/public/utils'; +import { modifyUrl } from '../../../../core/utils'; import { prependPath } from './prepend_path'; interface Options {