Update KibanaRequest to use the new WHATWG URL API

docs/development/core/server/kibana-plugin-core-server.kibanarequest.md

@@ -30,9 +30,9 @@ export declare class KibanaRequest<Params = unknown, Query = unknown, Body = unk
 |  [isSystemRequest](./kibana-plugin-core-server.kibanarequest.issystemrequest.md) |  | <code>boolean</code> | Whether or not the request is a "system request" rather than an application-level request. Can be set on the client using the <code>HttpFetchOptions#asSystemRequest</code> option. |
 |  [params](./kibana-plugin-core-server.kibanarequest.params.md) |  | <code>Params</code> |  |
 |  [query](./kibana-plugin-core-server.kibanarequest.query.md) |  | <code>Query</code> |  |
-|  [rewrittenUrl](./kibana-plugin-core-server.kibanarequest.rewrittenurl.md) |  | <code>Url</code> | URL rewritten in onPreRouting request interceptor. |
+|  [rewrittenUrl](./kibana-plugin-core-server.kibanarequest.rewrittenurl.md) |  | <code>URL</code> | URL rewritten in onPreRouting request interceptor. |
 |  [route](./kibana-plugin-core-server.kibanarequest.route.md) |  | <code>RecursiveReadonly&lt;KibanaRequestRoute&lt;Method&gt;&gt;</code> | matched route details |
 |  [socket](./kibana-plugin-core-server.kibanarequest.socket.md) |  | <code>IKibanaSocket</code> | [IKibanaSocket](./kibana-plugin-core-server.ikibanasocket.md) |
-|  [url](./kibana-plugin-core-server.kibanarequest.url.md) |  | <code>Url</code> | a WHATWG URL standard object. |
+|  [url](./kibana-plugin-core-server.kibanarequest.url.md) |  | <code>URL</code> | a WHATWG URL standard object. |
 |  [uuid](./kibana-plugin-core-server.kibanarequest.uuid.md) |  | <code>string</code> | A UUID to identify this request. |
 

docs/development/core/server/kibana-plugin-core-server.kibanarequest.rewrittenurl.md

@@ -9,5 +9,5 @@ URL rewritten in onPreRouting request interceptor.
 <b>Signature:</b>
 
 ```typescript
-readonly rewrittenUrl?: Url;
+readonly rewrittenUrl?: URL;
 ```

docs/development/core/server/kibana-plugin-core-server.kibanarequest.url.md

@@ -9,5 +9,5 @@ a WHATWG URL standard object.
 <b>Signature:</b>
 
 ```typescript
-readonly url: Url;
+readonly url: URL;
 ```

src/core/server/http/http_server.mocks.ts

@@ -16,6 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { parse as parseUrl } from 'url';
 import { Request } from 'hapi';
 import { merge } from 'lodash';
 import { Socket } from 'net';
