Return client errors for malformed Chart.js configurations

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) {
   </style>
   <foreignObject width="240" height="80"
    requiredFeatures="http://www.w3.org/TR/SVG11/feature#Extensibility">
-    <p xmlns="http://www.w3.org/1999/xhtml">${msg}</p>
+    <p xmlns="http://www.w3.org/1999/xhtml">${String(msg)
+      .replace(/&/g, '&amp;')
+      .replace(/</g, '&lt;')
+      .replace(/>/g, '&gt;')}</p>
   </foreignObject>
 </svg>`);
 }
 
-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);
+      }
     });
 }
 

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 };

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(