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
19 changes: 15 additions & 4 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 } from './html';
import { resultTableToHtml, escapeHtml } from './html';
import { resultTableToMarkdown } from './markdown';

// ─── Interfaces ─────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -45,10 +45,17 @@ 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.).
function formatCellValue(value: unknown): string {
if (value === null || value === undefined) return '';
if (typeof value === 'object') return JSON.stringify(value);
return String(value);
const text = (typeof value === 'object') ? JSON.stringify(value) : String(value);
return escapeHtml(text);
}
Comment thread
mattwar marked this conversation as resolved.

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

const data = {
columns: table.columns,
// 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) })),
rows: table.rows.map(row => row.map(cell => formatCellValue(cell)))
};
const json = JSON.stringify(data).replace(/<\//g, '<\\/');
Expand Down
66 changes: 61 additions & 5 deletions src/Client/tests/unit/dataTableProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ describe('SimpleDataTableProvider', () => {
provider.createView(webview, table);
const html: string = webview.setContent.mock.calls[0]![0];

// formatCellValue produces '{"a":1}', then JSON.stringify double-encodes it
expect(html).toContain('{\\\"a\\\":1}');
// 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}');
});

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

// The </ in JSON data must be escaped to prevent premature script closing.
// Only the real closing </script> tag should appear (at the very end).
// 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).
const matches = html.match(/<\/script>/g);
expect(matches).toHaveLength(1);
expect(html).toContain('<\\/script>'); // escaped form in JSON data
// The escaped form of '<' (`&lt;`) is what ends up in the JSON.
expect(html).toContain('&lt;/script&gt;');
});

it('HTML-escapes angle brackets in cell values so the grid does not parse them as tags', () => {
// 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.
const table = makeTable(
[{ name: 'Requestor', type: 'string' }],
[['George Washington <gwashington@contoso.com>']],
);
const webview = createMockWebView();
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;');
});

it('HTML-escapes ampersands and quotes in cell values', () => {
const table = makeTable(
[{ name: 'Col', type: 'string' }],
[['Tom & Jerry'], ['She said "hi"'], ["It's fine"]],
);
const webview = createMockWebView();
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.
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.
const table = makeTable(
[{ name: 'Value <units>', type: 'real' }],
[[42]],
);
const webview = createMockWebView();
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;');
});

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