Skip to content

Commit c5f2f8b

Browse files
CaideyipiCaideyipi
committed
Fix overflow in retention cutoff calculations
1 parent a7b8657 commit c5f2f8b

8 files changed

Lines changed: 113 additions & 30 deletions

File tree

‎iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/subscription/SubscriptionWalRetentionCalculator.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,15 @@ private SubscriptionRetentionBound buildTimeRetentionBound(final long retentionM
118118
return SubscriptionRetentionBound.retainAll();
119119
}
120120

121-
final long cutoffTimeMs = System.currentTimeMillis() - retentionMs;
121+
final long cutoffTimeMs = getTimeRetentionCutoffTimeMs(System.currentTimeMillis(), retentionMs);
122122
final Pair<Long, Long> deletionBound =
123123
consensusReqReader.getDeletionBoundBeforeTimestamp(cutoffTimeMs);
124124
return SubscriptionRetentionBound.of(deletionBound.left, deletionBound.right);
125125
}
126+
127+
static long getTimeRetentionCutoffTimeMs(final long currentTimeMs, final long retentionMs) {
128+
return retentionMs > 0 && currentTimeMs < Long.MIN_VALUE + retentionMs
129+
? Long.MIN_VALUE
130+
: currentTimeMs - retentionMs;
131+
}
126132
}

‎iotdb-core/consensus/src/test/java/org/apache/iotdb/consensus/iot/subscription/SubscriptionWalRetentionCalculatorTest.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.consensus.iot.subscription;
21+
22+
import org.junit.Test;
23+
24+
import static org.junit.Assert.assertEquals;
25+
26+
public class SubscriptionWalRetentionCalculatorTest {
27+
28+
@Test
29+
public void testTimeRetentionCutoffUsesNormalSubtractionWhenSafe() {
30+
assertEquals(
31+
700L, SubscriptionWalRetentionCalculator.getTimeRetentionCutoffTimeMs(1000L, 300L));
32+
}
33+
34+
@Test
35+
public void testTimeRetentionCutoffSaturatesOnUnderflow() {
36+
assertEquals(
37+
Long.MIN_VALUE,
38+
SubscriptionWalRetentionCalculator.getTimeRetentionCutoffTimeMs(Long.MIN_VALUE, 1L));
39+
}
40+
}

‎iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/LoginLockManager.java‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.concurrent.ConcurrentHashMap;
3737
import java.util.concurrent.ConcurrentLinkedDeque;
3838
import java.util.concurrent.ConcurrentMap;
39+
import java.util.concurrent.TimeUnit;
3940

4041
public class LoginLockManager {
4142
private static final Logger LOGGER = LoggerFactory.getLogger(LoginLockManager.class);
@@ -152,7 +153,7 @@ public boolean checkLock(long userId, String ip) {
152153
UserLockInfo userIpLock = userIpLocks.get(userIpKey);
153154
if (userIpLock != null) {
154155
long now = System.currentTimeMillis();
155-
long cutoffTime = now - (passwordLockTimeMinutes * 60 * 1000L);
156+
long cutoffTime = getLockWindowCutoffTime(now);
156157
userIpLock.removeOldFailures(cutoffTime);
157158
if (userIpLock.getFailureCount() >= failedLoginAttempts) {
158159
return true;
@@ -165,7 +166,7 @@ public boolean checkLock(long userId, String ip) {
165166
UserLockInfo userLock = userLocks.get(userId);
166167
if (userLock != null) {
167168
long now = System.currentTimeMillis();
168-
long cutoffTime = now - (passwordLockTimeMinutes * 60 * 1000L);
169+
long cutoffTime = getLockWindowCutoffTime(now);
169170
userLock.removeOldFailures(cutoffTime);
170171
return userLock.getFailureCount() >= failedLoginAttemptsPerUser;
171172
}
@@ -196,7 +197,7 @@ public void recordFailure(long userId, String ip) {
196197
}
197198

198199
long now = System.currentTimeMillis();
199-
long cutoffTime = now - (passwordLockTimeMinutes * 60 * 1000L);
200+
long cutoffTime = getLockWindowCutoffTime(now);
200201

201202
// Handle user@ip failures in sliding window
202203
if (failedLoginAttempts != -1) {
@@ -288,7 +289,7 @@ public void unlock(long userId, String ip) {
288289
/** Clean up expired locks (no failures in the sliding window) */
289290
public void cleanExpiredLocks() {
290291
long now = System.currentTimeMillis();
291-
long cutoffTime = now - (passwordLockTimeMinutes * 60 * 1000L);
292+
long cutoffTime = getLockWindowCutoffTime(now);
292293

293294
// Clean expired user locks
294295
userLocks
@@ -330,6 +331,17 @@ private String buildUserIpKey(long userId, String ip) {
330331
return userId + "@" + ip;
331332
}
332333

334+
private long getLockWindowCutoffTime(long currentTimeMillis) {
335+
return getLockWindowCutoffTime(currentTimeMillis, passwordLockTimeMinutes);
336+
}
337+
338+
static long getLockWindowCutoffTime(long currentTimeMillis, int passwordLockTimeMinutes) {
339+
final long lockWindowMs = TimeUnit.MINUTES.toMillis(passwordLockTimeMinutes);
340+
return currentTimeMillis < Long.MIN_VALUE + lockWindowMs
341+
? Long.MIN_VALUE
342+
: currentTimeMillis - lockWindowMs;
343+
}
344+
333345
private void checkForPotentialAttacks(long userId, String ip) {
334346
// Check if IP is locked by many users
335347
Set<Long> usersForIp = new HashSet<>();

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/LoginLockManagerTest.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.concurrent.ExecutorService;
3535
import java.util.concurrent.Executors;
3636
import java.util.concurrent.Future;
37+
import java.util.concurrent.TimeUnit;
3738
import java.util.concurrent.atomic.AtomicBoolean;
3839

3940
import static org.junit.Assert.assertEquals;
@@ -108,6 +109,24 @@ public void testAllConfigScenarios() {
108109
assertEquals(30, getField(customLockTime, "passwordLockTimeMinutes"));
109110
}
110111

112+
@Test
113+
public void testLargeLockTimeDoesNotOverflowCutoff() {
114+
long now = 1_000_000L;
115+
assertEquals(
116+
now - TimeUnit.MINUTES.toMillis(Integer.MAX_VALUE),
117+
LoginLockManager.getLockWindowCutoffTime(now, Integer.MAX_VALUE));
118+
assertEquals(Long.MIN_VALUE, LoginLockManager.getLockWindowCutoffTime(Long.MIN_VALUE, 1));
119+
120+
LoginLockManager largeWindowManager =
121+
new LoginLockManager(failedLoginAttempts, failedLoginAttemptsPerUser, Integer.MAX_VALUE);
122+
for (int i = 0; i < failedLoginAttempts; i++) {
123+
largeWindowManager.recordFailure(TEST_USER_ID, TEST_IP);
124+
}
125+
assertTrue(
126+
"Large lock window should not overflow and discard recent failures",
127+
largeWindowManager.checkLock(TEST_USER_ID, TEST_IP));
128+
}
129+
111130
private int getField(LoginLockManager manager, String fieldName) {
112131
try {
113132
java.lang.reflect.Field field = LoginLockManager.class.getDeclaredField(fieldName);
@@ -285,7 +304,8 @@ public void testCleanExpiredLocks() {
285304
Deque<Long> timestamps = (Deque<Long>) tsField.get(lockInfo);
286305

287306
timestamps.clear();
288-
timestamps.add(System.currentTimeMillis() - (passwordLockTimeMinutes * 60 * 1000L) - 5000);
307+
timestamps.add(
308+
System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(passwordLockTimeMinutes) - 5000);
289309

290310
} catch (Exception e) {
291311
fail("Failed to modify failure timestamps via reflection: " + e.getMessage());
@@ -337,7 +357,8 @@ public void testFailuresOutsideWindowNotCounted() throws Exception {
337357
Deque<Long> timestamps = (Deque<Long>) tsField.get(lockInfo);
338358

339359
timestamps.clear();
340-
timestamps.add(System.currentTimeMillis() - (passwordLockTimeMinutes * 60 * 1000L) - 5000);
360+
timestamps.add(
361+
System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(passwordLockTimeMinutes) - 5000);
341362

342363
} catch (Exception e) {
343364
fail("Reflection modification failed: " + e.getMessage());

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/dataregion/compaction/cross/CrossSpaceCompactionSelectorTest.java‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.apache.iotdb.db.storageengine.dataregion.tsfile.TsFileResource;
3636
import org.apache.iotdb.db.storageengine.dataregion.tsfile.TsFileResourceStatus;
3737
import org.apache.iotdb.db.storageengine.rescon.memory.SystemInfo;
38+
import org.apache.iotdb.db.utils.CommonUtils;
3839
import org.apache.iotdb.db.utils.datastructure.FixedPriorityBlockingQueue;
3940

4041
import org.apache.tsfile.exception.write.WriteProcessException;
@@ -272,7 +273,7 @@ public void testSeqFileWithDeviceIndexBeenDeletedBeforeSelection()
272273
"", "", 0, null, new CompactionScheduleContext());
273274
CrossSpaceCompactionCandidate candidate =
274275
new CrossSpaceCompactionCandidate(
275-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
276+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
276277

277278
CrossCompactionTaskResource crossCompactionTaskResource =
278279
selector.selectOneTaskResources(candidate);
@@ -343,7 +344,7 @@ public void testSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndAfterCopyingL
343344
// copy candidate source file list and add read lock
344345
CrossSpaceCompactionCandidate candidate =
345346
new CrossSpaceCompactionCandidate(
346-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
347+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
347348

348349
// the other thread holds write lock and delete file successfully after copying list
349350
cd1.countDown();
@@ -406,7 +407,7 @@ public void testSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndBeforeSetting
406407
// copy candidate source file list and add read lock
407408
CrossSpaceCompactionCandidate candidate =
408409
new CrossSpaceCompactionCandidate(
409-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
410+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
410411

411412
CrossCompactionTaskResource crossCompactionTaskResource =
412413
selector.selectOneTaskResources(candidate);
@@ -512,7 +513,7 @@ public void testSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndBeforeSetting
512513
// copy candidate source file list and add read lock
513514
CrossSpaceCompactionCandidate candidate =
514515
new CrossSpaceCompactionCandidate(
515-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
516+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
516517

517518
CrossCompactionTaskResource crossCompactionTaskResource =
518519
selector.selectOneTaskResources(candidate);
@@ -633,7 +634,7 @@ public void testSeqFileWithFileIndexBeenDeletedDuringSelectionAndAfterCopyingLis
633634
// copy candidate source file list and add read lock
634635
CrossSpaceCompactionCandidate candidate =
635636
new CrossSpaceCompactionCandidate(
636-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
637+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
637638

638639
// the other thread holds write lock and delete file successfully after copying list
639640
cd1.countDown();
@@ -750,7 +751,7 @@ public void testSeqFileWithFileIndexBeenDeletedDuringSelectionAndBeforeSettingCa
750751
// copy candidate source file list and add read lock
751752
CrossSpaceCompactionCandidate candidate =
752753
new CrossSpaceCompactionCandidate(
753-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
754+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
754755

755756
CrossCompactionTaskResource crossCompactionTaskResource =
756757
selector.selectOneTaskResources(candidate);
@@ -860,7 +861,7 @@ public void testSeqFileWithFileIndexBeenDeletedDuringSelectionAndBeforeSettingCo
860861
// copy candidate source file list and add read lock
861862
CrossSpaceCompactionCandidate candidate =
862863
new CrossSpaceCompactionCandidate(
863-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
864+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
864865

865866
CrossCompactionTaskResource crossCompactionTaskResource =
866867
selector.selectOneTaskResources(candidate);
@@ -982,7 +983,7 @@ public void testSeqFileWithFileIndexBeenDeletedBeforeSelection()
982983
"", "", 0, null, new CompactionScheduleContext());
983984
CrossSpaceCompactionCandidate candidate =
984985
new CrossSpaceCompactionCandidate(
985-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
986+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
986987

987988
CrossCompactionTaskResource crossCompactionTaskResource =
988989
selector.selectOneTaskResources(candidate);
@@ -1054,7 +1055,7 @@ public void testUnSeqFileWithDeviceIndexBeenDeletedBeforeSelection()
10541055
"", "", 0, null, new CompactionScheduleContext());
10551056
CrossSpaceCompactionCandidate candidate =
10561057
new CrossSpaceCompactionCandidate(
1057-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1058+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
10581059

10591060
CrossCompactionTaskResource crossCompactionTaskResource =
10601061
selector.selectOneTaskResources(candidate);
@@ -1125,7 +1126,7 @@ public void testUnSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndAfterCopyin
11251126
// copy candidate source file list and add read lock
11261127
CrossSpaceCompactionCandidate candidate =
11271128
new CrossSpaceCompactionCandidate(
1128-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1129+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
11291130

11301131
// the other thread holds write lock and delete file successfully after copying list
11311132
cd1.countDown();
@@ -1189,7 +1190,7 @@ public void testUnSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndBeforeSetti
11891190
// copy candidate source file list and add read lock
11901191
CrossSpaceCompactionCandidate candidate =
11911192
new CrossSpaceCompactionCandidate(
1192-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1193+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
11931194

11941195
CrossCompactionTaskResource crossCompactionTaskResource =
11951196
selector.selectOneTaskResources(candidate);
@@ -1296,7 +1297,7 @@ public void testUnSeqFileWithDeviceIndexBeenDeletedDuringSelectionAndBeforeSetti
12961297
// copy candidate source file list and add read lock
12971298
CrossSpaceCompactionCandidate candidate =
12981299
new CrossSpaceCompactionCandidate(
1299-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1300+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
13001301

13011302
CrossCompactionTaskResource crossCompactionTaskResource =
13021303
selector.selectOneTaskResources(candidate);
@@ -1418,7 +1419,7 @@ public void testUnSeqFileWithFileIndexBeenDeletedDuringSelectionAndAfterCopyingL
14181419
// copy candidate source file list and add read lock
14191420
CrossSpaceCompactionCandidate candidate =
14201421
new CrossSpaceCompactionCandidate(
1421-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1422+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
14221423

14231424
// the other thread holds write lock and delete file successfully after copying list
14241425
cd1.countDown();
@@ -1534,7 +1535,7 @@ public void testUnSeqFileWithFileIndexBeenDeletedDuringSelectionAndBeforeSetting
15341535
// copy candidate source file list and add read lock
15351536
CrossSpaceCompactionCandidate candidate =
15361537
new CrossSpaceCompactionCandidate(
1537-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1538+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
15381539

15391540
CrossCompactionTaskResource crossCompactionTaskResource =
15401541
selector.selectOneTaskResources(candidate);
@@ -1644,7 +1645,7 @@ public void testUnSeqFileWithFileIndexBeenDeletedDuringSelectionAndBeforeSetting
16441645
// copy candidate source file list and add read lock
16451646
CrossSpaceCompactionCandidate candidate =
16461647
new CrossSpaceCompactionCandidate(
1647-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1648+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
16481649

16491650
CrossCompactionTaskResource crossCompactionTaskResource =
16501651
selector.selectOneTaskResources(candidate);
@@ -1766,7 +1767,7 @@ public void testUnSeqFileWithFileIndexBeenDeletedBeforeSelection()
17661767
"", "", 0, null, new CompactionScheduleContext());
17671768
CrossSpaceCompactionCandidate candidate =
17681769
new CrossSpaceCompactionCandidate(
1769-
seqResources, unseqResources, System.currentTimeMillis() - Long.MAX_VALUE);
1770+
seqResources, unseqResources, CommonUtils.getTTLLowerBound(Long.MAX_VALUE));
17701771

17711772
CrossCompactionTaskResource crossCompactionTaskResource =
17721773
selector.selectOneTaskResources(candidate);

‎iotdb-core/datanode/src/test/java/org/apache/iotdb/db/storageengine/dataregion/compaction/cross/CrossSpaceCompactionWithFastPerformerTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import org.apache.iotdb.db.storageengine.dataregion.tsfile.TsFileResourceList;
4444
import org.apache.iotdb.db.storageengine.dataregion.tsfile.TsFileResourceStatus;
4545
import org.apache.iotdb.db.storageengine.dataregion.tsfile.generator.TsFileNameGenerator;
46+
import org.apache.iotdb.db.utils.CommonUtils;
4647
import org.apache.iotdb.db.utils.EnvironmentUtils;
4748

4849
import org.apache.tsfile.read.TimeValuePair;
@@ -407,7 +408,7 @@ public void testOneSeqFileAndSixUnseqFile() throws Exception {
407408
seqTsFileResourceList.addAll(seqResources);
408409
TsFileResourceList unseqTsFileResourceList = new TsFileResourceList();
409410
unseqTsFileResourceList.addAll(unseqResources);
410-
long timeLowerBound = System.currentTimeMillis() - Long.MAX_VALUE;
411+
long timeLowerBound = CommonUtils.getTTLLowerBound(Long.MAX_VALUE);
411412
CrossSpaceCompactionCandidate mergeResource =
412413
new CrossSpaceCompactionCandidate(
413414
seqTsFileResourceList, unseqTsFileResourceList, timeLowerBound);
@@ -709,7 +710,7 @@ public void testFiveSeqFileAndOneUnseqFileWithSomeDeviceNotInSeqFiles() throws E
709710
seqTsFileResourceList.addAll(seqResources);
710711
TsFileResourceList unseqTsFileResourceList = new TsFileResourceList();
711712
unseqTsFileResourceList.addAll(unseqResources);
712-
long timeLowerBound = System.currentTimeMillis() - Long.MAX_VALUE;
713+
long timeLowerBound = CommonUtils.getTTLLowerBound(Long.MAX_VALUE);
713714
CrossSpaceCompactionCandidate mergeResource =
714715
new CrossSpaceCompactionCandidate(
715716
seqTsFileResourceList, unseqTsFileResourceList, timeLowerBound);
@@ -1009,7 +1010,7 @@ public void testFiveSeqFileAndOneUnseqFile() throws Exception {
10091010
seqTsFileResourceList.addAll(seqResources);
10101011
TsFileResourceList unseqTsFileResourceList = new TsFileResourceList();
10111012
unseqTsFileResourceList.addAll(unseqResources);
1012-
long timeLowerBound = System.currentTimeMillis() - Long.MAX_VALUE;
1013+
long timeLowerBound = CommonUtils.getTTLLowerBound(Long.MAX_VALUE);
10131014
CrossSpaceCompactionCandidate mergeResource =
10141015
new CrossSpaceCompactionCandidate(
10151016
seqTsFileResourceList, unseqTsFileResourceList, timeLowerBound);

0 commit comments

Comments
 (0)