From 727f94bade0afb2641c4037231173ece21ff5fc5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 10 Nov 2026 19:40:41 +0000 Subject: [PATCH] fix: guard canvas size and scroll offset Reject a non-finite pixel ratio or layout length, and ignore non-finite scroll and zoom-range input so the time scale cannot stick on NaN. Co-authored-by: Joe Blau --- packages/chart/src/config.ts | 12 +++- packages/chart/src/core/chart.ts | 15 ++++- packages/chart/src/core/scale.ts | 10 ++- packages/chart/test/config.test.ts | 11 +++ packages/chart/test/layout-invariants.test.ts | 67 +++++++++++++++++++ packages/chart/test/scale.test.ts | 18 +++++ 6 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 packages/chart/test/layout-invariants.test.ts diff --git a/packages/chart/src/config.ts b/packages/chart/src/config.ts index 60018ed..d608a46 100644 --- a/packages/chart/src/config.ts +++ b/packages/chart/src/config.ts @@ -505,8 +505,18 @@ export function mergeDeep(base: T, override: unknown): T { * * @param partial - Any subset of the consolidated schema. */ +/** A CSS length used for layout: finite and `>= 0`. */ +export function assertLayoutLength(value: number, label: string): void { + if (!(Number.isFinite(value) && value >= 0)) throw new Error(`chart-ts: invalid ${label} ${value}`); +} + export function resolveConfig(partial?: DeepPartial): ChartConfig { - return mergeDeep(DEFAULT_CONFIG, partial ?? {}); + const config = mergeDeep(DEFAULT_CONFIG, partial ?? {}); + assertLayoutLength(config.width, 'width'); + assertLayoutLength(config.height, 'height'); + assertLayoutLength(config.priceAxis.width, 'priceAxis.width'); + assertLayoutLength(config.timeAxis.height, 'timeAxis.height'); + return config; } /** diff --git a/packages/chart/src/core/chart.ts b/packages/chart/src/core/chart.ts index 4f1f9c1..67bf7ec 100644 --- a/packages/chart/src/core/chart.ts +++ b/packages/chart/src/core/chart.ts @@ -5,6 +5,7 @@ */ import { + assertLayoutLength, mergeDeep, resolveConfig, type ChartConfig, @@ -377,7 +378,9 @@ export class Chart { this.indicators = options.registries?.indicators ?? createIndicatorRegistry(); this.drawings = options.registries?.drawings ?? createDrawingRegistry(); this.seriesRenderers = options.registries?.series ?? createSeriesRegistry(); - this.pixelRatio = options.pixelRatio ?? 1; + const ratio = options.pixelRatio ?? 1; + if (!(ratio > 0 && Number.isFinite(ratio))) throw new Error(`chart-ts: invalid pixelRatio ${ratio}`); + this.pixelRatio = ratio; this.snapshotDocument = options.document; this.clock = options.now ?? (() => Date.now()); if (options.container !== undefined) { @@ -560,6 +563,10 @@ export class Chart { /** Deep-merges a partial over the current config and re-renders. */ updateConfig(partial: DeepPartial): void { + if (partial.width !== undefined) assertLayoutLength(partial.width, 'width'); + if (partial.height !== undefined) assertLayoutLength(partial.height, 'height'); + if (partial.priceAxis?.width !== undefined) assertLayoutLength(partial.priceAxis.width, 'priceAxis.width'); + if (partial.timeAxis?.height !== undefined) assertLayoutLength(partial.timeAxis.height, 'timeAxis.height'); if (partial.priceAxis?.lockPriceToBarRatio === true && !this.config.priceAxis.lockPriceToBarRatio) { this.lockedRatio = this.scale.priceToBarRatio(); } @@ -1049,10 +1056,14 @@ export class Chart { * is scaled by `pixelRatio`, so text stays crisp on HiDPI displays. * * When `pixelRatio` is omitted the currently stored ratio is kept; passing - * it explicitly (including `1`) updates it. + * it explicitly (including `1`) updates it. A non-positive or non-finite + * ratio, or a non-finite or negative size, throws and leaves the canvas as it is. */ resize(width: number, height: number, pixelRatio?: number): void { const ratio = pixelRatio ?? this.pixelRatio; + if (!(ratio > 0 && Number.isFinite(ratio))) throw new Error(`chart-ts: invalid pixelRatio ${ratio}`); + assertLayoutLength(width, 'chart width'); + assertLayoutLength(height, 'chart height'); const backingWidth = Math.round(width * ratio); const backingHeight = Math.round(height * ratio); if (this.pixelRatio === ratio && this.canvas.width === backingWidth && this.canvas.height === backingHeight) return; diff --git a/packages/chart/src/core/scale.ts b/packages/chart/src/core/scale.ts index 368d606..f6dba32 100644 --- a/packages/chart/src/core/scale.ts +++ b/packages/chart/src/core/scale.ts @@ -182,21 +182,24 @@ export class TimeScale { return true; } - /** Scrolls by `bars` (slots in continuous mode; positive moves toward history). */ + /** Scrolls by `bars` (slots in continuous mode; positive moves toward history). Non-finite distances are ignored. */ scroll(bars: number, dataLength: number): void { + if (!Number.isFinite(bars)) return; this.scrollOffset += bars; this.clampScroll(dataLength); } - /** Scrolls so that bar `index` becomes the rightmost visible bar. */ + /** Scrolls so that bar `index` becomes the rightmost visible bar. A non-finite index is ignored. */ scrollTo(index: number, dataLength: number): void { + if (!Number.isFinite(index)) return; const slots = this.slotsFor(dataLength); this.scrollOffset = slots === null ? dataLength - 1 - index : slots[dataLength - 1] - slotOf(slots, index); this.clampScroll(dataLength); } - /** Fits bars `[from, to]` (any order) edge to edge in the viewport. */ + /** Fits bars `[from, to]` (any order) edge to edge in the viewport. Non-finite bounds are ignored. */ fitRange(from: number, to: number, dataLength: number): void { + if (!Number.isFinite(from) || !Number.isFinite(to)) return; const slots = this.slotsFor(dataLength); const lo = slots === null ? Math.min(from, to) : slotOf(slots, Math.min(from, to)); const hi = slots === null ? Math.max(from, to) : slotOf(slots, Math.max(from, to)); @@ -209,6 +212,7 @@ export class TimeScale { } private clampScroll(dataLength: number): void { + if (!Number.isFinite(this.scrollOffset)) this.scrollOffset = 0; const slots = this.slotsFor(dataLength); const maxOffset = Math.max(0, slots === null ? dataLength - 1 : slots[dataLength - 1] - slots[0]); const minOffset = -Math.ceil(this.width / this.barSpacing); diff --git a/packages/chart/test/config.test.ts b/packages/chart/test/config.test.ts index f55b876..c16b901 100644 --- a/packages/chart/test/config.test.ts +++ b/packages/chart/test/config.test.ts @@ -53,6 +53,17 @@ describe('resolveConfig', () => { assert.equal(cfg.crosshair.labelBackground, '#2962ff'); assert.equal(cfg.crosshair.labelColor, 'auto'); }); + it('rejects a non-finite or negative layout length', () => { + assert.throws(() => resolveConfig({ width: Number.NaN }), /invalid width NaN/); + assert.throws(() => resolveConfig({ height: Number.POSITIVE_INFINITY }), /invalid height/); + assert.throws(() => resolveConfig({ priceAxis: { width: -1 } }), /invalid priceAxis\.width -1/); + assert.throws(() => resolveConfig({ timeAxis: { height: Number.NaN } }), /invalid timeAxis\.height/); + const zero = resolveConfig({ width: 0, height: 0, priceAxis: { width: 0 }, timeAxis: { height: 0 } }); + assert.equal(zero.width, 0); + assert.equal(zero.height, 0); + assert.equal(zero.priceAxis.width, 0); + assert.equal(zero.timeAxis.height, 0); + }); it('deep-merges typography and crosshair label overrides', () => { const cfg = resolveConfig({ theme: { fontFamily: '"Geist", system-ui', monoFamily: '"Geist Mono", monospace', fontSize: 13 }, diff --git a/packages/chart/test/layout-invariants.test.ts b/packages/chart/test/layout-invariants.test.ts new file mode 100644 index 0000000..7b6a059 --- /dev/null +++ b/packages/chart/test/layout-invariants.test.ts @@ -0,0 +1,67 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { createChart } from '../dist/index.js'; +import { MockDocument } from '../dist/dom.js'; + +describe('canvas size and pixelRatio', () => { + it('rejects a non-positive or non-finite pixelRatio and a bad CSS size (#93)', () => { + for (const pixelRatio of [0, -2, Number.NaN, Number.POSITIVE_INFINITY]) { + assert.throws(() => createChart({ document: new MockDocument(), pixelRatio, config: { wasm: false } }), /invalid pixelRatio/); + } + assert.throws(() => createChart({ document: new MockDocument(), config: { wasm: false, width: Number.NaN } }), /invalid width/); + assert.throws(() => createChart({ document: new MockDocument(), config: { wasm: false, height: -1 } }), /invalid height/); + assert.throws(() => createChart({ document: new MockDocument(), config: { wasm: false, priceAxis: { width: Number.NaN } } }), /invalid priceAxis\.width/); + assert.throws(() => createChart({ document: new MockDocument(), config: { wasm: false, timeAxis: { height: Number.POSITIVE_INFINITY } } }), /invalid timeAxis\.height/); + + const doc = new MockDocument(); + const chart = createChart({ document: doc, pixelRatio: 2, config: { wasm: false, width: 100, height: 80 } }); + const canvas = doc.created[0]!; + assert.equal(canvas.width, 200); + const plot = chart.plotArea; + assert.throws(() => chart.resize(800, 400, 0), /invalid pixelRatio 0/); + assert.throws(() => chart.resize(800, 400, Number.POSITIVE_INFINITY), /invalid pixelRatio Infinity/); + assert.throws(() => chart.resize(Number.NaN, 400), /invalid chart width/); + assert.throws(() => chart.resize(800, -1), /invalid chart height/); + assert.equal(canvas.width, 200); + assert.deepEqual(chart.plotArea, plot); + assert.ok(Number.isFinite(chart.scale.xToIndex(40))); + + chart.resize(0, 0); + assert.equal(canvas.width, 0); + chart.resize(50, 40); + assert.equal(canvas.width, 100); + assert.equal(canvas.height, 80); + + assert.throws(() => chart.updateConfig({ width: Number.NaN }), /invalid width/); + assert.throws(() => chart.updateConfig({ height: Number.NEGATIVE_INFINITY }), /invalid height/); + assert.throws(() => chart.updateConfig({ priceAxis: { width: -4 } }), /invalid priceAxis\.width/); + assert.throws(() => chart.updateConfig({ timeAxis: { height: Number.NaN } }), /invalid timeAxis\.height/); + assert.equal(chart.getConfig().width, 100); + assert.equal(chart.getConfig().height, 80); + chart.updateConfig({ width: 120, priceAxis: { mode: 'logarithmic' }, timeAxis: { tickCount: 4 } }); + chart.updateConfig({ height: 90, priceAxis: { width: 48 }, timeAxis: { height: 20 } }); + assert.equal(chart.getConfig().width, 120); + assert.equal(chart.getConfig().height, 90); + assert.equal(chart.getConfig().priceAxis.width, 48); + assert.equal(chart.getConfig().timeAxis.height, 20); + assert.equal(chart.getConfig().priceAxis.mode, 'logarithmic'); + chart.destroy(); + }); +}); + +describe('time scale scroll offset', () => { + it('keeps the visible range finite when scroll input is not (#94)', () => { + const chart = createChart({ document: new MockDocument(), config: { wasm: false, data: Array.from({ length: 80 }, (_, i) => ({ time: 1_700_000_000 + i * 60, open: 10, high: 12, low: 9, close: 11 })) } }); + chart.scale.scrollBy(3); + const range = chart.scale.visibleRange(); + for (const bad of [Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]) { + chart.scale.scrollBy(bad); + chart.scale.scrollTo(bad); + chart.scale.zoomToRange(bad, 10); + chart.scale.zoomToRange(4, bad); + } + assert.deepEqual(chart.scale.visibleRange(), range); + assert.ok(Number.isFinite(range.from) && Number.isFinite(range.to)); + chart.destroy(); + }); +}); diff --git a/packages/chart/test/scale.test.ts b/packages/chart/test/scale.test.ts index e250fd0..2807db3 100644 --- a/packages/chart/test/scale.test.ts +++ b/packages/chart/test/scale.test.ts @@ -88,6 +88,24 @@ describe('TimeScale', () => { s.scrollTo(20, 50); assert.equal(s.visibleRange(50).to, 21); }); + it('ignores non-finite scroll, scrollTo and fitRange, and recovers a NaN offset', () => { + const s = new TimeScale(10, 100); + s.scroll(4, 50); + const offset = s.scrollOffset; + const range = s.visibleRange(50); + for (const bad of [Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]) { + s.scroll(bad, 50); + s.scrollTo(bad, 50); + s.fitRange(bad, 10, 50); + s.fitRange(3, bad, 50); + } + assert.equal(s.scrollOffset, offset); + assert.deepEqual(s.visibleRange(50), range); + s.scrollOffset = Number.NaN; + s.scroll(0, 50); + assert.ok(Number.isFinite(s.scrollOffset)); + assert.ok(Number.isFinite(s.visibleRange(50).from) && Number.isFinite(s.visibleRange(50).to)); + }); it('setViewport rejects negative widths', () => { const s = new TimeScale(6, 100); s.setViewport(-50);