Skip to content

Conversation

@ianks
Copy link
Contributor

@ianks ianks commented Jul 26, 2016

Previously, calling getLabelMoment with an out of bound index would cause an
error such as this:

Uncaught TypeError: Cannot read property 'null' of undefined

This happens because there is not always guaranteed to be a labelMoment on
at the current datasetIndex.

One example of this is practice comes from a this function call:

// since the are not always guaranteed to be at least two labelMoments
//                                \ / this index can be out of bounds
//                                 |
var tickWidth = me.getPixelForTick(1) - me.getPixelForTick(0) - 6;

This patch simply ensures that the labelMoments for the datasetIndex are
defined before accessing properties on it.

Previously, calling getLabelMoment with an out of bound index would cause an
error such as this:

```
Uncaught TypeError: Cannot read property 'null' of undefined
```

This happens because there is not always guaranteed to be a labelMoment on
at the current datasetIndex.

One example of this is practice comes from a this function call:

```js
// since the are not always guaranteed to be at least two labelMoments
//                                \ / this index can be out of bounds
//                                 |
var tickWidth = me.getPixelForTick(1) - me.getPixelForTick(0) - 6;
```

This patch simply ensures that the `labelMoments` for the `datasetIndex` are
defined before accessing properties on it.
@etimberg etimberg merged commit 8e2deed into chartjs:master Jul 27, 2016
@etimberg
Copy link
Member

Thanks @ianks

@courchef
Copy link
Contributor

courchef commented Aug 2, 2016

BTW, this was already solved by my pull request #3024, yet still opened because of a Travis CI build timeout error...

Also, with the use of the plugin Chart.Zoom, the value of index can also be null, so this fix is incomplete.

exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Fix out of bounds index access in getLabelMoment
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.

3 participants