Fix for JarHell Bootstrap Check can yield false positives #76217
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o added a unit test to test io error msg
elasticsearchmachine
added
external-contributor
BalasubramanyamEvani
changed the title
rjernst
reviewed
Aug 8, 2021
| throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to io error, caused when checking for Jar Hell", ioe); | ||
| } catch (final URISyntaxException urie) { | ||
| throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to malformed uri, caused when checking for Jar Hell", urie); | ||
| } catch (final Exception e) { |
There was a problem hiding this comment.
I think the original problem could still occur for another unknown exception. Instead of specializing other exception types, let’s specialize the only one we throw for jar hell, IllegalStateException. Then the general exception can catch all the others with a different message like “failed to load plugin foo while checking for jar hell”.
There was a problem hiding this comment.
Yes, it makes sense. I have made the changes to check only for IllegalStateException that gets specifically thrown, the general exception catches all the others
|
@elasticmachine ok to test |
rjernst
added
v7.15.0
auto-backport
rjernst
approved these changes
Aug 10, 2021
elasticsearchmachine
pushed a commit
to elasticsearchmachine/elasticsearch
that referenced
this pull request
Aug 10, 2021
When checking for jarhell while loading plugins, exceptions other than jarhell can occur. The wrapped exception message in these cases refers to jarhell failing, but in actuality it was eg a filesystem error. This commit separates the two cases, jarhell failure and other failure.
💚 Backport successful
|
rjernst
pushed a commit
that referenced
this pull request
Aug 10, 2021
When checking for jarhell while loading plugins, exceptions other than jarhell can occur. The wrapped exception message in these cases refers to jarhell failing, but in actuality it was eg a filesystem error. This commit separates the two cases, jarhell failure and other failure. Co-authored-by: E Bala <balasubramanyam.evani@gmail.com>
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Possible fix for Issue #75701 | JarHell Bootstrap Check can yield false positives |
Changes made:
Have you signed the contributor license agreement?
Yes
Have you followed the contributor guidelines? Yes
If submitting code, have you built your formula locally prior to submission with
gradle check?Yes
If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
Yes
If submitting code, have you checked that your submission is for an OS and architecture that we support?
I think so
If you are submitting this code for a class then read our [policy] (https://github.com/elastic/elasticsearch/blob/master/CONTRIBUTING.md#contributing-as-part-of-a-class) for that.
No