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

Issue #1797 - JEP 238 - Multi-Release JAR files break bytecode scanning. #1951

Merged
merged 1 commit into from
Nov 8, 2017

Conversation

sbordet
Copy link
Contributor

@sbordet sbordet commented Nov 8, 2017

Made MultiReleaseJarFile closeable and using try-with-resources in
AnnotationParser to avoid leaking file descriptors.

Made few code simplifications.

Signed-off-by: Simone Bordet simone.bordet@gmail.com

Made MultiReleaseJarFile closeable and using try-with-resources in
AnnotationParser to avoid leaking file descriptors.

Made few code simplifications.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet requested a review from joakime November 8, 2017 18:22
{
parseJarEntry(handlers, jarResource, e, resolver);
}
catch (Exception ex)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to catch any Exception?
What if the jar file itself has entries that can be iterated, but not read?
Wouldn't that result in a MultiException containing an entry for each and every entry?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets deal with this if it becomes a problem. Too complicated to work out now.

me.add(new RuntimeException("Error scanning entry " + e.getName() + " from jar " + jarResource, ex));
}
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An exception opening the JarFile isn't caught / added to the MultiException. (eg: a bad jar file)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, the method itself will pass a JarFile error out properly, no need for the MultiException route.

@sbordet sbordet merged commit b1d5fea into jetty-9.3.x Nov 8, 2017
@sbordet sbordet deleted the jetty-9.3.x-1797-multi_release_jar_npe branch November 8, 2017 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants