From 3210a6dc9148afa60f13eaa826e2776ad729a4a9 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Tue, 30 Oct 2018 09:31:15 +0100 Subject: [PATCH] [ML] Fix a call stack size exception triggered by a negative tickInterval. (#24742) In certain cases tickInterval mistakenly could end up being negative which made getTickValues() run into a call stack size exception. This PR fixes it by a) adding a check to getTickValues() that interval must not be 0 or smaller and b) changing the way the tickInterval is determined in the Anomaly Explorer Charts. --- .../explorer_charts/explorer_chart_distribution.js | 9 ++++----- .../explorer_charts/explorer_chart_single_metric.js | 9 ++++----- x-pack/plugins/ml/public/util/chart_utils.js | 6 ++++++ x-pack/plugins/ml/public/util/chart_utils.test.js | 9 +++++++++ 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js index 8fc6a6a272823..151bd60ad4c6e 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_distribution.js @@ -239,11 +239,10 @@ export class ExplorerChartDistribution extends React.Component { timeBuckets.setInterval('auto'); const xAxisTickFormat = timeBuckets.getScaledDateFormat(); - const emphasisStart = Math.max(config.selectedEarliest, config.plotEarliest); - const emphasisEnd = Math.min(config.selectedLatest, config.plotLatest); + const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest); // +1 ms to account for the ms that was substracted for query aggregations. - const interval = emphasisEnd - emphasisStart + 1; - const tickValues = getTickValues(emphasisStart, interval, config.plotEarliest, config.plotLatest); + const interval = config.selectedLatest - config.selectedEarliest + 1; + const tickValues = getTickValues(tickValuesStart, interval, config.plotEarliest, config.plotLatest); const xAxis = d3.svg.axis().scale(lineChartXScale) .orient('bottom') @@ -283,7 +282,7 @@ export class ExplorerChartDistribution extends React.Component { .call(yAxis); if (tooManyBuckets === false) { - removeLabelOverlap(gAxis, emphasisStart, interval, vizWidth); + removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth); } } diff --git a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_single_metric.js b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_single_metric.js index 1eb9092dd2af2..f6212e766b79a 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_single_metric.js +++ b/x-pack/plugins/ml/public/explorer/explorer_charts/explorer_chart_single_metric.js @@ -192,11 +192,10 @@ export class ExplorerChartSingleMetric extends React.Component { timeBuckets.setInterval('auto'); const xAxisTickFormat = timeBuckets.getScaledDateFormat(); - const emphasisStart = Math.max(config.selectedEarliest, config.plotEarliest); - const emphasisEnd = Math.min(config.selectedLatest, config.plotLatest); + const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest); // +1 ms to account for the ms that was substracted for query aggregations. - const interval = emphasisEnd - emphasisStart + 1; - const tickValues = getTickValues(emphasisStart, interval, config.plotEarliest, config.plotLatest); + const interval = config.selectedLatest - config.selectedEarliest + 1; + const tickValues = getTickValues(tickValuesStart, interval, config.plotEarliest, config.plotLatest); const xAxis = d3.svg.axis().scale(lineChartXScale) .orient('bottom') @@ -236,7 +235,7 @@ export class ExplorerChartSingleMetric extends React.Component { .call(yAxis); if (tooManyBuckets === false) { - removeLabelOverlap(gAxis, emphasisStart, interval, vizWidth); + removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth); } } diff --git a/x-pack/plugins/ml/public/util/chart_utils.js b/x-pack/plugins/ml/public/util/chart_utils.js index af0ef308c37bd..a5965e5356501 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.js +++ b/x-pack/plugins/ml/public/util/chart_utils.js @@ -239,6 +239,12 @@ const TICK_DIRECTION = { // the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts // to align axis ticks with the gray area resembling the swimlane cell selection. export function getTickValues(startTimeMs, tickInterval, earliest, latest) { + // A tickInterval equal or smaller than 0 would trigger a call stack exception, + // so we're trying to catch that before it happens. + if (tickInterval <= 0) { + throw Error('tickInterval must be larger than 0.'); + } + const tickValues = [startTimeMs]; function addTicks(ts, operator) { diff --git a/x-pack/plugins/ml/public/util/chart_utils.test.js b/x-pack/plugins/ml/public/util/chart_utils.test.js index 88ae5551f8797..7e67edd166667 100644 --- a/x-pack/plugins/ml/public/util/chart_utils.test.js +++ b/x-pack/plugins/ml/public/util/chart_utils.test.js @@ -136,6 +136,15 @@ describe('getTickValues', () => { 1519257600000 ]); }); + + test('invalid tickIntervals trigger an error', () => { + expect(() => { + getTickValues(1518652800000, 0, 1518274800000, 1519635600000); + }).toThrow(); + expect(() => { + getTickValues(1518652800000, -604800000, 1518274800000, 1519635600000); + }).toThrow(); + }); }); describe('isLabelLengthAboveThreshold', () => {