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
9 changes: 7 additions & 2 deletions web-common/src/components/vega/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,21 @@ export const timeGrainToVegaTimeUnitMap: Record<V1TimeGrain, TimeUnit> = {

export function sanitizeValueForVega(value: unknown) {
if (typeof value === "string") {
// Escape all special characters including quotes, brackets, operators, etc.
// Escape field-path syntax so Vega treats arbitrary values as flat field names.
return value.replace(
/[!@#$%^&*()+=\-[\]\\';,./{}|:<>?~]/g,
/[!@#$%^&*()+=\-[\]\\"';,./{}|:<>?~]/g,
(match) => `\\${match}`,
);
} else {
return String(value);
}
}

export function sanitizeTitleForVegaTooltip(value: unknown) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a small comment describing why we need to sanitise title. Also good to add a similar one for sanitizeValueForVega above as well.

// Tooltip titles are compiled into Vega expressions, so keep labels single-line and unescaped.
return String(value).replace(/\s+/g, " ").trim();
}

export function sanitizeValuesForSpec(values: unknown[]) {
return values.map((value) => sanitizeValueForVega(value));
}
Expand Down
12 changes: 9 additions & 3 deletions web-common/src/components/vega/vega-embed-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { EmbedOptions } from "svelte-vega";
import { expressionInterpreter } from "vega-interpreter";
import type { Config } from "vega-lite";
import type { ExpressionFunction } from "./types";
import { sanitizeTitleForVegaTooltip } from "./util";
import { getRillTheme } from "./vega-config";

export interface CreateEmbedOptionsParams {
Expand Down Expand Up @@ -67,9 +68,14 @@ export function createEmbedOptions({
}

export function getTooltipFormatter(colorMapping: ColorMapping) {
const colorMap = new Map<string, string>(
(colorMapping ?? []).map((m) => [m.value, m.color]),
);
const colorMap = new Map<string, string>();
for (const mapping of colorMapping ?? []) {
colorMap.set(mapping.value, mapping.color);
const tooltipTitle = sanitizeTitleForVegaTooltip(mapping.value);
if (!colorMap.has(tooltipTitle)) {
colorMap.set(tooltipTitle, mapping.color);
}
}

return (
items: Record<string, unknown>,
Expand Down
112 changes: 112 additions & 0 deletions web-common/src/features/components/charts/builder.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { sanitizeValueForVega } from "@rilldata/web-common/components/vega/util";
import type { ChartDataResult } from "@rilldata/web-common/features/components/charts";
import { generateVLLineChartSpec } from "@rilldata/web-common/features/components/charts/cartesian/line-chart/spec";
import chroma from "chroma-js";
import { splitAccessPath } from "vega-util";
import { parseExpression } from "vega-expression";
import type { TopLevelSpec } from "vega-lite";
import { describe, expect, it } from "vitest";

function chartDataForDimensionValue(value: string): ChartDataResult {
return {
data: [],
isFetching: false,
fields: {
created_date: {
field: "created_date",
displayName: "Time",
},
post_count: {
name: "post_count",
displayName: "Post Count",
},
post_title: {
name: "post_title",
displayName: "Post Title",
},
},
domainValues: {
post_title: [value],
},
isDarkMode: false,
hasComparison: false,
theme: {
primary: chroma("#1d4ed8"),
secondary: chroma("#7c3aed"),
},
};
}

describe("chart builder Vega field escaping", () => {
it("preserves arbitrary flat field names after Vega path escaping", () => {
const fieldNames = [
"Alpha\n\n[Beta](https://x.test/p)",
'Quote "q" \\ path',
"a[b].c",
];

for (const fieldName of fieldNames) {
expect(splitAccessPath(sanitizeValueForVega(fieldName))).toEqual([
fieldName,
]);
}
});

it("compiles pivoted tooltip fields for markdown dimension values", async () => {
stubCanvasContext();
const { compile } = await import("vega-lite");
const dimensionValue = "Alpha\n\n[Beta](https://x.test/p)";

const spec = generateVLLineChartSpec(
{
metrics_view: "reddit_posts",
x: { field: "created_date", type: "temporal" },
y: { field: "post_count", type: "quantitative" },
color: {
field: "post_title",
type: "nominal",
values: [dimensionValue],
},
},
chartDataForDimensionValue(dimensionValue),
);

const vegaSpec = compile(spec as TopLevelSpec).spec;
const tooltipExpression = findStringValue(
vegaSpec,
(value) => value.includes("timeFormat") && value.includes("Alpha"),
);

expect(tooltipExpression).toBeDefined();
expect(() => parseExpression(tooltipExpression!)).not.toThrow();
expect(JSON.stringify(vegaSpec)).toContain(
"Alpha [Beta](https://x.test/p)",
);
});
});

function findStringValue(
value: unknown,
predicate: (value: string) => boolean,
): string | undefined {
if (typeof value === "string") {
return predicate(value) ? value : undefined;
}

if (!value || typeof value !== "object") return undefined;

for (const child of Object.values(value)) {
const match = findStringValue(child, predicate);
if (match) return match;
}

return undefined;
}

function stubCanvasContext() {
if (typeof HTMLCanvasElement === "undefined") return;
Object.defineProperty(HTMLCanvasElement.prototype, "getContext", {
configurable: true,
value: () => null,
});
}
5 changes: 5 additions & 0 deletions web-common/src/features/components/charts/builder.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
sanitizeFieldName,
sanitizeTitleForVegaTooltip,
sanitizeValueForVega,
} from "@rilldata/web-common/components/vega/util";
import { createBrushParam } from "@rilldata/web-common/features/components/charts/brush-builder";
Expand Down Expand Up @@ -395,16 +396,19 @@ export function createCartesianMultiValueTooltipChannel(

if (domainValues) {
for (const value of domainValues) {
const title = sanitizeTitleForVegaTooltip(value);
// Add current period value
tooltipFields.push({
field: sanitizeValueForVega(value),
title,
type: "quantitative" as const,
formatType: yFormatType,
});

// Add previous period value
tooltipFields.push({
field: sanitizeValueForVega(value) + ComparisonDeltaPreviousSuffix,
title: title + ComparisonDeltaPreviousSuffix,
type: "quantitative" as const,
formatType: yFormatType,
});
Expand Down Expand Up @@ -433,6 +437,7 @@ export function createCartesianMultiValueTooltipChannel(
multiValueTooltipChannel = data.domainValues?.[colorField]?.map(
(value) => ({
field: sanitizeValueForVega(value as string),
title: sanitizeTitleForVegaTooltip(value),
type: "quantitative" as const,
formatType: yFormatType,
}),
Expand Down
Loading