Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 65 additions & 22 deletions src/Client/features/dataTableProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { IServer, ResultTable } from './server';
import type { IWebView } from './webview';
import type { IClipboard } from './clipboard';
import { formatCfHtml } from './clipboard';
import { resultTableToHtml, escapeHtml } from './html';
import { resultTableToHtml } from './html';
import { resultTableToMarkdown } from './markdown';

// ─── Interfaces ─────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -45,17 +45,18 @@ export interface IDataTableProvider {

// ─── Implementation ─────────────────────────────────────────────────────────

// Cell values and column names must be HTML-escaped before being handed to
// Simple-DataTables, which renders both via innerHTML. Without escaping, a
// value like `Name <user@example.com>` causes the grid to fail with
// "InvalidCharacterError: Failed to execute 'createElement' ..." because
// the angle-bracketed substring is parsed as a tag name. We reuse the shared
// `escapeHtml` from html.ts so this stays in sync with HTML rendering used
// elsewhere (clipboard "copy as HTML", markdown export, etc.).
// Cell values are passed raw (unescaped) to the webview. In the init
// script each value is wrapped in a Simple-DataTables cell object with
// all three fields set (data, text, order). That triggers the early-return
// path in Simple-DataTables' readDataCell, so it never parses cell strings
// as HTML — avoiding the InvalidCharacterError that values like
// "George <gw@x.com>" used to trigger — while still letting the library
// own row rendering (paging, virtualization). The library renders cell
// `text` as textContent, so characters like " and & display correctly
// without any escaping.
function formatCellValue(value: unknown): string {
if (value === null || value === undefined) return '';
const text = (typeof value === 'object') ? JSON.stringify(value) : String(value);
return escapeHtml(text);
return (typeof value === 'object') ? JSON.stringify(value) : String(value);
}

/** Generate a short random token for message scoping. */
Expand Down Expand Up @@ -89,11 +90,13 @@ class DataTableView implements IDataTableView {
});

