Skip to content

remove anti-pattern with security issues #1188

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

Closed
wants to merge 2 commits into from

Conversation

seuros
Copy link
Contributor

@seuros seuros commented Jun 11, 2025

This commit needs to be back-ported in all active versions.

7-2 7-1

@@ -1818,10 +1818,6 @@ protected DriverWrapper newDriverWrapper(final ThreadContext context, final Stri
catch (ClassCastException e) {
throw wrapException(context, context.runtime.getNameError(), e);
}
catch (IllegalAccessException e) { throw wrapException(context, e); }
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure Java exceptions will get caught higher up in Ruby here. If so then you just crashed the Ruby process.

@kares can you review this?

Copy link
Member

Choose a reason for hiding this comment

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

Mentioned this on matrix, just adding noise to the user due nesting real cause 2 levels deeper.

Also there aren't any security issues, it's a deprecation.

@seuros seuros requested a review from kares June 11, 2025 18:43
private Driver allocateDriver(final Class<? extends Driver> driverClass) {
try {
return driverClass.getDeclaredConstructor().newInstance();
} catch (InvocationTargetException e) {
Copy link
Member

Choose a reason for hiding this comment

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

could you please restore this do not do any exception handling an do it where it was before instead.

as explained already we do not want any extra RuntimeException or IllegalStateException wrapping...

@seuros seuros closed this Jun 12, 2025
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.

3 participants