Skip to content

Commit

Permalink
Jetty 9.4.x : fix tries mistakenly throwing ArrayIndexOutOfBoundsExce…
Browse files Browse the repository at this point in the history
…ption (#7503)

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>
  • Loading branch information
3 people authored Feb 1, 2022
1 parent 33c60d8 commit 09f4899
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ public boolean put(String s, V v)
// Do we need to create the new row?
if (t == _rows)
{
_rows = (char)MathUtils.cappedAdd(_rows, 1, _key.length);
if (_rows == _key.length)
return false;
_rows++;
_tree[row] = c;
}

Expand Down Expand Up @@ -435,6 +435,9 @@ private V getBest(int t, ByteBuffer b, int offset, int len)
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 = (byte)StringUtil.lowercases[c];
Expand Down
11 changes: 8 additions & 3 deletions jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTrie.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public ArrayTrie(int capacity)
if (capacity > MAX_CAPACITY)
throw new IllegalArgumentException("Capacity " + capacity + " > " + MAX_CAPACITY);
_value = (V[])new Object[capacity + 1];
_rowIndex = new char[(capacity + 1) * 32];
_rowIndex = new char[(capacity + 1) * ROW_SIZE];
_key = new String[capacity + 1];
}

Expand Down Expand Up @@ -171,7 +171,8 @@ public boolean put(String s, V v)
t = _rowIndex[idx];
if (t == 0)
{
if (++_rows >= _value.length)
_rows = (char)MathUtils.cappedAdd(_rows, 1, _value.length);
if (_rows == _value.length)
return false;
t = _rowIndex[idx] = _rows;
}
Expand All @@ -190,9 +191,10 @@ else if (c > 127)
t = big[c];
if (t == 0)
{
_rows = (char)MathUtils.cappedAdd(_rows, 1, _value.length);
if (_rows == _value.length)
return false;
t = big[c] = ++_rows;
t = big[c] = _rows;
}
}
}
Expand Down Expand Up @@ -368,6 +370,9 @@ private V getBest(int t, ByteBuffer b, int offset, int len)
int pos = b.position() + offset;
for (int i = 0; i < len; i++)
{
if (pos >= b.limit())
return null;

byte c = b.get(pos++);
int index = __lookup[c & 0x7f];
if (index >= 0)
Expand Down
20 changes: 20 additions & 0 deletions jetty-util/src/main/java/org/eclipse/jetty/util/MathUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,24 @@ public static long cappedAdd(long a, long b)
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;
}
}
}
25 changes: 25 additions & 0 deletions jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
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;
Expand All @@ -38,6 +39,7 @@
import static org.hamcrest.Matchers.is;
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;
Expand Down Expand Up @@ -89,13 +91,36 @@ public void testOverflow(Trie<Integer> trie) throws Exception
if (!trie.put("prefix" + i, i))
{
assertTrue(trie.isFull());
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
assertEquals(9, 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;
}
}

assertTrue(!trie.isFull() || !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(Trie<Integer> trie) throws Exception
Expand Down

0 comments on commit 09f4899

Please sign in to comment.