Skip to content

Commit 7e27c49

Browse files
qqwangxiaowrootsbera87
authored
fix default limiter overflow (#3338)
* add validation for negative elapsedtime --------- Co-authored-by: root <root@jscs02-sh001-storage-uat-02> Co-authored-by: sbera87 <[email protected]>
1 parent d83d567 commit 7e27c49

File tree

2 files changed

+34
-8
lines changed

2 files changed

+34
-8
lines changed

src/aws-cpp-sdk-core/include/aws/core/utils/ratelimiter/DefaultRateLimiter.h

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,23 @@ class DefaultRateLimiter : public RateLimiterInterface {
6161
auto now = m_elapsedTimeFunction();
6262
auto elapsedTime = (now - m_accumulatorUpdated).count();
6363

64-
// replenish the accumulator based on how much time has passed
65-
auto temp = elapsedTime * m_replenishNumerator + m_accumulatorFraction;
66-
m_accumulator += temp / m_replenishDenominator;
67-
m_accumulatorFraction = temp % m_replenishDenominator;
68-
69-
// the accumulator is capped based on the maximum rate
70-
m_accumulator = (std::min)(m_accumulator, m_maxRate);
71-
if (m_accumulator == m_maxRate) {
64+
// check for overflow case
65+
if (m_replenishNumerator != 0 && elapsedTime > 0 &&
66+
(elapsedTime > std::numeric_limits<int64_t>::max() / m_replenishNumerator ||
67+
m_accumulatorFraction > std::numeric_limits<int64_t>::max() - (elapsedTime * m_replenishNumerator))) {
68+
m_accumulator = m_maxRate;
7269
m_accumulatorFraction = 0;
70+
} else {
71+
// replenish the accumulator based on how much time has passed
72+
auto temp = elapsedTime * m_replenishNumerator + m_accumulatorFraction;
73+
m_accumulator += temp / m_replenishDenominator;
74+
m_accumulatorFraction = temp % m_replenishDenominator;
75+
76+
// the accumulator is capped based on the maximum rate
77+
m_accumulator = (std::min)(m_accumulator, m_maxRate);
78+
if (m_accumulator == m_maxRate) {
79+
m_accumulatorFraction = 0;
80+
}
7381
}
7482

7583
// if the accumulator is still negative, then we'll have to wait

tests/aws-cpp-sdk-core-tests/utils/ratelimiter/RateLimiterTests.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,24 @@ TEST_F(DefaultRateLimitTest, unnormalizedChangeRateLimitTest) {
225225
ASSERT_TRUE(delay.count() == 4910);
226226
}
227227

228+
TEST_F(DefaultRateLimitTest, overflowTest) {
229+
// 500MB/s
230+
int max_rate = 500 * 1024 * 1024;
231+
TestDefaultRateLimiter limiter(max_rate, DefaultRateLimitTest::GetTestTime);
232+
233+
// 7 days
234+
int64_t time_elapsed = 7LL * 24 * 60 * 60 * 1000000;
235+
236+
limiter.ApplyCost(0);
237+
auto delay = limiter.ApplyCost(0);
238+
ASSERT_TRUE(delay.count() == 0);
239+
240+
// The original code here would cause the sleep duration to be excessively long
241+
SetMillisecondsElapsed(time_elapsed);
242+
delay = limiter.ApplyCost(0);
243+
ASSERT_TRUE(delay.count() == 0);
244+
}
245+
228246
TEST_F(DefaultRateLimitTest, fractionalLimitTest) {
229247
TestDefaultRateLimiter limiter(100, DefaultRateLimitTest::GetTestTime);
230248

0 commit comments

Comments
 (0)