Skip to content

Commit

Permalink
[Lens][Visualize][Inspector][Reporting] Unified check for CSV cells f…
Browse files Browse the repository at this point in the history
…or known formula characters (and value escaping more in general) (#105221)

* ✨ Unify escaping logic for csv export

* 📝 Update api doc

* ✅ Fix test with new escape logic

* 👌 First batch of feedback

* 💬 Fix typo

* 👌 Memoize function

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
dej611 and kibanamachine committed Jul 16, 2021
1 parent 8e5d83d commit 2e5e4ca
Show file tree
Hide file tree
Showing 22 changed files with 183 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@
exporters: {
datatableToCSV: typeof datatableToCSV;
CSV_MIME_TYPE: string;
cellHasFormulas: (val: string) => boolean;
tableHasFormulas: (columns: import("../../expressions").DatatableColumn[], rows: Record<string, any>[]) => boolean;
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('Export CSV action', () => {
| Record<string, { content: string; type: string }>;
expect(result).toEqual({
'Hello Kibana.csv': {
content: `First Name,Last Name${LINE_FEED_CHARACTER}Kibana,undefined${LINE_FEED_CHARACTER}`,
content: `First Name,Last Name${LINE_FEED_CHARACTER}Kibana,${LINE_FEED_CHARACTER}`,
type: 'text/plain;charset=utf-8',
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class ExportCSVAction implements Action<ExportContext> {
csvSeparator: this.params.core.uiSettings.get('csv:separator', ','),
quoteValues: this.params.core.uiSettings.get('csv:quoteValues', true),
formatFactory,
escapeFormulaValues: false,
}),
type: exporters.CSV_MIME_TYPE,
};
Expand Down
11 changes: 11 additions & 0 deletions src/plugins/data/common/exports/constants.ts
Original file line number Diff line number Diff line change
@@ -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
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const CSV_FORMULA_CHARS = ['=', '+', '-', '@'];
export const nonAlphaNumRE = /[^a-zA-Z0-9]/;
export const allDoubleQuoteRE = /"/g;
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import expect from '@kbn/expect';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { allDoubleQuoteRE, nonAlphaNumRE } from './constants';
import { cellHasFormulas } from './formula_checks';

import { RawValue } from '../types';
import { cellHasFormulas } from './cell_has_formula';

const nonAlphaNumRE = /[^a-zA-Z0-9]/;
const allDoubleQuoteRE = /"/g;
type RawValue = string | object | null | undefined;

export function createEscapeValue(
quoteValues: boolean,
Expand All @@ -22,7 +21,6 @@ export function createEscapeValue(
return `"${formulasEscaped.replace(allDoubleQuoteRE, '""')}"`;
}
}

return val == null ? '' : val.toString();
};
}
13 changes: 13 additions & 0 deletions src/plugins/data/common/exports/export_csv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ function getDefaultOptions() {
csvSeparator: ',',
quoteValues: true,
formatFactory,
escapeFormulaValues: false,
};
}

Expand Down Expand Up @@ -71,4 +72,16 @@ describe('CSV exporter', () => {
'columnOne\r\n"Formatted_""value"""\r\n'
);
});

test('should escape formulas', () => {
const datatable = getDataTable();
datatable.rows[0].col1 = '=1';
expect(
datatableToCSV(datatable, {
...getDefaultOptions(),
escapeFormulaValues: true,
formatFactory: () => ({ convert: (v: unknown) => v } as FieldFormat),
})
).toMatch('columnOne\r\n"\'=1"\r\n');
});
});
26 changes: 6 additions & 20 deletions src/plugins/data/common/exports/export_csv.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,40 +10,26 @@

import { FormatFactory } from 'src/plugins/data/common/field_formats/utils';
import { Datatable } from 'src/plugins/expressions';
import { createEscapeValue } from './escape_value';

export const LINE_FEED_CHARACTER = '\r\n';
const nonAlphaNumRE = /[^a-zA-Z0-9]/;
const allDoubleQuoteRE = /"/g;
export const CSV_MIME_TYPE = 'text/plain;charset=utf-8';

// TODO: enhance this later on
function escape(val: object | string, quoteValues: boolean) {
if (val != null && typeof val === 'object') {
val = val.valueOf();
}

val = String(val);

if (quoteValues && nonAlphaNumRE.test(val)) {
val = `"${val.replace(allDoubleQuoteRE, '""')}"`;
}

return val;
}

interface CSVOptions {
csvSeparator: string;
quoteValues: boolean;
escapeFormulaValues: boolean;
formatFactory: FormatFactory;
raw?: boolean;
}

