Skip to content

Commit 725feeb

Browse files
authored
Fix CIDR trie lookups honoring TTLs (#26854)
* Fix CIDR trie lookups honoring TTLs * changelog
1 parent 9c28715 commit 725feeb

3 files changed

Lines changed: 68 additions & 4 deletions

File tree

changelog/unreleased/pr-26854.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
type = "f"
2+
message = "Fixed Enterprise MongoDB Data Adapter CIDR entries being cached up to one hour after their TTL had expired."
3+
4+
pulls = ["26854"]

graylog2-server/src/main/java/org/graylog2/utilities/CIDRPatriciaTrie.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,20 @@ public void insertCIDR(String cidr, String rangeName, long expireAfter) {
113113
}
114114
}
115115

116+
/**
117+
* Returns the rangeName of the range with the longest prefix that contains the IP address, ignoring any
118+
* range whose TTL has elapsed, or null if no live range contains it.
119+
*
120+
* <p>An expired range is skipped rather than terminating the search, so a lookup falls through to a
121+
* shorter enclosing range that is still live.
122+
*
123+
* @param ip IP address to check against the collection of ranges
124+
* @return the name of the longest-prefix live range containing the IP if one exists, null otherwise
125+
* @see #longestPrefixRangeLookupWithTtl(String, long) to supply an explicit lookup time, or 0 to ignore
126+
* expiry entirely
127+
*/
116128
public String longestPrefixRangeLookup(String ip) {
117-
return longestPrefixRangeLookupWithTtl(ip, 0L);
129+
return longestPrefixRangeLookupWithTtl(ip, DateTime.now(DateTimeZone.UTC).getMillis());
118130
}
119131

120132
/**

graylog2-server/src/test/java/org/graylog2/utilities/CIDRPatriciaTrieTest.java

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,12 @@ public void testCleanCopy() {
105105
// Copy should be a deep copy.
106106
assertThat(copy).isNotSameAs(trie);
107107
// No operations performed on the original trie, so all nodes should still exist in the trie.
108+
// Probed with an explicit lookup time of 0 to ignore expiry: the point here is that cleanCopy
109+
// did not mutate the original, not whether these nodes have expired (two of them have).
108110
assertThat(trie).satisfies(t -> {
109-
assertThat(t.longestPrefixRangeLookup("192.168.1.100")).isEqualTo("IPv4 Range 1");
110-
assertThat(t.longestPrefixRangeLookup("10.0.5.1")).isEqualTo("IPv4 Range 2");
111-
assertThat(t.longestPrefixRangeLookup("35.139.253.123")).isEqualTo("IPv4 Range 3");
111+
assertThat(t.longestPrefixRangeLookupWithTtl("192.168.1.100", 0L)).isEqualTo("IPv4 Range 1");
112+
assertThat(t.longestPrefixRangeLookupWithTtl("10.0.5.1", 0L)).isEqualTo("IPv4 Range 2");
113+
assertThat(t.longestPrefixRangeLookupWithTtl("35.139.253.123", 0L)).isEqualTo("IPv4 Range 3");
112114
});
113115
// Confirm two nodes with TTLs have been cleaned out of the copy and the one without a TTL has not.
114116
assertThat(copy).satisfies(t -> {
@@ -146,6 +148,52 @@ public void testCleanCopyExpiredNodes() {
146148
}
147149
}
148150

151+
@Test
152+
public void testLookupSkipsExpiredRange() {
153+
try {
154+
final CIDRPatriciaTrie trie = new CIDRPatriciaTrie();
155+
final long expireAt = DateTime.now(DateTimeZone.UTC).getMillis() + 500L;
156+
trie.insertCIDR("192.168.1.0/24", "Expiring Range", expireAt);
157+
trie.insertCIDR("10.0.0.0/8", "Permanent Range");
158+
159+
// Before expiry both resolve.
160+
assertThat(trie.longestPrefixRangeLookup("192.168.1.100")).isEqualTo("Expiring Range");
161+
assertThat(trie.longestPrefixRangeLookup("10.0.5.1")).isEqualTo("Permanent Range");
162+
163+
DateTimeUtils.setCurrentMillisOffset(501);
164+
165+
// The expired range no longer matches, even though cleanCopy has not run and the node is
166+
// still present in the trie.
167+
assertThat(trie.longestPrefixRangeLookup("192.168.1.100")).isNull();
168+
assertThat(trie.longestPrefixRangeLookupWithTtl("192.168.1.100", 0L)).isEqualTo("Expiring Range");
169+
170+
// A range without a TTL is unaffected.
171+
assertThat(trie.longestPrefixRangeLookup("10.0.5.1")).isEqualTo("Permanent Range");
172+
} finally {
173+
DateTimeUtils.setCurrentMillisSystem();
174+
}
175+
}
176+
177+
@Test
178+
public void testLookupFallsBackToLiveEnclosingRangeWhenLongestPrefixExpired() {
179+
try {
180+
final CIDRPatriciaTrie trie = new CIDRPatriciaTrie();
181+
final long expireAt = DateTime.now(DateTimeZone.UTC).getMillis() + 500L;
182+
trie.insertCIDR("10.1.2.0/24", "Specific Expiring", expireAt);
183+
trie.insertCIDR("10.0.0.0/8", "Broad Permanent");
184+
185+
// The more specific range wins while it is live.
186+
assertThat(trie.longestPrefixRangeLookup("10.1.2.42")).isEqualTo("Specific Expiring");
187+
188+
DateTimeUtils.setCurrentMillisOffset(501);
189+
190+
// Once it expires the search continues to the shorter enclosing range rather than giving up.
191+
assertThat(trie.longestPrefixRangeLookup("10.1.2.42")).isEqualTo("Broad Permanent");
192+
} finally {
193+
DateTimeUtils.setCurrentMillisSystem();
194+
}
195+
}
196+
149197
@Test
150198
public void testToBinaryIP() {
151199
String cidrBinary = CIDRPatriciaTrie.toBinaryString("2002:0000:0000:1234:0000:0000:0000:0000", 64);

0 commit comments

Comments
 (0)