Skip to content

chart: fix division by zero and step overflow in json engine - #2011

Open
reginaldalfret wants to merge 1 commit into
Netflix:mainfrom
reginaldalfret:fix/json-graph-engine-subsecond-step-overflow
Open

reginaldalfret wants to merge 1 commit into
Netflix:mainfrom
reginaldalfret:fix/json-graph-engine-subsecond-step-overflow

Conversation

@reginaldalfret

Copy link
Copy Markdown

Summary

This PR addresses two edge-case issues in JsonGraphEngine:

  1. Sub-second step division by zero: When serializing the explain metadata block, graphDatapoints was computed as graphLines * ((end - start) / (config.step / 1000) + 1). When queries use sub-second step sizes (such as 500ms, 100ms, etc., which are valid in Step.allowedStepSizes), integer division (config.step / 1000) truncates to 0, resulting in ArithmeticException: / by zero. Dividing directly by config.step in milliseconds avoids division by zero while preserving the exact datapoint count.
  2. Large step integer overflow: config.step was cast to Int via .asInstanceOf[Int]. For large step sizes (> 24.8 days in milliseconds), this overflows to negative values, causing the values loop (timestamp += step) to decrease and loop infinitely. Keeping step as Long matches CsvGraphEngine and prevents overflow.
  3. Test additions: Added regression test coverage for explain stats, sub-second step handling, large step rollover, and stats.json in JsonGraphEngineSuite. Also normalized newline comparison in CsvGraphEngineSuite for cross-platform consistency.

Verification

All 335 tests in atlas-chart pass:

  • atlas-chart/test (335 passed, 0 failed)
  • atlas-chart/scalafmtCheck passed
  • checkLicenseHeaders passed

In JsonGraphEngine, graphDatapoints was calculated by dividing (config.step / 1000). For sub-second step sizes (such as 500ms or 100ms, supported in Step.allowedStepSizes), integer division evaluates to 0 and throws ArithmeticException: / by zero. Using config.step directly in milliseconds avoids division by zero while preserving the exact datapoint count.

Additionally, step was cast to Int, which overflows for large step intervals (> 24.8 days) and causes timestamp += step to decrease in an infinite loop. Keep step as Long.

Also add unit test coverage for stats.json and ensure test string stripping is newline-agnostic.

Signed-off-by: reginaldalfret <reginaldalfret@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant