diff --git a/index.js b/index.js index 3e9d8b0..acb2205 100644 --- a/index.js +++ b/index.js @@ -11,6 +11,7 @@ const telemetry = require('./telemetry'); const { getPdfBufferFromPng, getPdfBufferWithText } = require('./lib/pdf'); const { logger } = require('./logging'); const { renderChartJs } = require('./lib/charts'); +const { ChartConfigError } = require('./lib/chart_config'); const { renderGraphviz } = require('./lib/graphviz'); const { toChartJs, parseSize } = require('./lib/google_image_charts'); const { renderQr, DEFAULT_QR_SIZE } = require('./lib/qr'); @@ -120,14 +121,17 @@ function failSvg(res, msg, statusCode = 500) { -

${msg}

+

${String(msg) + .replace(/&/g, '&') + .replace(//g, '>')}

`); } -async function failPdf(res, msg) { +async function failPdf(res, msg, statusCode = 500) { const buf = await getPdfBufferWithText(msg); - res.writeHead(500, { + res.writeHead(statusCode, { 'Content-Type': 'application/pdf', 'X-quickchart-error': sanitizeErrorHeader(msg), }); @@ -183,7 +187,7 @@ async function renderChartToPdf(req, res, opts) { function doChartjsRender(req, res, opts) { if (!opts.chart) { - opts.failFn(res, 'You are missing variable `c` or `chart`'); + opts.failFn(res, 'You are missing variable `c` or `chart`', 400); return; } @@ -192,12 +196,15 @@ function doChartjsRender(req, res, opts) { let untrustedInput = opts.chart; if (opts.encoding === 'base64') { + if (typeof untrustedInput !== 'string') { + opts.failFn(res, 'Base64 chart configuration must be a string', 400); + return; + } // TODO(ian): Move this decoding up the call stack. try { untrustedInput = Buffer.from(opts.chart, 'base64').toString('utf8'); } catch (err) { - logger.warn('base64 malformed', err); - opts.failFn(res, err); + opts.failFn(res, 'Invalid base64 chart configuration', 400); return; } } @@ -213,8 +220,14 @@ function doChartjsRender(req, res, opts) { ) .then(opts.onRenderHandler) .catch((err) => { - logger.warn('Chart error', err); - opts.failFn(res, err); + if (err instanceof ChartConfigError) { + // Do not log input, Error objects, or stacks for expected client failures. + logger.info({ code: 'INVALID_CHART_CONFIG' }, 'Chart configuration rejected'); + opts.failFn(res, err.message, 400); + } else { + logger.error(err, 'Chart rendering failed'); + opts.failFn(res, err); + } }); } diff --git a/lib/chart_config.js b/lib/chart_config.js new file mode 100644 index 0000000..f5e1e79 --- /dev/null +++ b/lib/chart_config.js @@ -0,0 +1,79 @@ +class ChartConfigError extends Error { + constructor(message) { + super(`Invalid chart configuration: ${message}`); + this.name = 'ChartConfigError'; + } +} + +function isObject(value) { + return value !== null && Object.prototype.toString.call(value) === '[object Object]'; +} + +function validateType(type, field) { + if (type != null && (typeof type !== 'string' || !type.trim())) { + throw new ChartConfigError(`${field} must be a non-empty string`); + } +} + +// Validate only the containers QuickChart and Chart.js need to mutate. Do not +// JSON-roundtrip configurations: callbacks, gradients, and plugin data are valid. +function validateChartConfig(chart) { + if (!isObject(chart)) { + throw new ChartConfigError('expected an object, not a string, array, or primitive'); + } + validateType(chart.type, 'type'); + if (chart.data != null && !isObject(chart.data)) { + throw new ChartConfigError('data must be an object'); + } + if (chart.options != null && !isObject(chart.options)) { + throw new ChartConfigError('options must be an object'); + } + if (chart.plugins != null && !Array.isArray(chart.plugins)) { + throw new ChartConfigError('plugins must be an array'); + } + if ( + chart.options && + chart.options.plugins != null && + chart.options.plugins !== false && + !isObject(chart.options.plugins) + ) { + throw new ChartConfigError('options.plugins must be an object or false'); + } + + const data = chart.data || {}; + if (data.labels != null && !Array.isArray(data.labels)) { + throw new ChartConfigError('data.labels must be an array'); + } + if (data.datasets != null && !Array.isArray(data.datasets)) { + throw new ChartConfigError('data.datasets must be an array'); + } + const datasets = data.datasets || []; + for (let i = 0; i < datasets.length; i += 1) { + if (!isObject(datasets[i])) { + throw new ChartConfigError(`data.datasets[${i}] must be an object`); + } + validateType(datasets[i].type, `data.datasets[${i}].type`); + // These QuickChart shorthand types manipulate arrays before Chart.js runs. + if ( + (chart.type === 'sparkline' || chart.type === 'progressBar') && + !Array.isArray(datasets[i].data) + ) { + throw new ChartConfigError(`data.datasets[${i}].data must be an array for ${chart.type}`); + } + } +} + +function validateChartTypes(chart, Chart) { + chart.data.datasets.forEach((dataset, index) => { + const type = dataset.type || chart.config.type; + // Chart.js 3/4 use a registry; v2 plugins extend Chart.controllers. + const controllers = Chart.registry ? Chart.registry.controllers.items : Chart.controllers; + if (!Object.prototype.hasOwnProperty.call(controllers, type)) { + throw new ChartConfigError( + `data.datasets[${index}] must use a registered chart type (set type on the chart or dataset)`, + ); + } + }); +} + +module.exports = { ChartConfigError, validateChartConfig, validateChartTypes }; diff --git a/lib/charts.js b/lib/charts.js index 367fc7d..3e77f69 100644 --- a/lib/charts.js +++ b/lib/charts.js @@ -6,6 +6,7 @@ const { CanvasRenderService } = require('chartjs-node-canvas'); const { fixNodeVmObject } = require('./util'); const { logger } = require('../logging'); const { uniqueSvg } = require('./svg'); +const { ChartConfigError, validateChartConfig, validateChartTypes } = require('./chart_config'); // Polyfills require('canvas-5-polyfill'); @@ -128,9 +129,11 @@ async function renderChartJs( format, untrustedChart, ) { + // Loading the renderer is a server operation, outside the input-evaluation catch. + const Chart = await getChartJsForVersion(version); let chart; if (typeof untrustedChart === 'string') { - // The chart could contain Javascript - run it in a VM. + // Keep support for JavaScript expressions, callbacks, and chart helpers. try { const { getGradientFill, getGradientFillHelper } = getGradientFunctions(width, height); const chartFunction = new Function( @@ -144,17 +147,22 @@ async function renderChartJs( getGradientFill, getGradientFillHelper, { draw: patternDraw }, - getChartJsForVersion(version), + Chart, ); } catch (err) { - logger.error('Input Error', err, untrustedChart); - return Promise.reject(new Error(`Invalid input\n${err}`)); + const message = err && err.message ? err.message : String(err); + throw new ChartConfigError(`could not evaluate JavaScript: ${message}`); } } else { // The chart is just a simple JSON object. chart = untrustedChart; } + validateChartConfig(chart); + // Match Chart.js defaults before QuickChart's shorthand/plugin transforms. + chart.data = chart.data || {}; + chart.data.datasets = chart.data.datasets || []; + // Patch some bugs and issues. fixNodeVmObject(chart); @@ -168,7 +176,7 @@ async function renderChartJs( // TODO(ian): Move special chart type out of this file. if (chart.type === 'sparkline') { if (chart.data.datasets.length < 1) { - return Promise.reject(new Error('"sparkline" requres 1 dataset')); + throw new ChartConfigError('"sparkline" requires at least 1 dataset'); } chart.type = 'line'; const dataseries = chart.data.datasets[0].data; @@ -216,7 +224,7 @@ async function renderChartJs( chart.type = 'horizontalBar'; if (chart.data.datasets.length < 1 || chart.data.datasets.length > 2) { - throw new Error('progressBar chart requires 1 or 2 datasets'); + throw new ChartConfigError('progressBar chart requires 1 or 2 datasets'); } let usePercentage = false; @@ -227,7 +235,7 @@ async function renderChartJs( chart.data.datasets.push({ data: Array(dataLen).fill(100) }); } if (chart.data.datasets[0].data.length !== chart.data.datasets[1].data.length) { - throw new Error('progressBar datasets must have the same size of data'); + throw new ChartConfigError('progressBar datasets must have the same size of data'); } chart.data.labels = chart.labels || Array.from(Array(dataLen).keys()); @@ -424,6 +432,22 @@ async function renderChartJs( const canvasRenderService = await getRenderer(width, height, version, format); + // Run after user plugins initialize/register controllers, but before Chart.js + // constructs dataset controllers. Avoid a hardcoded list of supported types. + chart.plugins.push({ + id: 'quickchart-config-validation', + beforeUpdate: chartInstance => { + try { + validateChartTypes(chartInstance, Chart); + } catch (err) { + // The constructor has already registered this instance. On rejection, + // renderToBuffer cannot receive it to perform its usual cleanup. + chartInstance.destroy(); + throw err; + } + }, + }); + if (format === 'svg') { // SVG rendering doesn't work asychronously. return Buffer.from( diff --git a/test/ci/chart_validation.js b/test/ci/chart_validation.js new file mode 100644 index 0000000..94846de --- /dev/null +++ b/test/ci/chart_validation.js @@ -0,0 +1,286 @@ +/* eslint-env node, mocha */ + +const assert = require('assert'); +const request = require('supertest'); +const { CanvasRenderService } = require('chartjs-node-canvas'); + +const app = require('../../index'); +const { logger } = require('../../logging'); +const { renderChartJs } = require('../../lib/charts'); +const { ChartConfigError } = require('../../lib/chart_config'); + +function render(chart, version = '2.9.4') { + return renderChartJs(120, 80, 'white', 1, version, 'png', chart); +} + +describe('chart configuration validation', () => { + const invalidInputs = [ + ['raw Mermaid', 'graph TD; A --> B'], + ['quoted Mermaid', '"graph TD; A --> B"'], + ['quoted Graphviz', '"digraph { A -> B }"'], + ['computed string', '(() => "graph TD; A --> B")()'], + ['null expression', 'null'], + ['undefined expression', 'undefined'], + ['number expression', '42'], + ['boolean expression', 'true'], + ['array expression', '[]'], + ['function expression', '(() => {})'], + ['boxed string', 'new String("graph TD; A --> B")'], + ['date expression', 'new Date()'], + ['promise expression', 'Promise.resolve({type: "bar"})'], + ['null object', null], + ['number object', 42], + ['boolean object', true], + ['array object', []], + ['string data', { type: 'bar', data: 'graph TD; A --> B' }], + ['array data', { type: 'bar', data: [] }], + ['string datasets', { type: 'bar', data: { datasets: 'bad' } }], + ['object datasets', { type: 'bar', data: { datasets: {} } }], + ['string dataset', { type: 'pie', data: { datasets: ['bad'] } }], + ['null dataset', { type: 'line', data: { datasets: [null] } }], + ['array dataset', { type: 'bar', data: { datasets: [[]] } }], + ['string labels', { type: 'bar', data: { labels: 'bad' } }], + ['string options', { type: 'bar', options: 'bad' }], + ['array options', { type: 'bar', options: [] }], + ['object plugins', { type: 'bar', plugins: {} }], + ['string plugin options', { type: 'bar', options: { plugins: 'bad' } }], + ['non-string chart type', { type: 42 }], + ['blank chart type', { type: '' }], + ['non-string dataset type', { data: { datasets: [{ type: 42 }] } }], + ['empty sparkline', { type: 'sparkline' }], + ['empty progress bar', { type: 'progressBar' }], + ['non-array sparkline data', { type: 'sparkline', data: { datasets: [{ data: {} }] } }], + ['browser reference during evaluation', '({ type: window.chartType })'], + ['throwing null during evaluation', '(() => { throw null; })()'], + ['throwing undefined during evaluation', '(() => { throw undefined; })()'], + ]; + + invalidInputs.forEach(([name, input]) => { + it(`rejects ${name} as client input`, async () => { + await assert.rejects(render(input), ChartConfigError); + }); + }); + + ['2.9.4', '3', '4'].forEach(version => { + it(`rejects unknown and missing effective chart types in v${version}`, async () => { + const Chart = + version === '2.9.4' + ? require('chart.js') + : version === '3' + ? require('chart.js-v3') + : (await import('chart.js-v4/auto')).Chart; + const instances = Object.keys(Chart.instances); + for (const chart of [ + { type: 'notAChart', data: { datasets: [{ data: [1] }] } }, + { type: 'bar', data: { datasets: [{ type: 'notAChart', data: [1] }] } }, + { data: { datasets: [{ data: [1] }] } }, + ]) { + await assert.rejects(render(chart, version), ChartConfigError); + assert.deepStrictEqual(Object.keys(Chart.instances), instances); + } + }); + + it(`preserves computed configs and callbacks in v${version}`, async () => { + const input = `(() => ({ + type: 'bar', + data: { labels: ['A'], datasets: [{ data: [1, 2].map(x => x * 2) }] }, + options: { scales: ${ + version === '2.9.4' + ? "{ yAxes: [{ ticks: { callback: x => '$' + x } }] }" + : "{ y: { ticks: { callback: x => '$' + x } } }" + } }, + plugins: [{ id: 'callback-check', beforeDraw: chart => { + if (chart.data.datasets[0].data[0] !== 2) throw new Error('Callback lost'); + } }] + }))()`; + const buf = await render(input, version); + assert(buf.length > 0); + }); + + it(`allows mixed charts with dataset-level types in v${version}`, async () => { + const buf = await render( + { + options: { scales: version === '2.9.4' ? { xAxes: [{}], yAxes: [{}] } : {} }, + data: { + labels: ['A'], + datasets: [ + { type: 'bar', data: [1] }, + { type: 'line', data: [2] }, + ], + }, + }, + version, + ); + assert(buf.length > 0); + }); + }); + + it('provides a resolved Chart helper to JavaScript expressions', async () => { + const buf = await render(`({ + type: 'bar', + data: { labels: ['A'], datasets: [{ data: [Chart.helpers.isArray([]) ? 1 : 0] }] } + })`); + assert(buf.length > 0); + }); + + it('allows inline plugins to register a custom controller', async () => { + const Chart = require('chart.js'); + try { + const buf = await render({ + type: 'bar', + data: { labels: ['A'], datasets: [{ type: 'validationTestBar', data: [1] }] }, + plugins: [ + { + beforeInit: () => { + Chart.controllers.validationTestBar = Chart.controllers.bar; + }, + }, + ], + }); + assert(buf.length > 0); + } finally { + delete Chart.controllers.validationTestBar; + } + }); + + it('allows omitted data and null defaults on empty charts', async () => { + for (const chart of [{ type: 'pie' }, { type: 'bar', data: null, options: null }]) { + const buf = await render(chart); + assert(buf.length > 0); + } + }); + + it('preserves object-map dataset data in Chart.js 3', async () => { + const buf = await render({ type: 'bar', data: { datasets: [{ data: { A: 1, B: 2 } }] } }, '3'); + assert(buf.length > 0); + }); + + it('keeps server-side animation disabled by the renderer', async () => { + const buf = await render({ + type: 'bar', + data: { labels: ['A'], datasets: [{ data: [1] }] }, + options: { animation: { duration: 1000 } }, + }); + assert(buf.length > 0); + }); +}); + +describe('chart validation responses and logging', () => { + let originalLoggers; + let logs; + beforeEach(() => { + logs = []; + originalLoggers = {}; + ['info', 'warn', 'error'].forEach(level => { + originalLoggers[level] = logger[level]; + logger[level] = (...args) => logs.push({ level, args }); + }); + }); + afterEach(() => { + Object.keys(originalLoggers).forEach(level => { + logger[level] = originalLoggers[level]; + }); + }); + + ['png', 'svg', 'pdf'].forEach(format => { + ['get', 'post'].forEach(method => { + it(`returns a useful 400 for ${method.toUpperCase()} ${format} without error logs`, async () => { + const params = { chart: '"graph TD; A --> B"', format }; + const req = request(app)[method]('/chart'); + const res = await (method === 'get' ? req.query(params) : req.send(params)).expect(400); + assert(res.headers['x-quickchart-error'].includes('expected an object')); + const contentTypes = { png: 'image/png', svg: 'image/svg+xml', pdf: 'application/pdf' }; + assert.equal(res.headers['content-type'], contentTypes[format]); + assert.deepStrictEqual(logs, [ + { + level: 'info', + args: [{ code: 'INVALID_CHART_CONFIG' }, 'Chart configuration rejected'], + }, + ]); + }); + }); + }); + + it('classifies JSON body shapes, syntax errors, references, and invalid chart types', async () => { + for (const chart of [ + { type: 'bar', data: 'bad' }, + { type: 'chart', data: { datasets: [{ data: [1] }] } }, + 'graph TD; A --> B', + '({type: window.chartType})', + ]) { + await request(app) + .post('/chart') + .send({ chart }) + .expect(400); + } + assert(logs.every(log => log.level === 'info')); + assert(!JSON.stringify(logs).includes('stack')); + assert(!JSON.stringify(logs).includes('graph TD')); + }); + + it('validates base64-decoded configurations', async () => { + await request(app) + .get('/chart') + .query({ + chart: Buffer.from('"graph TD; A --> B"').toString('base64'), + encoding: 'base64', + }) + .expect(400); + assert(logs.every(log => log.level === 'info')); + await request(app) + .post('/chart') + .send({ chart: {}, encoding: 'base64' }) + .expect(400); + }); + + it('returns 400 for missing configs in every output format', async () => { + for (const format of ['png', 'svg', 'pdf']) { + const res = await request(app) + .get('/chart') + .query({ format }) + .expect(400); + assert(res.headers['x-quickchart-error'].includes('missing variable')); + } + }); + + it('escapes evaluation error text in SVG responses', async () => { + const res = await request(app) + .get('/chart') + .query({ + chart: '(() => { throw new Error(""); })()', + format: 'svg', + }) + .expect(400); + const body = Buffer.isBuffer(res.body) ? res.body.toString() : res.text; + assert(!body.includes('