const data = {
// Column names are HTML-escaped for the same reason as cell values:
// Simple-DataTables renders headings via innerHTML, so a column name
// containing `<...>` would either inject markup or throw the same
// `InvalidCharacterError` we saw with cell values.
columns: table.columns.map(c => ({ ...c, name: escapeHtml(c.name) })),
// Pass column names and cell values raw (no HTML escaping).
// The init script wraps each cell as a { data, text, order } object
// and configures an explicit per-column `type` (never "html"), so
// Simple-DataTables takes its early-return path in readDataCell and
// never parses cell strings as HTML. Rendering goes through the
// textContent path, so '<', '>', '&', and '"' display verbatim.
columns: table.columns.map(c => ({ ...c })),
rows: table.rows.map(row => row.map(cell => formatCellValue(cell)))
};
const json = JSON.stringify(data).replace(/<\//g, '<\\/');
Expand Down Expand Up @@ -267,10 +270,14 @@ class DataTableView implements IDataTableView {
var tableData = ${tableDataJson};

// ── Initialize Simple-DataTables grid ──
var headings = tableData.columns.map(function(c) { return c.name; });
var data = tableData.rows;
// Map Kusto column types to Simple-DataTables sort types so numeric and
// date columns sort correctly instead of as text.
// date columns sort correctly instead of as text. Importantly, we set
// an explicit type for EVERY column (not just numeric/date/bool), because
// Simple-DataTables defaults the type to "html" — and for html-typed
// columns the renderer treats cell.data as an array of node objects to
// splice into the <td>. Our cell.data is a string, which breaks
// rendering. Using "string" type makes the renderer go through the
// text-content path and use cell.text instead.
var columnSettings = [];
tableData.columns.forEach(function(c, i) {
var kustoType = c.type;
Expand All @@ -279,13 +286,49 @@ class DataTableView implements IDataTableView {
kustoType === 'real' || kustoType === 'decimal') ? 'number'
: (kustoType === 'datetime') ? 'date'
: (kustoType === 'bool') ? 'boolean'
: null;
if (sortType) {
columnSettings.push({ select: i, type: sortType });
: 'string';
columnSettings.push({ select: i, type: sortType });
});

// Compute a sort-friendly "order" value for a raw cell string based on
// the Simple-DataTables column type. We supply this ourselves so the
// library does not need to parse the cell as HTML to derive it.
function computeOrder(text, sortType) {
if (sortType === 'number') {
var n = parseFloat(text);
return isNaN(n) ? text : n;
}
if (sortType === 'date') {
var t = Date.parse(text);
return isNaN(t) ? text : t;
}
if (sortType === 'boolean') {
var s = String(text).toLowerCase().trim();
return (s === 'false' || s === '0' || s === '' || s === 'null' || s === 'undefined') ? 0 : 1;
}
return text;
}
var columnSortTypes = columnSettings.map(function(c) { return c.type; });

// Build heading cell objects (data + text) and row cells (data + text +
// order). Plain objects whose keys are all in [data, text, order,
// attributes] are returned as-is by Simple-DataTables' readDataCell —
// it never parses cell strings as HTML in that path — avoiding the
// InvalidCharacterError that <...> values used to trigger. Rendering
// goes through the library's normal paged row rendering, so we keep
// the benefits of not materializing every <tr>/<td> ourselves.
var headings = tableData.columns.map(function(c) {
return { data: c.name, text: c.name };
});
var rows = tableData.rows.map(function(row) {
return row.map(function(text, i) {
var sortType = columnSortTypes[i];
return { data: text, text: text, order: computeOrder(text, sortType) };
});
});

var grid = new simpleDatatables.DataTable(tableEl, {
data: { headings: headings, data: data },
data: { headings: headings, data: rows },
columns: columnSettings,
perPage: 100,
perPageSelect: [50, 100, 500, 1000],
Expand Down
63 changes: 36 additions & 27 deletions src/Client/tests/unit/dataTableProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,12 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

// formatCellValue produces '{"a":1}', then HTML-escapes the quotes
// to '{&quot;a&quot;:1}', which is then JSON.stringified into the
// embedded data payload.
expect(html).toContain('{&quot;a&quot;:1}');
// formatCellValue produces '{"a":1}'. That string is then JSON
// serialized for the embedded payload, so the inner `"` become
// `\"`. We no longer HTML-escape cell values (Simple-DataTables
// renders cell text via textContent), so `&quot;` must NOT appear.
expect(html).toContain('{\\"a\\":1}');
expect(html).not.toContain('&quot;');
});

it('escapes closing script tags in JSON data', () => {
Expand All @@ -168,19 +170,26 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

// The cell value is HTML-escaped before JSON serialization, so the
// raw `</script>` text never appears in the embedded data. Only
// the real closing </script> tag should appear (at the very end).
// The cell value is no longer HTML-escaped, but the embedded JSON
// is still protected against early `</script>` termination by the
// `</` -> `<\/` replacement at serialization time. Only the real
// closing </script> tag should appear; the cell payload should use
// the `<\/script>` form.
const matches = html.match(/<\/script>/g);
expect(matches).toHaveLength(1);
// The escaped form of '<' (`&lt;`) is what ends up in the JSON.
expect(html).toContain('&lt;/script&gt;');
expect(html).toContain('<\\/script>');
});

it('HTML-escapes angle brackets in cell values so the grid does not parse them as tags', () => {
it('preserves angle brackets in cell values (passed via cell objects)', () => {
// Repro for the customer bug: `print Requestor = "George Washington <gwashington@contoso.com>"`
// crashed Simple-DataTables with `InvalidCharacterError: Failed to execute 'createElement'`
// because it interpreted the `<gwashington...>` substring as an HTML tag.
//
// The fix is to pass each cell as a Simple-DataTables cell
// object with all three of data/text/order set, which triggers
// the early-return path in readDataCell and bypasses HTML
// parsing entirely. Angle brackets therefore survive verbatim
// in the JSON payload.
const table = makeTable(
[{ name: 'Requestor', type: 'string' }],
[['George Washington <gwashington@contoso.com>']],
Expand All @@ -189,13 +198,11 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

// The raw `<gwashington...>` substring must NOT survive into the grid's
// data payload, where Simple-DataTables would render it as HTML.
expect(html).not.toContain('<gwashington@contoso.com>');
expect(html).toContain('&lt;gwashington@contoso.com&gt;');
expect(html).toContain('George Washington <gwashington@contoso.com>');
expect(html).not.toContain('&lt;gwashington@contoso.com&gt;');
});

it('HTML-escapes ampersands and quotes in cell values', () => {
it('preserves ampersands and quotes in cell values', () => {
const table = makeTable(
[{ name: 'Col', type: 'string' }],
[['Tom & Jerry'], ['She said "hi"'], ["It's fine"]],
Expand All @@ -204,19 +211,21 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

expect(html).toContain('Tom &amp; Jerry');
// Quote becomes &quot;.
expect(html).toContain('&quot;');
// Apostrophes are intentionally not escaped (matching the shared
// escapeHtml helper in html.ts) since they are safe in element
// content. The literal `'` should make it through.
// Cell values are no longer HTML-escaped — they appear raw in
// the JSON payload (with JSON's own `\"` quote escaping).
expect(html).toContain('Tom & Jerry');
expect(html).not.toContain('&amp;');
expect(html).not.toContain('&quot;');
expect(html).toContain('She said \\"hi\\"');
expect(html).toContain("It's fine");
});

it('HTML-escapes angle brackets in column names so headings do not parse as tags', () => {
// Simple-DataTables renders headings via innerHTML too, so a column
// name containing `<...>` would otherwise either inject markup or
// throw the same InvalidCharacterError we saw with cell values.
it('preserves angle brackets in column names (passed via cell objects)', () => {
// Column names are passed as Simple-DataTables heading cell
// objects ({ data, text }), with column types supplied separately
// via the `columns` config. The library returns these heading
// objects as-is without re-parsing as HTML, so angle brackets
// survive raw in the JSON payload and do not need HTML-escaping.
const table = makeTable(
[{ name: 'Value <units>', type: 'real' }],
[[42]],
Expand All @@ -225,8 +234,8 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

expect(html).not.toContain('Value <units>');
expect(html).toContain('Value &lt;units&gt;');
expect(html).toContain('Value <units>');
expect(html).not.toContain('Value &lt;units&gt;');
});

it('includes Simple-DataTables initialization in the script', () => {
Expand Down
Loading