Skip to content

Commit c45b8a6

Browse files
author
Joel Griffith
committed
Treat csvs with formulas differently than those that aren't escaped
1 parent b49c39c commit c45b8a6

8 files changed

Lines changed: 65 additions & 13 deletions

File tree

‎src/dev/build/tasks/os_packages/docker_generator/resources/bin/kibana-docker‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ kibana_vars=(
195195
xpack.reporting.capture.viewport.width
196196
xpack.reporting.capture.zoom
197197
xpack.reporting.csv.checkForFormulas
198+
xpack.reporting.csv.escapeFormulaValues
198199
xpack.reporting.csv.enablePanelActionDownload
199200
xpack.reporting.csv.maxSizeBytes
200201
xpack.reporting.csv.scroll.duration

‎x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.test.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ describe('CSV Execute Job', function() {
300300
});
301301
});
302302

303-
describe('Cells with formula values', () => {
303+
describe('Warning when cells have formulas', () => {
304304
it('returns `csv_contains_formulas` when cells contain formulas', async function() {
305305
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
306306
callAsCurrentUserStub.onFirstCall().returns({
@@ -353,6 +353,7 @@ describe('CSV Execute Job', function() {
353353

354354
it('returns no warnings when cells have no formulas', async function() {
355355
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
356+
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(false);
356357
callAsCurrentUserStub.onFirstCall().returns({
357358
hits: {
358359
hits: [{ _source: { one: 'foo', two: 'bar' } }],
@@ -376,6 +377,33 @@ describe('CSV Execute Job', function() {
376377
expect(csvContainsFormulas).toEqual(false);
377378
});
378379

380+
it('returns no warnings when cells have formulas but are escaped', async function() {
381+
configGetStub.withArgs('csv', 'checkForFormulas').returns(true);
382+
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true);
383+
callAsCurrentUserStub.onFirstCall().returns({
384+
hits: {
385+
hits: [{ _source: { '=SUM(A1:A2)': 'foo', two: 'bar' } }],
386+
},
387+
_scroll_id: 'scrollId',
388+
});
389+
390+
const executeJob = await executeJobFactory(mockReportingPlugin, mockLogger);
391+
const jobParams = getJobDocPayload({
392+
headers: encryptedHeaders,
393+
fields: ['=SUM(A1:A2)', 'two'],
394+
conflictedTypesFields: [],
395+
searchRequest: { index: null, body: null },
396+
});
397+
398+
const { csv_contains_formulas: csvContainsFormulas } = await executeJob(
399+
'job123',
400+
jobParams,
401+
cancellationToken
402+
);
403+
404+
expect(csvContainsFormulas).toEqual(false);
405+
});
406+
379407
it('returns no warnings when configured not to', async () => {
380408
configGetStub.withArgs('csv', 'checkForFormulas').returns(false);
381409
callAsCurrentUserStub.onFirstCall().returns({
@@ -446,7 +474,7 @@ describe('CSV Execute Job', function() {
446474
});
447475
});
448476

449-
describe('Formula values', () => {
477+
describe('Escaping cells with formulas', () => {
450478
it('escapes values with formulas', async () => {
451479
configGetStub.withArgs('csv', 'escapeFormulaValues').returns(true);
452480
callAsCurrentUserStub.onFirstCall().returns({

‎x-pack/legacy/plugins/reporting/export_types/csv/server/execute_job.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
123123
const generateCsv = createGenerateCsv(jobLogger);
124124
const bom = config.get('csv', 'useByteOrderMarkEncoding') ? CSV_BOM_CHARS : '';
125125

126-
const { content, maxSizeReached, size, csvContainsFormulas } = await generateCsv({
126+
const { content, maxSizeReached, size, csvContainsFormulas, warnings } = await generateCsv({
127127
searchRequest,
128128
fields,
129129
metaFields,
@@ -140,12 +140,14 @@ export const executeJobFactory: ExecuteJobFactory<ESQueueWorkerExecuteFn<
140140
},
141141
});
142142

143+
// @TODO: Consolidate these one-off warnings into the warnings array (max-size reached and csv contains formulas)
143144
return {
144145
content_type: 'text/csv',
145146
content: bom + content,
146147
max_size_reached: maxSizeReached,
147148
size,
148149
csv_contains_formulas: csvContainsFormulas,
150+
warnings,
149151
};
150152
};
151153
};

‎x-pack/legacy/plugins/reporting/export_types/csv/server/lib/cell_has_formula.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import { startsWith } from 'lodash';
8+
import { CSV_FORMULA_CHARS } from '../../../../common/constants';
9+
10+
export const cellHasFormulas = (val: string) =>
11+
CSV_FORMULA_CHARS.some(formulaChar => startsWith(val, formulaChar));

‎x-pack/legacy/plugins/reporting/export_types/csv/server/lib/check_cells_for_formulas.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@
55
*/
66

77
import * as _ from 'lodash';
8-
9-
const formulaValues = ['=', '+', '-', '@'];
8+
import { cellHasFormulas } from './cell_has_formula';
109

1110
interface IFlattened {
1211
[header: string]: string;
1312
}
1413

1514
export const checkIfRowsHaveFormulas = (flattened: IFlattened, fields: string[]) => {
1615
const pruned = _.pick(flattened, fields);
17-
const csvValues = [..._.keys(pruned), ...(_.values(pruned) as string[])];
16+
const cells = [..._.keys(pruned), ...(_.values(pruned) as string[])];
1817

19-
return _.some(csvValues, cell => _.some(formulaValues, char => _.startsWith(cell, char)));
18+
return _.some(cells, cell => cellHasFormulas(cell));
2019
};

‎x-pack/legacy/plugins/reporting/export_types/csv/server/lib/escape_value.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,18 @@
55
*/
66

77
import { RawValue } from './types';
8-
import { CSV_FORMULA_CHARS } from '../../../../common/constants';
8+
import { cellHasFormulas } from './cell_has_formula';
99

1010
const nonAlphaNumRE = /[^a-zA-Z0-9]/;
1111
const allDoubleQuoteRE = /"/g;
1212

13-
const valHasFormulas = (val: string) =>
14-
CSV_FORMULA_CHARS.some(formulaChar => val.startsWith(formulaChar));
15-
1613
export function createEscapeValue(
1714
quoteValues: boolean,
1815
escapeFormulas: boolean
1916
): (val: RawValue) => string {
2017
return function escapeValue(val: RawValue) {
2118
if (val && typeof val === 'string') {
22-
const formulasEscaped = escapeFormulas && valHasFormulas(val) ? "'" + val : val;
19+
const formulasEscaped = escapeFormulas && cellHasFormulas(val) ? "'" + val : val;
2320
if (quoteValues && nonAlphaNumRE.test(formulasEscaped)) {
2421
return `"${formulasEscaped.replace(allDoubleQuoteRE, '""')}"`;
2522
}

‎x-pack/legacy/plugins/reporting/export_types/csv/server/lib/generate_csv.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7+
import { i18n } from '@kbn/i18n';
78
import { Logger } from '../../../../types';
89
import { GenerateCsvParams, SavedSearchGeneratorResult } from '../../types';
910
import { createFlattenHit } from './flatten_hit';
@@ -29,11 +30,14 @@ export function createGenerateCsv(logger: Logger) {
2930
const escapeValue = createEscapeValue(settings.quoteValues, settings.escapeFormulaValues);
3031
const builder = new MaxSizeStringBuilder(settings.maxSizeBytes);
3132
const header = `${fields.map(escapeValue).join(settings.separator)}\n`;
33+
const warnings: string[] = [];
34+
3235
if (!builder.tryAppend(header)) {
3336
return {
3437
size: 0,
3538
content: '',
3639
maxSizeReached: true,
40+
warnings: [],
3741
};
3842
}
3943

@@ -82,11 +86,20 @@ export function createGenerateCsv(logger: Logger) {
8286
const size = builder.getSizeInBytes();
8387
logger.debug(`finished generating, total size in bytes: ${size}`);
8488

89+
if (csvContainsFormulas && settings.escapeFormulaValues) {
90+
warnings.push(
91+
i18n.translate('xpack.reporting.exportTypes.csv.generateCsv.escapedFormulaValues', {
92+
defaultMessage: 'CSV may contain formulas whose values have been escaped',
93+
})
94+
);
95+
}
96+
8597
return {
8698
content: builder.getString(),
87-
csvContainsFormulas,
99+
csvContainsFormulas: csvContainsFormulas && !settings.escapeFormulaValues,
88100
maxSizeReached,
89101
size,
102+
warnings,
90103
};
91104
};
92105
}

‎x-pack/legacy/plugins/reporting/export_types/csv/types.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ export interface SavedSearchGeneratorResult {
8787
size: number;
8888
maxSizeReached: boolean;
8989
csvContainsFormulas?: boolean;
90+
warnings: string[];
9091
}
9192

9293
export interface CsvResultFromSearch {

0 commit comments

Comments
 (0)