Jetty 10.0.x #7517 #7518 trie fixes (#7533)

* Fix #7518 Trie.getBest with empty Key (#7527)

Fix #7518 Trie.getBest with empty Key

 * Only increment current row if not recursing.
 * Fixed match ending with big char

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Jetty 9.4.x 7517 trie stack overflow (#7528)

Fix #7518 Trie stack overflows

* Avoid recursion where possible

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* Added extra tests

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* removed empty file

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2022-02-08 06:20:50 +11:00 committed by GitHub
parent 9e41227a3b
commit bdc60ee711
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 85 additions and 28 deletions

View File

@ -231,44 +231,46 @@ class TreeTrie<V> extends AbstractTrie<V>
return getBest(_root, b, offset, len);
}
private V getBest(Node<V> t, byte[] b, int offset, int len)
private V getBest(Node<V> node, byte[] b, int offset, int len)
{
for (int i = 0; i < len; i++)
{
Node<V> next;
byte c = b[offset + i];
int index = c >= 0 && c < 0x7f ? _lookup[c] : -1;
if (index >= 0)
{
if (t._nextIndex[index] == null)
if (node._nextIndex[index] == null)
break;
t = t._nextIndex[index];
next = node._nextIndex[index];
}
else
{
Node<V> n = null;
for (int j = t._nextOther.size(); j-- > 0; )
for (int j = node._nextOther.size(); j-- > 0; )
{
n = t._nextOther.get(j);
n = node._nextOther.get(j);
if (n._c == c)
break;
n = null;
}
if (n == null)
break;
t = n;
next = n;
}
// Is the next Trie is a match
if (t._key != null)
if (node._key != null)
{
// Recurse so we can remember this possibility
V best = getBest(t, b, offset + i + 1, len - i - 1);
V best = getBest(next, b, offset + i + 1, len - i - 1);
if (best != null)
return best;
break;
}
node = next;
}
return t._value;
return node._value;
}
@Override
@ -289,44 +291,47 @@ class TreeTrie<V> extends AbstractTrie<V>
return getBest(_root, s, offset, len);
}
private V getBest(Node<V> t, String s, int offset, int len)
private V getBest(Node<V> node, String s, int offset, int len)
{
for (int i = 0; i < len; i++)
{
Node<V> next;
char c = s.charAt(offset + i);
int index = c < 0x7f ? _lookup[c] : -1;
if (index >= 0)
{
if (t._nextIndex[index] == null)
if (node._nextIndex[index] == null)
break;
t = t._nextIndex[index];
next = node._nextIndex[index];
}
else
{
Node<V> n = null;
for (int j = t._nextOther.size(); j-- > 0; )
for (int j = node._nextOther.size(); j-- > 0; )
{
n = t._nextOther.get(j);
n = node._nextOther.get(j);
if (n._c == c)
break;
n = null;
}
if (n == null)
break;
t = n;
next = n;
}
// Is the next Trie is a match
if (t._key != null)
if (node._key != null)
{
// Recurse so we can remember this possibility
V best = getBest(t, s, offset + i + 1, len - i - 1);
V best = getBest(next, s, offset + i + 1, len - i - 1);
if (best != null)
return best;
break;
}
node = next;
}
return t._value;
return node._value;
}
@Override
@ -337,8 +342,9 @@ class TreeTrie<V> extends AbstractTrie<V>
return getBest(_root, b, offset, len);
}
private V getBest(Node<V> t, ByteBuffer b, int offset, int len)
private V getBest(Node<V> node, ByteBuffer b, int offset, int len)
{
Node<V> next;
int pos = b.position() + offset;
for (int i = 0; i < len; i++)
{
@ -346,36 +352,37 @@ class TreeTrie<V> extends AbstractTrie<V>
int index = c >= 0 && c < 0x7f ? _lookup[c] : -1;
if (index >= 0)
{
if (t._nextIndex[index] == null)
if (node._nextIndex[index] == null)
break;
t = t._nextIndex[index];
next = node._nextIndex[index];
}
else
{
Node<V> n = null;
for (int j = t._nextOther.size(); j-- > 0; )
for (int j = node._nextOther.size(); j-- > 0; )
{
n = t._nextOther.get(j);
n = node._nextOther.get(j);
if (n._c == c)
break;
n = null;
}
if (n == null)
break;
t = n;
next = n;
}
// Is the next Trie is a match
if (t._key != null)
if (node._key != null)
{
// Recurse so we can remember this possibility
V best = getBest(t, b, offset + i + 1, len - i - 1);
V best = getBest(next, b, offset + i + 1, len - i - 1);
if (best != null)
return best;
break;
}
node = next;
}
return t._value;
return node._value;
}
@Override

