Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #7509 from eclipse/jetty-10.0.x-7496-Trie-Overflow 

Jetty 10.0.x : fix tries mistakenly throwing ArrayIndexOutOfBoundsException
This commit is contained in:
Simone Bordet 2022-02-01 19:05:31 +01:00 committed by GitHub
parent df526f351c abc4f05992
commit 66d3165a8c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 103 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java
View File

@ -71,7 +71,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
* the 16 bit indexes can overflow and the trie * the 16 bit indexes can overflow and the trie
* cannot find existing entries anymore. * 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 * 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); super(caseSensitive);
if (capacity > MAX_CAPACITY) if (capacity > MAX_CAPACITY)
throw new IllegalArgumentException("ArrayTernaryTrie maximum capacity overflow (" + capacity + " > " + MAX_CAPACITY + ")"); throw new IllegalArgumentException("ArrayTernaryTrie maximum capacity overflow (" + capacity + " > " + MAX_CAPACITY + ")");
_value = (V[])new Object[capacity]; _value = (V[])new Object[capacity + 1];
_tree = new char[capacity * ROW_SIZE]; _tree = new char[(capacity + 1) * ROW_SIZE];
_key = new String[capacity]; _key = new String[capacity + 1];
} }
@Override @Override
@ -130,26 +130,28 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
{ {
int t = 0; int t = 0;
int limit = s.length(); int limit = s.length();
if (limit > MAX_CAPACITY)
return false;
int last = 0; int last = 0;
for (int k = 0; k < limit; k++) for (int k = 0; k < limit; k++)
{ {
char c = s.charAt(k); char c = s.charAt(k);
if (isCaseInsensitive()) if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c); c = StringUtil.asciiToLowerCase(c);
while (true) while (true)
{ {
if (_rows == MAX_CAPACITY)
return false;
int row = ROW_SIZE * t; int row = ROW_SIZE * t;
// Do we need to create the new row? // Do we need to create the new row?
if (t == _rows) if (t == _rows)
{ {
_rows++; _rows = (char)MathUtils.cappedAdd(_rows, 1, _key.length);
if (_rows > _key.length) if (_rows == _key.length)
{
_rows--;
return false; return false;
}
_tree[row] = c; _tree[row] = c;
} }
@ -177,12 +179,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
// Do we need to create the new row? // Do we need to create the new row?
if (t == _rows) if (t == _rows)
{ {
_rows++; if (_rows == _key.length)
if (_rows > _key.length)
{
_rows--;
return false; return false;
} _rows++;
} }
// Put the key and value // Put the key and value
@ -199,7 +198,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
for (int i = 0; i < len; ) for (int i = 0; i < len; )
{ {
char c = s.charAt(offset + i++); char c = s.charAt(offset + i++);
if (isCaseInsensitive()) if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c); c = StringUtil.asciiToLowerCase(c);
while (true) while (true)
@ -257,7 +256,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
} }
} }
return (V)_value[t]; return _value[t];
} }
@Override @Override
@ -281,7 +280,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
{ {
char c = s.charAt(offset++); char c = s.charAt(offset++);
len--; len--;
if (isCaseInsensitive()) if (isCaseInsensitive() && c < 128)
c = StringUtil.asciiToLowerCase(c); c = StringUtil.asciiToLowerCase(c);
while (true) while (true)
@ -312,7 +311,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
break loop; break loop;
} }
} }
return (V)_value[node]; return _value[node];
} }
@Override @Override
@ -369,7 +368,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
break loop; break loop;
} }
} }
return (V)_value[node]; return _value[node];
} }
private V getBest(int t, ByteBuffer b, int offset, int len) private V getBest(int t, ByteBuffer b, int offset, int len)
@ -380,6 +379,9 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
loop: loop:
for (int i = 0; i < len; i++) for (int i = 0; i < len; i++)
{ {
if (o + i >= b.limit())
return null;
byte c = (byte)(b.get(o + i) & 0x7f); byte c = (byte)(b.get(o + i) & 0x7f);
if (isCaseInsensitive()) if (isCaseInsensitive())
c = StringUtil.asciiToLowerCase(c); c = StringUtil.asciiToLowerCase(c);
@ -412,7 +414,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
break loop; break loop;
} }
} }
return (V)_value[node]; return _value[node];
} }
@Override @Override
@ -430,7 +432,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
buf.append(','); buf.append(',');
buf.append(_key[r]); buf.append(_key[r]);
buf.append('='); buf.append('=');
buf.append(String.valueOf(_value[r])); buf.append(_value[r]);
} }
} }
buf.append('}'); buf.append('}');
@ -442,7 +444,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
{ {
Set<String> keys = new HashSet<>(); 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) if (_key[r] != null && _value[r] != null)
keys.add(_key[r]); keys.add(_key[r]);
@ -453,7 +455,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public int size() public int size()
{ {
int s = 0; int s = 0;
for (int r = 0; r <= _rows; r++) for (int r = 0; r < _rows; r++)
{ {
if (_key[r] != null && _value[r] != null) if (_key[r] != null && _value[r] != null)
s++; s++;
@ -463,7 +465,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public boolean isEmpty() public boolean isEmpty()
{ {
for (int r = 0; r <= _rows; r++) for (int r = 0; r < _rows; r++)
{ {
if (_key[r] != null && _value[r] != null) if (_key[r] != null && _value[r] != null)
return false; return false;
@ -474,7 +476,7 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
public Set<Map.Entry<String, V>> entrySet() public Set<Map.Entry<String, V>> entrySet()
{ {
Set<Map.Entry<String, V>> entries = new HashSet<>(); 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) if (_key[r] != null && _value[r] != null)
entries.add(new AbstractMap.SimpleEntry<>(_key[r], _value[r])); entries.add(new AbstractMap.SimpleEntry<>(_key[r], _value[r]));
@ -559,7 +561,10 @@ class ArrayTernaryTrie<V> extends AbstractTrie<V>
boolean added = _trie.put(s, v); boolean added = _trie.put(s, v);
while (!added && _growby > 0) 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()) for (Map.Entry<String, V> entry : _trie.entrySet())
{ {
bigger.put(entry.getKey(), entry.getValue()); bigger.put(entry.getKey(), entry.getValue());

3
jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTrie.java
View File

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

20
jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java
View File

@ -57,4 +57,24 @@ public class MathUtils
return Long.MAX_VALUE; 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;
}
}
} }

48
jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java
View File

@ -13,6 +13,8 @@
package org.eclipse.jetty.util; package org.eclipse.jetty.util;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.List; import java.util.List;
@ -29,7 +31,10 @@ import static org.eclipse.jetty.util.AbstractTrie.requiredCapacity;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue; 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.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.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
@ -110,6 +115,48 @@ public class TrieTest
return impls.stream().map(Arguments::of); 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 @ParameterizedTest
@MethodSource("implementations") @MethodSource("implementations")
public void testKeySet(AbstractTrie<Integer> trie) throws Exception public void testKeySet(AbstractTrie<Integer> trie) throws Exception
@ -295,6 +342,7 @@ public class TrieTest
testGetString(trie); testGetString(trie);
testGetBestArray(trie); testGetBestArray(trie);
testGetBestBuffer(trie); testGetBestBuffer(trie);
assertNull(trie.getBest("Large: This is a really large key and should blow the maximum size of the array trie as lots of nodes should already be used."));
} }
@Test @Test