[Reporting] Config flag to escape formula CSV values (#63645)
* Adds a new xpack.reporting.csv.escapeFormulaValues boolean to auto-escape potentially bad cells * Treat csvs with formulas differently than those that aren't escaped Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
parent
96b3fd1fe3
commit
86922e80a8
|
@ -195,6 +195,7 @@ kibana_vars=(
|
|||
xpack.reporting.capture.viewport.width
|
||||
xpack.reporting.capture.zoom
|
||||
xpack.reporting.csv.checkForFormulas
|
||||
xpack.reporting.csv.escapeFormulaValues
|
||||
xpack.reporting.csv.enablePanelActionDownload
|
||||
xpack.reporting.csv.maxSizeBytes
|
||||
xpack.reporting.csv.scroll.duration
|
||||
|
|
|
@ -20,6 +20,7 @@ export const API_GENERATE_IMMEDIATE = `${API_BASE_URL_V1}/generate/immediate/csv
|
|||
export const CONTENT_TYPE_CSV = 'text/csv';
|
||||
export const CSV_REPORTING_ACTION = 'downloadCsvReport';
|
||||
export const CSV_BOM_CHARS = '\ufeff';
|
||||
export const CSV_FORMULA_CHARS = ['=', '+', '-', '@'];
|
||||
|
||||
export const WHITELISTED_JOB_CONTENT_TYPES = [
|
||||
'application/json',
|
||||
|
|
|
@ -300,7 +300,7 @@ describe('CSV Execute Job', function() {
|
|||
});
|
||||
});
|
||||
|
||||
describe('Cells with formula values', () => {
|
||||
describe('Warning when cells have formulas', () => {
|
||||
it('returns `csv_contains_formulas` when cells contain formulas', async function() {
|
||||
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
|
@ -353,6 +353,7 @@ describe('CSV Execute Job', function() {
|
|||
|
||||
it('returns no warnings when cells have no formulas', async function() {
|
||||
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
|
||||
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
hits: {
|
||||
hits: [{ _source: { one: 'foo', two: 'bar' } }],
|
||||
|
@ -376,6 +377,33 @@ describe('CSV Execute Job', function() {
|
|||
expect(csvContainsFormulas).toEqual(false);
|
||||
});
|
||||
|
||||
it('returns no warnings when cells have formulas but are escaped', async function() {
|
||||
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
|
||||
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
hits: {
|
||||
hits: [{ _source: { '=SUM(A1:A2)': 'foo', two: 'bar' } }],
|
||||
},
|
||||
_scroll_id: 'scrollId',
|
||||
});
|
||||
|
||||
const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
|
||||
const jobParams = getJobDocPayload({
|
||||
headers: encryptedHeaders,
|
||||
fields: ['=SUM(A1:A2)', 'two'],
|
||||
conflictedTypesFields: [],
|
||||
searchRequest: { index: null, body: null },
|
||||
});
|
||||
|
||||
const { csv_contains_formulas: csvContainsFormulas } = await executeJob(
|
||||
'job123',
|
||||
jobParams,
|
||||
cancellationToken
|
||||
);
|
||||
|
||||
expect(csvContainsFormulas).toEqual(false);
|
||||
});
|
||||
|
||||
it('returns no warnings when configured not to', async () => {
|
||||
configGetStub.withArgs('csv', 'checkForFormulas').returns(false);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
|
@ -446,6 +474,50 @@ describe('CSV Execute Job', function() {
|
|||
});
|
||||
});
|
||||
|
||||
describe('Escaping cells with formulas', () => {
|
||||
it('escapes values with formulas', async () => {
|
||||
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
hits: {
|
||||
hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }],
|
||||
},
|
||||
_scroll_id: 'scrollId',
|
||||
});
|
||||
|
||||
const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
|
||||
const jobParams = getJobDocPayload({
|
||||
headers: encryptedHeaders,
|
||||
fields: ['one', 'two'],
|
||||
conflictedTypesFields: [],
|
||||
searchRequest: { index: null, body: null },
|
||||
});
|
||||
const { content } = await executeJob('job123', jobParams, cancellationToken);
|
||||
|
||||
expect(content).toEqual("one,two\n\"'=cmd|' /C calc'!A0\",bar\n");
|
||||
});
|
||||
|
||||
it('does not escapes values with formulas', async () => {
|
||||
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false);
|
||||
callAsCurrentUserStub.onFirstCall().returns({
|
||||
hits: {
|
||||
hits: [{ _source: { one: `=cmd|' /C calc'!A0`, two: 'bar' } }],
|
||||
},
|
||||
_scroll_id: 'scrollId',
|
||||
});
|
||||
|
||||
const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
|
||||
const jobParams = getJobDocPayload({
|
||||
headers: encryptedHeaders,
|
||||
fields: ['one', 'two'],
|
||||
conflictedTypesFields: [],
|
||||
searchRequest: { index: null, body: null },
|
||||
});
|
||||
const { content } = await executeJob('job123', jobParams, cancellationToken);
|
||||
|
||||
expect(content).toEqual('one,two\n"=cmd|\' /C calc\'!A0",bar\n');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Elasticsearch call errors', function() {
|
||||
it('should reject Promise if search call errors out', async function() {
|
||||
callAsCurrentUserStub.rejects(new Error());
|
||||
|
|
|
@ -123,7 +123,7 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
|
|||
const generateCsv = createGenerateCsv(jobLogger);
|
||||
const bom = config.get('csv', 'useByteOrderMarkEncoding') ? CSV_BOM_CHARS : '';
|
||||
|
||||
const { content, maxSizeReached, size, csvContainsFormulas } = await generateCsv({
|
||||
const { content, maxSizeReached, size, csvContainsFormulas, warnings } = await generateCsv({
|
||||
searchRequest,
|
||||
fields,
|
||||
metaFields,
|
||||
|
@ -136,15 +136,18 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
|
|||
checkForFormulas: config.get('csv', 'checkForFormulas'),
|
||||
maxSizeBytes: config.get('csv', 'maxSizeBytes'),
|
||||
scroll: config.get('csv', 'scroll'),
|
||||
escapeFormulaValues: config.get('csv', 'escapeFormulaValues'),
|
||||
},
|
||||
});
|
||||
|
||||
// @TODO: Consolidate these one-off warnings into the warnings array (max-size reached and csv contains formulas)
|
||||
return {
|
||||
content_type: 'text/csv',
|
||||
content: bom + content,
|
||||
max_size_reached: maxSizeReached,
|
||||
size,
|
||||
csv_contains_formulas: csvContainsFormulas,
|
||||
warnings,
|
||||
};
|
||||
};
|
||||
};
|
||||
|
|
|
@ -0,0 +1,11 @@
|
|||
/*
|
||||
* 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 { startsWith } from 'lodash';
|
||||
import { CSV_FORMULA_CHARS } from '../../../../common/constants';
|
||||
|
||||
export const cellHasFormulas = (val: string) =>
|
||||
CSV_FORMULA_CHARS.some(formulaChar => startsWith(val, formulaChar));
|
|
@ -5,8 +5,7 @@
|
|||
*/
|
||||
|
||||
import * as _ from 'lodash';
|
||||
|
||||
const formulaValues = ['=', '+', '-', '@'];
|
||||
import { cellHasFormulas } from './cell_has_formula';
|
||||
|
||||
interface IFlattened {
|
||||
[header: string]: string;
|
||||
|
@ -14,7 +13,7 @@ interface IFlattened {
|
|||
|
||||
export const checkIfRowsHaveFormulas = (flattened: IFlattened, fields: string[]) => {
|
||||
const pruned = _.pick(flattened, fields);
|
||||
const csvValues = [..._.keys(pruned), ...(_.values(pruned) as string[])];
|
||||
const cells = [..._.keys(pruned), ...(_.values(pruned) as string[])];
|
||||
|
||||
return _.some(csvValues, cell => _.some(formulaValues, char => _.startsWith(cell, char)));
|
||||
return _.some(cells, cell => cellHasFormulas(cell));
|
||||
};
|
||||
|
|
|
@ -11,7 +11,7 @@ describe('escapeValue', function() {
|
|||
describe('quoteValues is true', function() {
|
||||
let escapeValue: (val: string) => string;
|
||||
beforeEach(function() {
|
||||
escapeValue = createEscapeValue(true);
|
||||
escapeValue = createEscapeValue(true, false);
|
||||
});
|
||||
|
||||
it('should escape value with spaces', function() {
|
||||
|
@ -46,7 +46,7 @@ describe('escapeValue', function() {
|
|||
describe('quoteValues is false', function() {
|
||||
let escapeValue: (val: string) => string;
|
||||
beforeEach(function() {
|
||||
escapeValue = createEscapeValue(false);
|
||||
escapeValue = createEscapeValue(false, false);
|
||||
});
|
||||
|
||||
it('should return the value unescaped', function() {
|
||||
|
@ -54,4 +54,34 @@ describe('escapeValue', function() {
|
|||
expect(escapeValue(value)).to.be(value);
|
||||
});
|
||||
});
|
||||
|
||||
describe('escapeValues', () => {
|
||||
describe('when true', () => {
|
||||
let escapeValue: (val: string) => string;
|
||||
beforeEach(function() {
|
||||
escapeValue = createEscapeValue(true, true);
|
||||
});
|
||||
|
||||
['@', '+', '-', '='].forEach(badChar => {
|
||||
it(`should escape ${badChar} injection values`, function() {
|
||||
expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be(
|
||||
`"'${badChar}cmd|' /C calc'!A0"`
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('when false', () => {
|
||||
let escapeValue: (val: string) => string;
|
||||
beforeEach(function() {
|
||||
escapeValue = createEscapeValue(true, false);
|
||||
});
|
||||
|
||||
['@', '+', '-', '='].forEach(badChar => {
|
||||
it(`should not escape ${badChar} injection values`, function() {
|
||||
expect(escapeValue(`${badChar}cmd|' /C calc'!A0`)).to.be(`"${badChar}cmd|' /C calc'!A0"`);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -5,15 +5,20 @@
|
|||
*/
|
||||
|
||||
import { RawValue } from './types';
|
||||
import { cellHasFormulas } from './cell_has_formula';
|
||||
|
||||
const nonAlphaNumRE = /[^a-zA-Z0-9]/;
|
||||
const allDoubleQuoteRE = /"/g;
|
||||
|
||||
export function createEscapeValue(quoteValues: boolean): (val: RawValue) => string {
|
||||
export function createEscapeValue(
|
||||
quoteValues: boolean,
|
||||
escapeFormulas: boolean
|
||||
): (val: RawValue) => string {
|
||||
return function escapeValue(val: RawValue) {
|
||||
if (val && typeof val === 'string') {
|
||||
if (quoteValues && nonAlphaNumRE.test(val)) {
|
||||
return `"${val.replace(allDoubleQuoteRE, '""')}"`;
|
||||
const formulasEscaped = escapeFormulas && cellHasFormulas(val) ? "'" + val : val;
|
||||
if (quoteValues && nonAlphaNumRE.test(formulasEscaped)) {
|
||||
return `"${formulasEscaped.replace(allDoubleQuoteRE, '""')}"`;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -4,6 +4,7 @@
|
|||
* you may not use this file except in compliance with the Elastic License.
|
||||
*/
|
||||
|
||||
import { i18n } from '@kbn/i18n';
|
||||
import { Logger } from '../../../../types';
|
||||
import { GenerateCsvParams, SavedSearchGeneratorResult } from '../../types';
|
||||
import { createFlattenHit } from './flatten_hit';
|
||||
|
@ -26,14 +27,17 @@ export function createGenerateCsv(logger: Logger) {
|
|||
cancellationToken,
|
||||
settings,
|
||||
}: GenerateCsvParams): Promise<SavedSearchGeneratorResult> {
|
||||
const escapeValue = createEscapeValue(settings.quoteValues);
|
||||
const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues);
|
||||
const builder = new MaxSizeStringBuilder(settings.maxSizeBytes);
|
||||
const header = `${fields.map(escapeValue).join(settings.separator)}\n`;
|
||||
const warnings: string[] = [];
|
||||
|
||||
if (!builder.tryAppend(header)) {
|
||||
return {
|
||||
size: 0,
|
||||
content: '',
|
||||
maxSizeReached: true,
|
||||
warnings: [],
|
||||
};
|
||||
}
|
||||
|
||||
|
@ -82,11 +86,20 @@ export function createGenerateCsv(logger: Logger) {
|
|||
const size = builder.getSizeInBytes();
|
||||
logger.debug(`finished generating, total size in bytes: ${size}`);
|
||||
|
||||
if (csvContainsFormulas && settings.escapeFormulaValues) {
|
||||
warnings.push(
|
||||
i18n.translate('xpack.reporting.exportTypes.csv.generateCsv.escapedFormulaValues', {
|
||||
defaultMessage: 'CSV may contain formulas whose values have been escaped',
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
content: builder.getString(),
|
||||
csvContainsFormulas,
|
||||
csvContainsFormulas: csvContainsFormulas && !settings.escapeFormulaValues,
|
||||
maxSizeReached,
|
||||
size,
|
||||
warnings,
|
||||
};
|
||||
};
|
||||
}
|
||||
|
|
|
@ -87,6 +87,7 @@ export interface SavedSearchGeneratorResult {
|
|||
size: number;
|
||||
maxSizeReached: boolean;
|
||||
csvContainsFormulas?: boolean;
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
export interface CsvResultFromSearch {
|
||||
|
@ -109,5 +110,6 @@ export interface GenerateCsvParams {
|
|||
maxSizeBytes: number;
|
||||
scroll: ScrollConfig;
|
||||
checkForFormulas?: boolean;
|
||||
escapeFormulaValues: boolean;
|
||||
};
|
||||
}
|
||||
|
|
|
@ -173,6 +173,7 @@ export async function generateCsvSearch(
|
|||
...uiSettings,
|
||||
maxSizeBytes: config.get('csv', 'maxSizeBytes'),
|
||||
scroll: config.get('csv', 'scroll'),
|
||||
escapeFormulaValues: config.get('csv', 'escapeFormulaValues'),
|
||||
timezone,
|
||||
},
|
||||
};
|
||||
|
|
|
@ -114,6 +114,7 @@ const CaptureSchema = schema.object({
|
|||
|
||||
const CsvSchema = schema.object({
|
||||
checkForFormulas: schema.boolean({ defaultValue: true }),
|
||||
escapeFormulaValues: schema.boolean({ defaultValue: false }),
|
||||
enablePanelActionDownload: schema.boolean({ defaultValue: true }),
|
||||
maxSizeBytes: schema.number({
|
||||
defaultValue: 1024 * 1024 * 10, // 10MB
|
||||
|
|
Loading…
Reference in a new issue