[Reporting] Fix Generating Reports with long jobParams RISON (#45603)…

… (#46404) * Rename route file for consistency * Generate via jobParams allows post payload * jobParams as post body * [Reporting] Skip failing test * fix types * canvas pdf generation to use post payload * api test * tests * fix a typescript * fix fn test * fix tests * fix tests * run esArchiver fewer times * return rison that came in invalid
elastic · Sep 25, 2019 · 4008783 · 4008783
1 parent dfe1aa0
commit 4008783
Show file tree

Hide file tree

Showing 8 changed files with 155 additions and 20 deletions.
diff --git a/x-pack/legacy/plugins/canvas/public/components/workpad_header/workpad_export/index.ts b/x-pack/legacy/plugins/canvas/public/components/workpad_header/workpad_export/index.ts
@@ -53,7 +53,8 @@ export const WorkpadExport = compose<ComponentProps, {}>(
       enabled,
       getExportUrl: type => {
         if (type === 'pdf') {
-          return getAbsoluteUrl(getPdfUrl(workpad, { pageCount }));
+          const { createPdfUri } = getPdfUrl(workpad, { pageCount });
+          return getAbsoluteUrl(createPdfUri);
         }
 
         throw new Error(strings.getUnknownExportErrorMessage(type));

diff --git a/x-pack/legacy/plugins/canvas/public/components/workpad_header/workpad_export/utils.ts b/x-pack/legacy/plugins/canvas/public/components/workpad_header/workpad_export/utils.ts
@@ -6,7 +6,6 @@
 
 import rison from 'rison-node';
 import chrome from 'ui/chrome';
-import { QueryString } from 'ui/utils/query_string';
 // @ts-ignore Untyped local.
 import { fetch } from '../../../../common/lib/fetch';
 import { CanvasWorkpad } from '../../../../types';
@@ -20,10 +19,15 @@ interface PageCount {
 
 type Arguments = [CanvasWorkpad, PageCount];
 
+interface PdfUrlData {
+  createPdfUri: string;
+  createPdfPayload: { jobParams: string };
+}
+
 export function getPdfUrl(
   { id, name: title, width, height }: CanvasWorkpad,
   { pageCount }: PageCount
-) {
+): PdfUrlData {
   const reportingEntry = chrome.addBasePath('/api/reporting/generate');
   const canvasEntry = '/app/canvas#';
 
@@ -54,13 +58,15 @@ export function getPdfUrl(
     title,
   };
 
-  return `${reportingEntry}/printablePdf?${QueryString.param(
-    'jobParams',
-    rison.encode(jobParams)
-  )}`;
+  return {
+    createPdfUri: `${reportingEntry}/printablePdf`,
+    createPdfPayload: {
+      jobParams: rison.encode(jobParams),
+    },
+  };
 }
 
 export function createPdf(...args: Arguments) {
-  const createPdfUri = getPdfUrl(...args);
-  return fetch.post(createPdfUri);
+  const { createPdfUri, createPdfPayload } = getPdfUrl(...args);
+  return fetch.post(createPdfUri, createPdfPayload);
 }
diff --git a/x-pack/legacy/plugins/reporting/public/lib/reporting_client.ts b/x-pack/legacy/plugins/reporting/public/lib/reporting_client.ts
@@ -27,10 +27,14 @@ class ReportingClient {
   };
 
   public createReportingJob = async (exportType: string, jobParams: any) => {
-    const query = {
-      jobParams: rison.encode(jobParams),
-    };
-    const resp = await kfetch({ method: 'POST', pathname: `${API_BASE_URL}/${exportType}`, query });
+    const jobParamsRison = rison.encode(jobParams);
+    const resp = await kfetch({
+      method: 'POST',
+      pathname: `${API_BASE_URL}/${exportType}`,
+      body: JSON.stringify({
+        jobParams: jobParamsRison,
+      }),
+    });
     jobCompletionNotifications.add(resp.job.id);
     return resp;
   };

diff --git a/...ugins/reporting/server/routes/generate.ts → .../server/routes/generate_from_jobparams.ts b/...ugins/reporting/server/routes/generate.ts → .../server/routes/generate_from_jobparams.ts
@@ -5,6 +5,7 @@
  */
 
 import boom from 'boom';
+import Joi from 'joi';
 import { Request, ResponseToolkit } from 'hapi';
 import rison from 'rison-node';
 import { API_BASE_URL } from '../../common/constants';
@@ -14,7 +15,7 @@ import { HandlerErrorFunction, HandlerFunction } from './types';
 
 const BASE_GENERATE = `${API_BASE_URL}/generate`;
 
-export function registerGenerate(
+export function registerGenerateFromJobParams(
   server: KbnServer,
   handler: HandlerFunction,
   handleError: HandlerErrorFunction
@@ -25,16 +26,48 @@ export function registerGenerate(
   server.route({
     path: `${BASE_GENERATE}/{exportType}`,
     method: 'POST',
-    config: getRouteConfig(request => request.params.exportType),
+    config: {
+      ...getRouteConfig(request => request.params.exportType),
+      validate: {
+        params: Joi.object({
+          exportType: Joi.string().required(),
+        }).required(),
+        payload: Joi.object({
+          jobParams: Joi.string()
+            .optional()
+            .default(null),
+        }).allow(null), // allow optional payload
+        query: Joi.object({
+          jobParams: Joi.string().default(null),
+        }).default(),
+      },
+    },
     handler: async (request: Request, h: ResponseToolkit) => {
+      let jobParamsRison: string | null;
+
+      if (request.payload) {
+        const { jobParams: jobParamsPayload } = request.payload as { jobParams: string };
+        jobParamsRison = jobParamsPayload;
+      } else {
+        const { jobParams: queryJobParams } = request.query as { jobParams: string };
+        if (queryJobParams) {
+          jobParamsRison = queryJobParams;
+        } else {
+          jobParamsRison = null;
+        }
+      }
+
+      if (!jobParamsRison) {
+        throw boom.badRequest('A jobParams RISON string is required');
+      }
+
       const { exportType } = request.params;
       let response;
       try {
-        // @ts-ignore
-        const jobParams = rison.decode(request.query.jobParams);
+        const jobParams = rison.decode(jobParamsRison);
         response = await handler(exportType, jobParams, request, h);
       } catch (err) {
-        throw handleError(exportType, err);
+        throw boom.badRequest(`invalid rison: ${jobParamsRison}`);
       }
       return response;
     },

diff --git a/x-pack/legacy/plugins/reporting/server/routes/index.ts b/x-pack/legacy/plugins/reporting/server/routes/index.ts
@@ -9,7 +9,7 @@ import { Request, ResponseToolkit } from 'hapi';
 import { API_BASE_URL } from '../../common/constants';
 import { KbnServer, Logger } from '../../types';
 import { enqueueJobFactory } from '../lib/enqueue_job';
-import { registerGenerate } from './generate';
+import { registerGenerateFromJobParams } from './generate_from_jobparams';
 import { registerGenerateCsvFromSavedObject } from './generate_from_savedobject';
 import { registerGenerateCsvFromSavedObjectImmediate } from './generate_from_savedobject_immediate';
 import { registerJobs } from './jobs';
@@ -60,7 +60,7 @@ export function registerRoutes(server: KbnServer, logger: Logger) {
     return err;
   }
 
-  registerGenerate(server, handler, handleError);
+  registerGenerateFromJobParams(server, handler, handleError);
   registerLegacy(server, handler, handleError);
 
   // Register beta panel-action download-related API's

diff --git a/x-pack/test/reporting/api/generate/csv_job_params.ts b/x-pack/test/reporting/api/generate/csv_job_params.ts
@@ -0,0 +1,74 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import expect from '@kbn/expect';
+import supertest from 'supertest';
+import { JOB_PARAMS_RISON } from './fixtures';
+
+// eslint-disable-next-line import/no-default-export
+export default function({ getService }: { getService: any }) {
+  const esArchiver = getService('esArchiver');
+  const supertestSvc = getService('supertest');
+  const generateAPI = {
+    getCsvFromParamsInPayload: async (jobParams: object = {}) => {
+      return await supertestSvc
+        .post(`/api/reporting/generate/csv`)
+        .set('kbn-xsrf', 'xxx')
+        .send(jobParams);
+    },
+    getCsvFromParamsInQueryString: async (jobParams: string = '') => {
+      return await supertestSvc
+        .post(`/api/reporting/generate/csv?jobParams=${encodeURIComponent(jobParams)}`)
+        .set('kbn-xsrf', 'xxx');
+    },
+  };
+
+  describe('Generation from Job Params', () => {
+    before(async () => {
+      await esArchiver.load('reporting/logs');
+      await esArchiver.load('logstash_functional');
+    }); // prettier-ignore
+    after(async () => {
+      await esArchiver.unload('reporting/logs');
+      await esArchiver.unload('logstash_functional');
+    }); // prettier-ignore
+
+    it('Rejects bogus jobParams', async () => {
+      const { status: resStatus, text: resText } = (await generateAPI.getCsvFromParamsInPayload({
+        jobParams: 0,
+      })) as supertest.Response;
+
+      expect(resText).to.match(/\\\"jobParams\\\" must be a string/);
+      expect(resStatus).to.eql(400);
+    });
+
+    it('Rejects empty jobParams', async () => {
+      const {
+        status: resStatus,
+        text: resText,
+      } = (await generateAPI.getCsvFromParamsInPayload()) as supertest.Response;
+
+      expect(resStatus).to.eql(400);
+      expect(resText).to.match(/jobParams RISON string is required/);
+    });
+
+    it('Accepts jobParams in POST payload', async () => {
+      const { status: resStatus, text: resText } = (await generateAPI.getCsvFromParamsInPayload({
+        jobParams: JOB_PARAMS_RISON,
+      })) as supertest.Response;
+      expect(resText).to.match(/"jobtype":"csv"/);
+      expect(resStatus).to.eql(200);
+    });
+
+    it('Accepts jobParams in query string', async () => {
+      const { status: resStatus, text: resText } = (await generateAPI.getCsvFromParamsInQueryString(
+        JOB_PARAMS_RISON
+      )) as supertest.Response;
+      expect(resText).to.match(/"jobtype":"csv"/);
+      expect(resStatus).to.eql(200);
+    });
+  });
+}
diff --git a/x-pack/test/reporting/api/generate/fixtures.ts b/x-pack/test/reporting/api/generate/fixtures.ts
@@ -178,3 +178,19 @@ export const CSV_RESULT_NANOS = `date,message,"_id"
 "2015-01-01T12:10:30.123456789Z","Hello 2",
 "2015-01-01T12:10:30","Hello 1",
 `;
+
+// This concatenates lines of multi-line string into a single line.
+// It is so long strings can be entered at short widths, making syntax highlighting easier on editors
+function singleLine(literals: TemplateStringsArray): string {
+  return literals[0].split('\n').join('');
+}
+
+export const JOB_PARAMS_RISON = singleLine`(conflictedTypesFields:!(),fields:!('@ti
+mestamp',clientip,extension),indexPatternId:'logstash-*',metaFields:!(_source,_id,_type,_
+index,_score),searchRequest:(body:(_source:(excludes:!(),includes:!('@timestamp',clientip
+,extension)),docvalue_fields:!(),query:(bool:(filter:!((match_all:()),(range:('@timestamp
+':(gte:'2015-09-20T10:19:40.307Z',lt:'2015-09-20T10:26:56.221Z'))),(range:('@timestamp':(
+format:strict_date_optional_time,gte:'2004-09-17T21:19:34.213Z',lte:'2019-09-17T21:19:34.
+213Z')))),must:!(),must_not:!(),should:!())),script_fields:(),sort:!(('@timestamp':(order
+:desc,unmapped_type:boolean))),stored_fields:!('@timestamp',clientip,extension),version:!
+t),index:'logstash-*'),title:'A Saved Search With a DATE FILTER',type:search)`;
diff --git a/x-pack/test/reporting/api/generate/index.js b/x-pack/test/reporting/api/generate/index.js
@@ -8,5 +8,6 @@ export default function ({ loadTestFile }) {
   describe('CSV', function () {
     this.tags('ciGroup2');
     loadTestFile(require.resolve('./csv_saved_search'));
+    loadTestFile(require.resolve('./csv_job_params'));
   });
 }