#7496 Fix tries mistakenly throwing ArrayIndexOutOfBoundsException

Fixes #7496 fix getBest() throwing ArrayIndexOutOfBoundsException on full tries

Fixing jetty-maven-plugin IT test javax-annotation-api failure

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Co-authored-by: Greg Wilkins <gregw@webtide.com>
Co-authored-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
This commit is contained in:
Ludovic Orban 2022-02-01 15:18:22 +01:00
parent 6c66ec509c
commit 73832c4ab8
4 changed files with 94 additions and 25 deletions

View File

@ -71,7 +71,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
* the 16 bit indexes can overflow and the trie
* cannot find existing entries anymore.
*/
private static final int MAX_CAPACITY = Character.MAX_VALUE;
private static final int MAX_CAPACITY = Character.MAX_VALUE - 1;
/**
* The Trie rows in a single array which allows a lookup of row,character
@ -111,9 +111,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
super(caseSensitive);
if (capacity > MAX_CAPACITY)
throw new IllegalArgumentException("ArrayTernaryTrie maximum capacity overflow (" + capacity + " > " + MAX_CAPACITY + ")");
_value = (V[])new Object[capacity];
_tree = new char[capacity * ROW_SIZE];
_key = new String[capacity];
_value = (V[])new Object[capacity + 1];
_tree = new char[(capacity + 1) * ROW_SIZE];
_key = new String[capacity + 1];
}
@Override
@ -134,7 +134,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
for (int k = 0; k < limit; k++)
{
char c = s.charAt(k);
if (isCaseInsensitive())
if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c);
while (true)
@ -144,12 +144,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
// Do we need to create the new row?
if (t == _rows)
{
_rows++;
if (_rows > _key.length)
{
_rows--;
_rows = (char)MathUtils.cappedAdd(_rows, 1, _key.length);
if (_rows == _key.length)
return false;
}
_tree[row] = c;
}
@ -177,12 +174,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
// Do we need to create the new row?
if (t == _rows)
{
_rows++;
if (_rows > _key.length)
{
_rows--;
if (_rows == _key.length)
return false;
}
_rows++;
}
// Put the key and value
@ -199,7 +193,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
for (int i = 0; i < len; )
{
char c = s.charAt(offset + i++);
if (isCaseInsensitive())
if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c);
while (true)
@ -257,7 +251,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
}
}
return (V)_value[t];
return _value[t];
}
@Override
@ -281,7 +275,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
{
char c = s.charAt(offset++);
len--;
if (isCaseInsensitive())
if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c);
while (true)
@ -312,7 +306,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
break loop;
}
}
return (V)_value[node];
return _value[node];
}
@Override
@ -369,7 +363,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
break loop;
}
}
return (V)_value[node];
return _value[node];
}
private V getBest(int t, ByteBuffer b, int offset, int len)
@ -380,6 +374,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
loop:
for (int i = 0; i < len; i++)
{
if (o + i >= b.limit())
return null;
byte c = (byte)(b.get(o + i) & 0x7f);
if (isCaseInsensitive())
c = StringUtil.asciiToLowerCase(c);
@ -442,7 +439,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
{
Set<String> keys = new HashSet<>();
for (int r = 0; r <= _rows; r++)
for (int r = 0; r < _rows; r++)
{
if (_key[r] != null && _value[r] != null)
keys.add(_key[r]);
@ -453,7 +450,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public int size()
{
int s = 0;
for (int r = 0; r <= _rows; r++)
for (int r = 0; r < _rows; r++)
{
if (_key[r] != null && _value[r] != null)
s++;
@ -463,7 +460,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public boolean isEmpty()
{
for (int r = 0; r <= _rows; r++)
for (int r = 0; r < _rows; r++)
{
if (_key[r] != null && _value[r] != null)
return false;
@ -474,7 +471,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public Set<Map.Entry<String, V>> entrySet()
{
Set<Map.Entry<String, V>> entries = new HashSet<>();
for (int r = 0; r <= _rows; r++)
for (int r = 0; r < _rows; r++)
{
if (_key[r] != null && _value[r] != null)
entries.add(new AbstractMap.SimpleEntry<>(_key[r], _value[r]));
@ -559,7 +556,10 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
boolean added = _trie.put(s, v);
while (!added && _growby > 0)
{
ArrayTernaryTrie<V> bigger = new ArrayTernaryTrie<>(_trie.isCaseInsensitive(), _trie._key.length + _growby);
int newCapacity = _trie._key.length + _growby;
if (newCapacity > MAX_CAPACITY)
return false;
ArrayTernaryTrie<V> bigger = new ArrayTernaryTrie<>(_trie.isCaseInsensitive(), newCapacity);
for (Map.Entry<String, V> entry : _trie.entrySet())
{
bigger.put(entry.getKey(), entry.getValue());

View File

@ -450,6 +450,9 @@ class ArrayTrie<V> extends AbstractTrie<V>
int pos = b.position() + offset;
for (int i = 0; i < len; i++)
{
if (pos >= b.limit())
return null;
byte c = b.get(pos++);
int next = lookup(row, (char)(c & 0xff));
if (next < 0)

View File

@ -57,4 +57,24 @@ public class MathUtils
return Long.MAX_VALUE;
}
}
/**
* Returns the sum of its arguments, capping to {@code maxValue}.
*
* @param a the first value
* @param b the second value
* @return the sum of the values, capped to {@code maxValue}
*/
public static int cappedAdd(int a, int b, int maxValue)
{
try
{
int sum = Math.addExact(a, b);
return Math.min(sum, maxValue);
}
catch (ArithmeticException x)
{
return maxValue;
}
}
}

View File

@ -13,6 +13,8 @@
package org.eclipse.jetty.util;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@ -29,7 +31,9 @@ import static org.eclipse.jetty.util.AbstractTrie.requiredCapacity;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -111,6 +115,48 @@ public class TrieTest
return impls.stream().map(Arguments::of);
}
@ParameterizedTest
@MethodSource("implementations")
public void testOverflow(AbstractTrie<Integer> trie) throws Exception
{
int i = 0;
while (true)
{
if (++i > 10000)
break; // must not be fixed size
if (!trie.put("prefix" + i, i))
{
String key = "prefix" + i;
// Assert that all keys can be gotten.
for (String k : trie.keySet())
{
assertNotNull(trie.get(k));
assertNotNull(trie.get(toAsciiDirectByteBuffer(k, 0))); // has to be a direct buffer
assertNull(trie.get(toAsciiDirectByteBuffer(k, k.length()))); // has to be a direct buffer
}
// Assert that all getBest() variants do work on full tries.
assertNotNull(trie.getBest(key), "key=" + key);
assertNotNull(trie.getBest(key.getBytes(StandardCharsets.US_ASCII), 0, key.length()), "key=" + key);
assertNotNull(trie.getBest(toAsciiDirectByteBuffer(key, 0), 0, key.length()), "key=" + key); // has to be a direct buffer
assertNull(trie.getBest(toAsciiDirectByteBuffer(key, key.length()), 0, key.length()), "key=" + key); // has to be a direct buffer
break;
}
}
if (trie instanceof ArrayTrie || trie instanceof ArrayTernaryTrie)
assertFalse(trie.put("overflow", 0));
}
private static ByteBuffer toAsciiDirectByteBuffer(String s, int pos)
{
ByteBuffer bb = ByteBuffer.allocateDirect(s.length());
bb.put(s.getBytes(StandardCharsets.US_ASCII));
bb.position(pos);
return bb;
}
@ParameterizedTest
@MethodSource("implementations")
public void testKeySet(AbstractTrie<Integer> trie) throws Exception