export function datatableToCSV(
{ columns, rows }: Datatable,
{ csvSeparator, quoteValues, formatFactory, raw }: CSVOptions
{ csvSeparator, quoteValues, formatFactory, raw, escapeFormulaValues }: CSVOptions
) {
const escapeValues = createEscapeValue(quoteValues, escapeFormulaValues);
// Build the header row by its names
const header = columns.map((col) => escape(col.name, quoteValues));
const header = columns.map((col) => escapeValues(col.name));

const formatters = columns.reduce<Record<string, ReturnType<FormatFactory>>>(
(memo, { id, meta }) => {
Expand All @@ -56,7 +42,7 @@ export function datatableToCSV(
// Convert the array of row objects to an array of row arrays
const csvRows = rows.map((row) => {
return columns.map((column) =>
escape(raw ? row[column.id] : formatters[column.id].convert(row[column.id]), quoteValues)
escapeValues(raw ? row[column.id] : formatters[column.id].convert(row[column.id]))
);
});

Expand Down
21 changes: 21 additions & 0 deletions src/plugins/data/common/exports/formula_checks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { startsWith } from 'lodash';
import { Datatable } from 'src/plugins/expressions';
import { CSV_FORMULA_CHARS } from './constants';

export const cellHasFormulas = (val: string) =>
CSV_FORMULA_CHARS.some((formulaChar) => startsWith(val, formulaChar));

export const tableHasFormulas = (columns: Datatable['columns'], rows: Datatable['rows']) => {
return (
columns.some(({ name }) => cellHasFormulas(name)) ||
rows.some((row) => columns.some(({ id }) => cellHasFormulas(row[id])))
);
};
3 changes: 3 additions & 0 deletions src/plugins/data/common/exports/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
*/

export { datatableToCSV, CSV_MIME_TYPE } from './export_csv';
export { createEscapeValue } from './escape_value';
export { CSV_FORMULA_CHARS } from './constants';
export { cellHasFormulas, tableHasFormulas } from './formula_checks';
4 changes: 3 additions & 1 deletion src/plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,12 @@ export {
* Exporters (CSV)
*/

import { datatableToCSV, CSV_MIME_TYPE } from '../common';
import { datatableToCSV, CSV_MIME_TYPE, cellHasFormulas, tableHasFormulas } from '../common';
export const exporters = {
datatableToCSV,
CSV_MIME_TYPE,
cellHasFormulas,
tableHasFormulas,
};

/*
Expand Down
40 changes: 21 additions & 19 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -927,6 +927,8 @@ export type ExistsFilter = Filter & {
export const exporters: {
datatableToCSV: typeof datatableToCSV;
CSV_MIME_TYPE: string;
cellHasFormulas: (val: string) => boolean;
tableHasFormulas: (columns: import("../../expressions").DatatableColumn[], rows: Record<string, any>[]) => boolean;
};

// Warning: (ae-missing-release-tag) "ExpressionFunctionKibana" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
Expand Down Expand Up @@ -2777,25 +2779,25 @@ export interface WaitUntilNextSessionCompletesOptions {
// src/plugins/data/public/index.ts:170:26 - (ae-forgotten-export) The symbol "TruncateFormat" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:170:26 - (ae-forgotten-export) The symbol "HistogramFormat" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:213:23 - (ae-forgotten-export) The symbol "datatableToCSV" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:238:27 - (ae-forgotten-export) The symbol "isFilterable" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:238:27 - (ae-forgotten-export) The symbol "isNestedField" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:238:27 - (ae-forgotten-export) The symbol "validateIndexPattern" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:238:27 - (ae-forgotten-export) The symbol "flattenHitWrapper" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:238:27 - (ae-forgotten-export) The symbol "formatHitProvider" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:410:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:410:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:410:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:412:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:413:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:422:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:423:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:424:1 - (ae-forgotten-export) The symbol "IpAddress" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:425:1 - (ae-forgotten-export) The symbol "isDateHistogramBucketAggConfig" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:429:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:430:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:433:1 - (ae-forgotten-export) The symbol "parseInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:434:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:437:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:240:27 - (ae-forgotten-export) The symbol "isFilterable" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:240:27 - (ae-forgotten-export) The symbol "isNestedField" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:240:27 - (ae-forgotten-export) The symbol "validateIndexPattern" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:240:27 - (ae-forgotten-export) The symbol "flattenHitWrapper" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:240:27 - (ae-forgotten-export) The symbol "formatHitProvider" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:412:20 - (ae-forgotten-export) The symbol "getResponseInspectorStats" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:412:20 - (ae-forgotten-export) The symbol "tabifyAggResponse" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:412:20 - (ae-forgotten-export) The symbol "tabifyGetColumns" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:414:1 - (ae-forgotten-export) The symbol "CidrMask" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:415:1 - (ae-forgotten-export) The symbol "dateHistogramInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:424:1 - (ae-forgotten-export) The symbol "InvalidEsCalendarIntervalError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:425:1 - (ae-forgotten-export) The symbol "InvalidEsIntervalFormatError" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:426:1 - (ae-forgotten-export) The symbol "IpAddress" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:427:1 - (ae-forgotten-export) The symbol "isDateHistogramBucketAggConfig" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:431:1 - (ae-forgotten-export) The symbol "isValidEsInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:432:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:435:1 - (ae-forgotten-export) The symbol "parseInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:436:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/index.ts:439:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/query/state_sync/connect_to_query_state.ts:34:5 - (ae-forgotten-export) The symbol "FilterStateStore" needs to be exported by the entry point index.d.ts
// src/plugins/data/public/search/session/session_service.ts:62:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jest.mock('../../../../../share/public', () => ({
}));
jest.mock('../../../../common', () => ({
datatableToCSV: jest.fn().mockReturnValue('csv'),
tableHasFormulas: jest.fn().mockReturnValue(false),
}));

describe('Inspector Data View', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@
*/

import React, { Component } from 'react';
import { memoize } from 'lodash';
import PropTypes from 'prop-types';
import { FormattedMessage } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';

import { EuiButton, EuiContextMenuItem, EuiContextMenuPanel, EuiPopover } from '@elastic/eui';
import { CSV_MIME_TYPE, datatableToCSV } from '../../../../common';
import {
EuiButton,
EuiContextMenuItem,
EuiContextMenuPanel,
EuiPopover,
EuiToolTip,
} from '@elastic/eui';
import { CSV_MIME_TYPE, datatableToCSV, tableHasFormulas } from '../../../../common';
import { Datatable } from '../../../../../expressions';
import { downloadMultipleAs } from '../../../../../share/public';
import { FieldFormatsStart } from '../../../field_formats';
Expand All @@ -30,6 +37,10 @@ interface DataDownloadOptionsProps {
fieldFormats: FieldFormatsStart;
}

const detectFormulasInTables = memoize((datatables: Datatable[]) =>
datatables.some(({ columns, rows }) => tableHasFormulas(columns, rows))
);

class DataDownloadOptions extends Component<DataDownloadOptionsProps, DataDownloadOptionsState> {
static propTypes = {
title: PropTypes.string.isRequired,
Expand Down Expand Up @@ -74,6 +85,7 @@ class DataDownloadOptions extends Component<DataDownloadOptionsProps, DataDownlo
quoteValues: this.props.uiSettings.get('csv:quoteValues', true),
raw: !isFormatted,
formatFactory: this.props.fieldFormats.deserialize,
escapeFormulaValues: false,
}),
type: CSV_MIME_TYPE,
};
Expand All @@ -96,6 +108,7 @@ class DataDownloadOptions extends Component<DataDownloadOptionsProps, DataDownlo
};

renderFormattedDownloads() {
const detectedFormulasInTables = detectFormulasInTables(this.props.datatables);
const button = (
<EuiButton iconType="arrowDown" iconSide="right" size="s" onClick={this.onTogglePopover}>
<FormattedMessage
Expand All @@ -104,6 +117,20 @@ class DataDownloadOptions extends Component<DataDownloadOptionsProps, DataDownlo
/>
</EuiButton>
);
const downloadButton = detectedFormulasInTables ? (
<EuiToolTip
position="top"
content={i18n.translate('data.inspector.table.exportButtonFormulasWarning', {
defaultMessage:
'Your CSV contains characters which spreadsheet applications can interpret as formulas',
})}
>
{button}
</EuiToolTip>
) : (
button
);

const items = [
<EuiContextMenuItem
key="csv"
Expand Down Expand Up @@ -139,7 +166,7 @@ class DataDownloadOptions extends Component<DataDownloadOptionsProps, DataDownlo
return (
<EuiPopover
id="inspectorDownloadData"
button={button}
button={downloadButton}
isOpen={this.state.isPopoverOpen}
closePopover={this.closePopover}
panelPaddingSize="none"
Expand Down
Loading

0 comments on commit 2e5e4ca

Please sign in to comment.