@@ -72,6 +73,7 @@ function createKibanaRequestMock<P = any, Q = any, B = any>({
   auth = { isAuthenticated: true },
 }: RequestFixtureOptions<P, Q, B> = {}) {
   const queryString = stringify(query, { sort: false });
+  const url = parseUrl(`${path}${queryString ? `?${queryString}` : ''}`);
 
   return KibanaRequest.from<P, Q, B>(
     createRawRequestMock({
@@ -83,12 +85,7 @@ function createKibanaRequestMock<P = any, Q = any, B = any>({
       payload: body,
       path,
       method,
-      url: {
-        path,
-        pathname: path,
-        query: queryString,
-        search: queryString ? `?${queryString}` : queryString,
-      },
+      url,
       route: {
         settings: { tags: routeTags, auth: routeAuthRequired, app: kibanaRouteOptions },
       },
@@ -121,6 +118,11 @@ interface DeepPartialArray<T> extends Array<DeepPartial<T>> {}
 type DeepPartialObject<T> = { [P in keyof T]+?: DeepPartial<T[P]> };
 
 function createRawRequestMock(customization: DeepPartial<Request> = {}) {
+  const pathname = customization.url?.pathname || '/';
+  const path = `${pathname}${customization.url?.search || ''}`;
+  const url = Object.assign({ pathname, path, href: path }, customization.url);
+
+  // @ts-expect-error _core isn't supposed to be accessed - remove once we upgrade to hapi v18
   return merge(
     {},
     {
@@ -129,17 +131,21 @@ function createRawRequestMock(customization: DeepPartial<Request> = {}) {
         isAuthenticated: true,
       },
       headers: {},
-      path: '/',
+      path,
       route: { settings: {} },
-      url: {
-        href: '/',
-      },
+      url,
       raw: {
         req: {
-          url: '/',
+          url: path,
           socket: {},
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     },
     customization
   ) as Request;

src/core/server/http/http_server.ts

@@ -271,7 +271,7 @@ export class HttpServer {
     }
 
     this.registerOnPreRouting((request, response, toolkit) => {
-      const oldUrl = request.url.href!;
+      const oldUrl = request.url.pathname + request.url.search;
       const newURL = basePathService.remove(oldUrl);
       const shouldRedirect = newURL !== oldUrl;
       if (shouldRedirect) {

src/core/server/http/integration_tests/lifecycle.test.ts

@@ -124,7 +124,13 @@ describe('OnPreRouting', () => {
     const router = createRouter('/');
 
     router.get({ path: '/login', validate: false }, (context, req, res) => {
-      return res.ok({ body: { rewrittenUrl: req.rewrittenUrl?.path } });
+      return res.ok({
+        body: {
+          rewrittenUrl: req.rewrittenUrl
+            ? `${req.rewrittenUrl.pathname}${req.rewrittenUrl.search}`
+            : undefined,
+        },
+      });
     });
 
     registerOnPreRouting((req, res, t) => t.rewriteUrl('/login'));
@@ -143,7 +149,13 @@ describe('OnPreRouting', () => {
     const router = createRouter('/');
 
     router.get({ path: '/reroute-2', validate: false }, (context, req, res) => {
-      return res.ok({ body: { rewrittenUrl: req.rewrittenUrl?.path } });
+      return res.ok({
+        body: {
+          rewrittenUrl: req.rewrittenUrl
+            ? `${req.rewrittenUrl.pathname}${req.rewrittenUrl.search}`
+            : undefined,
+        },
+      });
     });
 
     registerOnPreRouting((req, res, t) => t.rewriteUrl('/reroute-1'));
@@ -163,7 +175,13 @@ describe('OnPreRouting', () => {
     const router = createRouter('/');
 
     router.get({ path: '/login', validate: false }, (context, req, res) => {
-      return res.ok({ body: { rewrittenUrl: req.rewrittenUrl?.path } });
+      return res.ok({
+        body: {
+          rewrittenUrl: req.rewrittenUrl
+            ? `${req.rewrittenUrl.pathname}${req.rewrittenUrl.search}`
+            : undefined,
+        },
+      });
     });
 
     registerOnPreRouting((req, res, t) => t.next());

src/core/server/http/lifecycle/on_pre_routing.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { URL } from 'url';
 import { Lifecycle, Request, ResponseToolkit as HapiResponseToolkit } from 'hapi';
 import { Logger } from '../../logging';
 import {
@@ -110,10 +111,30 @@ export function adoptToHapiOnRequest(fn: OnPreRoutingHandler, log: Logger) {
 
       if (preRoutingResult.isRewriteUrl(result)) {
         const appState = request.app as KibanaRequestState;
-        appState.rewrittenUrl = appState.rewrittenUrl ?? request.url;
+        appState.rewrittenUrl =
+          // @ts-expect-error request._core isn't supposed to be accessed - remove once we upgrade to hapi v18
+          appState.rewrittenUrl ?? new URL(request.url.href!, request._core.info.uri);
 
         const { url } = result;
-        request.setUrl(url);
+
+        // TODO: Remove once we upgrade to Node.js 12!
+        //
+        // Warning: The following for-loop took 10 days to write, and is a hack
+        // to force V8 to make a copy of the string in memory.
+        //
+        // The reason why we need this is because of what appears to be a bug
+        // in V8 that caused some URL paths to not be routed correctly once
+        // `request.setUrl` was called with the path.
+        //
+        // The details can be seen in this discussion on Twitter:
+        // https://twitter.com/wa7son/status/1319992632366518277
+        let urlCopy = '';
+        for (let i = 0; i < url.length; i++) {
+          urlCopy += url[i];
+        }
+
+        request.setUrl(urlCopy);
+
         // We should update raw request as well since it can be proxied to the old platform
         request.raw.req.url = url;
         return responseToolkit.continue;

src/core/server/http/router/request.ts

@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { Url } from 'url';
+import { URL } from 'url';
 import uuid from 'uuid';
 import { Request, RouteOptionsApp, ApplicationState } from 'hapi';
 import { Observable, fromEvent, merge } from 'rxjs';
@@ -45,7 +45,7 @@ export interface KibanaRouteOptions extends RouteOptionsApp {
 export interface KibanaRequestState extends ApplicationState {
   requestId: string;
   requestUuid: string;
-  rewrittenUrl?: Url;
+  rewrittenUrl?: URL;
 }
 
 /**
@@ -163,7 +163,7 @@ export class KibanaRequest<
    */
   public readonly uuid: string;
   /** a WHATWG URL standard object. */
-  public readonly url: Url;
+  public readonly url: URL;
   /** matched route details */
   public readonly route: RecursiveReadonly<KibanaRequestRoute<Method>>;
   /**
@@ -190,7 +190,7 @@ export class KibanaRequest<
   /**
    * URL rewritten in onPreRouting request interceptor.
    */
-  public readonly rewrittenUrl?: Url;
+  public readonly rewrittenUrl?: URL;
 
   /** @internal */
   protected readonly [requestSymbol]: Request;
@@ -212,7 +212,8 @@ export class KibanaRequest<
     this.uuid = appState?.requestUuid ?? uuid.v4();
     this.rewrittenUrl = appState?.rewrittenUrl;
 
-    this.url = request.url;
+    // @ts-expect-error request._core isn't supposed to be accessed - remove once we upgrade to hapi v18
+    this.url = new URL(request.url.href!, request._core.info.uri);
     this.headers = deepFreeze({ ...request.headers });
     this.isSystemRequest =
       request.headers['kbn-system-request'] === 'true' ||
@@ -304,8 +305,8 @@ export class KibanaRequest<
     if (authOptions === false) return false;
     throw new Error(
       `unexpected authentication options: ${JSON.stringify(authOptions)} for route: ${
-        this.url.href
-      }`
+        this.url.pathname
+      }${this.url.search}`
     );
   }
 }

src/core/server/server.api.md

@@ -162,7 +162,7 @@ import { Type } from '@kbn/config-schema';
 import { TypeOf } from '@kbn/config-schema';
 import { UpdateDocumentByQueryParams } from 'elasticsearch';
 import { UpdateDocumentParams } from 'elasticsearch';
-import { Url } from 'url';
+import { URL } from 'url';
 
 // @public
 export interface AppCategory {
@@ -1007,11 +1007,11 @@ export class KibanaRequest<Params = unknown, Query = unknown, Body = unknown, Me
     readonly params: Params;
     // (undocumented)
     readonly query: Query;
-    readonly rewrittenUrl?: Url;
+    readonly rewrittenUrl?: URL;
     readonly route: RecursiveReadonly<KibanaRequestRoute<Method>>;
     // (undocumented)
     readonly socket: IKibanaSocket;
-    readonly url: Url;
+    readonly url: URL;
     readonly uuid: string;
     }
 

x-pack/plugins/actions/server/lib/task_runner_factory.test.ts

@@ -145,6 +145,12 @@ test('executes the task by calling the executor with proper parameters', async (
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     },
   });
 });
@@ -271,6 +277,12 @@ test('uses API key when provided', async () => {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     },
   });
 });
@@ -310,6 +322,12 @@ test(`doesn't use API key when not provided`, async () => {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     },
   });
 });

x-pack/plugins/actions/server/lib/task_runner_factory.ts

@@ -102,6 +102,12 @@ export class TaskRunnerFactory {
               url: '/',
             },
           },
+          // TODO: Remove once we upgrade to hapi v18
+          _core: {
+            info: {
+              uri: 'http://localhost',
+            },
+          },
         } as unknown) as KibanaRequest;
 
         let executorResult: ActionTypeExecutorResult<unknown>;

x-pack/plugins/alerts/server/alerts_client_factory.test.ts

@@ -60,6 +60,12 @@ const fakeRequest = ({
       url: '/',
     },
   },
+  // TODO: Remove once we upgrade to hapi v18
+  _core: {
+    info: {
+      uri: 'http://localhost',
+    },
+  },
   getSavedObjectsClient: () => savedObjectsClient,
 } as unknown) as Request;
 

x-pack/plugins/alerts/server/plugin.test.ts

@@ -149,6 +149,12 @@ describe('Alerting Plugin', () => {
               url: '/',
             },
           },
+          // TODO: Remove once we upgrade to hapi v18
+          _core: {
+            info: {
+              uri: 'http://localhost',
+            },
+          },
           getSavedObjectsClient: jest.fn(),
         } as unknown) as KibanaRequest;
         await startContract.getAlertsClientWithRequest(fakeRequest);

x-pack/plugins/alerts/server/task_runner/task_runner.test.ts

@@ -364,6 +364,12 @@ describe('Task Runner', () => {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     });
     expect(actionsClient.enqueueExecution).toHaveBeenCalledTimes(1);
     expect(actionsClient.enqueueExecution.mock.calls[0]).toMatchInlineSnapshot(`
@@ -662,6 +668,12 @@ describe('Task Runner', () => {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     });
   });
 
@@ -694,6 +706,12 @@ describe('Task Runner', () => {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     });
   });
 

x-pack/plugins/alerts/server/task_runner/task_runner.ts

@@ -101,6 +101,12 @@ export class TaskRunner {
           url: '/',
         },
       },
+      // TODO: Remove once we upgrade to hapi v18
+      _core: {
+        info: {
+          uri: 'http://localhost',
+        },
+      },
     } as unknown) as KibanaRequest;
   }