From 400878363bae1c633b7ebfb955b53dd27e150c6a Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Wed, 25 Sep 2019 09:23:52 -0700 Subject: [PATCH] [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 --- .../workpad_header/workpad_export/index.ts | 3 +- .../workpad_header/workpad_export/utils.ts | 22 ++++-- .../reporting/public/lib/reporting_client.ts | 12 ++- ...generate.ts => generate_from_jobparams.ts} | 43 +++++++++-- .../plugins/reporting/server/routes/index.ts | 4 +- .../reporting/api/generate/csv_job_params.ts | 74 +++++++++++++++++++ .../test/reporting/api/generate/fixtures.ts | 16 ++++ x-pack/test/reporting/api/generate/index.js | 1 + 8 files changed, 155 insertions(+), 20 deletions(-) rename x-pack/legacy/plugins/reporting/server/routes/{generate.ts => generate_from_jobparams.ts} (54%) create mode 100644 x-pack/test/reporting/api/generate/csv_job_params.ts 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 index 7f87231d8ae303..7eca74381d9563 100644 --- 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( 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 index 99e50ee43eb109..b06bf5a74b528a 100644 --- 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 index 8481d2993ffb34..b9574dfa457f3b 100644 --- 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/x-pack/legacy/plugins/reporting/server/routes/generate.ts b/x-pack/legacy/plugins/reporting/server/routes/generate_from_jobparams.ts similarity index 54% rename from x-pack/legacy/plugins/reporting/server/routes/generate.ts rename to x-pack/legacy/plugins/reporting/server/routes/generate_from_jobparams.ts index c1a3e39d21e9c2..9dc1e9d12e308b 100644 --- a/x-pack/legacy/plugins/reporting/server/routes/generate.ts +++ b/x-pack/legacy/plugins/reporting/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 index cef2137352fd32..cbfd096e7100bb 100644 --- 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 new file mode 100644 index 00000000000000..c8d6f11b74f9dd --- /dev/null +++ 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 index b52ef11f0f35f9..ba0930165a1fc4 100644 --- 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 index 99286762f44f10..eeeef2b43f967a 100644 --- 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')); }); }