Skip to content

Fix the bug that numeric overflow might occur when refilling tokens in ParamFlowChecker - #838

Merged
sczyh30 merged 1 commit into
masterfrom
fix/param-token-overflow
Jun 18, 2019
Merged

Fix the bug that numeric overflow might occur when refilling tokens in ParamFlowChecker#838
sczyh30 merged 1 commit into
masterfrom
fix/param-token-overflow

Conversation

@sczyh30

@sczyh30 sczyh30 commented Jun 16, 2019

Copy link
Copy Markdown
Member

Signed-off-by: Eric Zhao sczyh16@gmail.com

Describe what this PR does / why we need it

Fix the bug that numeric overflow might occur when refilling tokens in ParamFlowChecker:

https://github.com/alibaba/Sentinel/blob/1.6.1/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowChecker.java#L168

int toAddCount = (int)((passTime * tokenCount) / (rule.getDurationInSec() * 1000));

In the scenario of infrequent API invocations, when the passTime (i.e. time duration from the last request) and tokenCount are both large, the toAddCount might overflow.

Does this pull request fix one issue?

Fixes #837

Describe how you did it

Use long instead of int. The internal has been replaced with AtomicLong.

Describe how to verify it

Run the test cases.

Special notes for reviews

NONE

…n ParamFlowChecker

- use AtomicLong to replace AtomicInteger

Signed-off-by: Eric Zhao <sczyh16@gmail.com>
@sczyh30 sczyh30 added the to-review To review label Jun 16, 2019
@sczyh30 sczyh30 added this to the 1.6.2 milestone Jun 16, 2019

@CarpenterLee CarpenterLee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

ParamFlowChecker has an int overflow risk

2 participants