View File

@ -29,9 +29,10 @@ import org.junit.jupiter.params.provider.MethodSource;
import static org.eclipse.jetty.util.AbstractTrie.requiredCapacity;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
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;
@ -115,6 +116,20 @@ public class TrieTest
return impls.stream().map(Arguments::of);
}
public static Stream<Arguments> emptyImplementations()
{
List<AbstractTrie<Integer>> impls = new ArrayList<>();
for (boolean caseSensitive : new boolean[] {true, false})
{
impls.add(new ArrayTrie<Integer>(caseSensitive, 128));
impls.add(new ArrayTernaryTrie<Integer>(caseSensitive, 128));
impls.add(new TreeTrie<>(caseSensitive));
}
return impls.stream().map(Arguments::of);
}
@ParameterizedTest
@MethodSource("implementations")
public void testOverflow(AbstractTrie<Integer> trie) throws Exception
@ -432,6 +447,38 @@ public class TrieTest
assertThat(trie.getBest(key + ";xxxx"), is(103));
}
@ParameterizedTest
@MethodSource("emptyImplementations")
public void testHttp(AbstractTrie<Integer> trie)
{
trie.put("Host:", 1);
trie.put("Host: name", 2);
assertThat(trie.getBest("Other: header\r\n"), nullValue());
assertThat(trie.getBest("Host: other\r\n"), is(1));
assertThat(trie.getBest("Host: name\r\n"), is(2));
if (trie.isCaseInsensitive())
assertThat(trie.getBest("HoSt: nAme\r\n"), is(2));
else
assertThat(trie.getBest("HoSt: nAme\r\n"), nullValue());
assertThat(trie.getBest(BufferUtil.toBuffer("Other: header\r\n")), nullValue());
assertThat(trie.getBest(BufferUtil.toBuffer("Host: other\r\n")), is(1));
assertThat(trie.getBest(BufferUtil.toBuffer("Host: name\r\n")), is(2));
if (trie.isCaseInsensitive())
assertThat(trie.getBest(BufferUtil.toBuffer("HoSt: nAme\r\n")), is(2));
else
assertThat(trie.getBest(BufferUtil.toBuffer("HoSt: nAme\r\n")), nullValue());
assertThat(trie.getBest(BufferUtil.toDirectBuffer("Other: header\r\n")), nullValue());
assertThat(trie.getBest(BufferUtil.toDirectBuffer("Host: other\r\n")), is(1));
assertThat(trie.getBest(BufferUtil.toDirectBuffer("Host: name\r\n")), is(2));
if (trie.isCaseInsensitive())
assertThat(trie.getBest(BufferUtil.toDirectBuffer("HoSt: nAme\r\n")), is(2));
else
assertThat(trie.getBest(BufferUtil.toDirectBuffer("HoSt: nAme\r\n")), nullValue());
}
@Test
public void testArrayTrieCapacity()
{
@ -441,5 +488,8 @@ public class TrieTest
assertThat(trie.get(huge), is("wow"));
assertThrows(IllegalArgumentException.class, () -> new ArrayTrie<String>(Character.MAX_VALUE + 1));
assertThat(trie.keySet(), contains(huge));
assertThat(trie.toString(), containsString(huge));
}
}