Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some ArrayTrie methods throw StackOverflowError when cointaining a very large entry #7517

Closed
lorban opened this issue Feb 2, 2022 · 5 comments · Fixed by #7528 or #7533
Closed

Some ArrayTrie methods throw StackOverflowError when cointaining a very large entry #7517

lorban opened this issue Feb 2, 2022 · 5 comments · Fixed by #7528 or #7533
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@lorban
Copy link
Contributor

lorban commented Feb 2, 2022

Jetty version(s)
9.4.x

Description
keySet() and toString() recurse too deep on large entries, so they're likely to throw StackOverflowError.

Consider the following:

Trie<String> trie = new ArrayTrie<>(Character.MAX_VALUE - 1);

char[] c1 = new char[Character.MAX_VALUE - 1];
Arrays.fill(c1, 'a');
String huge = new String(c1);
assertTrue(trie.put(huge, "wow"));
assertThat(trie.get(huge), is("wow"));

assertThat(trie.keySet(), notNullValue());   // <--- throws StackOverflowError
assertThat(trie.toString(), notNullValue()); // <--- throws StackOverflowError
@lorban lorban added the Bug For general bugs on Jetty side label Feb 2, 2022
@lorban lorban linked a pull request Feb 3, 2022 that will close this issue
@lorban lorban added this to To do in Jetty 9.4.46 (FROZEN) via automation Feb 3, 2022
@lorban lorban moved this from To do to Reviewer approved in Jetty 9.4.46 (FROZEN) Feb 3, 2022
@lorban lorban moved this from Reviewer approved to Review in progress in Jetty 9.4.46 (FROZEN) Feb 3, 2022
gregw added a commit that referenced this issue Feb 7, 2022
* 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>
@joakime joakime moved this from Review in progress to In progress in Jetty 9.4.46 (FROZEN) Feb 17, 2022
@joakime joakime added this to To do in Jetty 10.0.9/11.0.9 (FROZEN) via automation Feb 17, 2022
@joakime joakime moved this from To do to In progress in Jetty 10.0.9/11.0.9 (FROZEN) Feb 17, 2022
@joakime
Copy link
Contributor

joakime commented Feb 17, 2022

@lorban is this done/complete now?

@lorban lorban linked a pull request Feb 18, 2022 that will close this issue
@lorban lorban moved this from In progress to Done in Jetty 9.4.46 (FROZEN) Feb 18, 2022
@lorban lorban moved this from In progress to Done in Jetty 10.0.9/11.0.9 (FROZEN) Feb 18, 2022
@lorban
Copy link
Contributor Author

lorban commented Feb 18, 2022

@joakime yes, it's all done and merged across the necessary branches. Thanks for the heads up.

@lorban lorban closed this as completed Feb 18, 2022
@gregw
Copy link
Contributor

gregw commented Feb 18, 2022

@lorban did these changes make it into the jetty-12 branch? I think we need a process to track such issues?

@gregw gregw reopened this Feb 18, 2022
Jetty 10.0.9/11.0.9 (FROZEN) automation moved this from Done to In progress Feb 18, 2022
Jetty 9.4.46 (FROZEN) automation moved this from Done to In progress Feb 18, 2022
@lorban
Copy link
Contributor Author

lorban commented Feb 18, 2022

@gregw no, they did not get merged to 12.0.x and you're right that we're going to need a process to remind us that.

I'm going to merge that right away.

lorban added a commit that referenced this issue Feb 18, 2022
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
Copy link
Contributor Author

lorban commented Feb 18, 2022

Merged to 12.0.x.

@lorban lorban closed this as completed Feb 18, 2022
Jetty 10.0.9/11.0.9 (FROZEN) automation moved this from In progress to Done Feb 18, 2022
Jetty 9.4.46 (FROZEN) automation moved this from In progress to Done Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
3 participants