From bdc60ee7119d70c73a3122fbfe4572d060bb2a3b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 8 Feb 2022 06:20:50 +1100 Subject: [PATCH] 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 * Jetty 9.4.x 7517 trie stack overflow (#7528) Fix #7518 Trie stack overflows * Avoid recursion where possible Signed-off-by: Greg Wilkins * Added extra tests Signed-off-by: Greg Wilkins * removed empty file Signed-off-by: Greg Wilkins --- .../java/org/eclipse/jetty/util/TreeTrie.java | 61 +++++++++++-------- .../java/org/eclipse/jetty/util/TrieTest.java | 52 +++++++++++++++- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java index a96446f6aec..77507d3e1ed 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/TreeTrie.java @@ -231,44 +231,46 @@ class TreeTrie extends AbstractTrie return getBest(_root, b, offset, len); } - private V getBest(Node t, byte[] b, int offset, int len) + private V getBest(Node node, byte[] b, int offset, int len) { for (int i = 0; i < len; i++) { + Node 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 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 extends AbstractTrie return getBest(_root, s, offset, len); } - private V getBest(Node t, String s, int offset, int len) + private V getBest(Node node, String s, int offset, int len) { for (int i = 0; i < len; i++) { + Node 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 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 extends AbstractTrie return getBest(_root, b, offset, len); } - private V getBest(Node t, ByteBuffer b, int offset, int len) + private V getBest(Node node, ByteBuffer b, int offset, int len) { + Node next; int pos = b.position() + offset; for (int i = 0; i < len; i++) { @@ -346,36 +352,37 @@ class TreeTrie extends AbstractTrie 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 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 diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java index 58c531a1e12..28a7aa3b917 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/TrieTest.java @@ -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 emptyImplementations() + { + List> impls = new ArrayList<>(); + + for (boolean caseSensitive : new boolean[] {true, false}) + { + impls.add(new ArrayTrie(caseSensitive, 128)); + impls.add(new ArrayTernaryTrie(caseSensitive, 128)); + impls.add(new TreeTrie<>(caseSensitive)); + } + + return impls.stream().map(Arguments::of); + } + @ParameterizedTest @MethodSource("implementations") public void testOverflow(AbstractTrie trie) throws Exception @@ -432,6 +447,38 @@ public class TrieTest assertThat(trie.getBest(key + ";xxxx"), is(103)); } + @ParameterizedTest + @MethodSource("emptyImplementations") + public void testHttp(AbstractTrie 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(Character.MAX_VALUE + 1)); + + assertThat(trie.keySet(), contains(huge)); + assertThat(trie.toString(), containsString(huge)); } }