Skip to content

Use class' class loader when looking up CDP implementations. #10509

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

Conversation

dennisoelkers
Copy link
Contributor

@dennisoelkers dennisoelkers commented Apr 5, 2022

Description

Motivation and Context

Prior to this change, CdpVersionFinder was using ServiceLoader.load without passing in a class loader explicitly to find CDP implementations. In this case, ServiceLoader is using the current thread's default class loader to resolve the specified class name's service.

In projects with a plugin system, this can lead to problems, when selenium dependencies are part of a jar file that is not on the class path the application was started with. For these projects, using selenium results in a ClassNotFoundException when connecting to a remote webdriver.

To fix this, using the class loader of the class itself would be beneficial. In projects without a plugin architecture, it will mostly be
identical with the class loader of the current thread, in other cases there is a very high chance that the service which is to be loaded can be resolved through the class loader of CdpVersionFinder.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
Prior to this change, `CdpVersionFinder` was using `ServiceLoader.load` without
passing in a class loader explicitly to find CDP implementations. In
this case, `ServiceLoader` is using the current thread's default class
loader to resolve the specified class name's service.

In projects with a plugin system, this can lead to problems, when
selenium dependencies are part of a jar file that is not on the class
path the application was started with. For these projects, using
selenium results in a `ClassNotFoundException` when connecting to a
remote webdriver.

To fix this, using the class loader of the class itself would be
beneficial. In projects without a plugin architecture, it will mostly be
identical with the class loader of the current thread, in other cases
there is a very high chance that the service which is to be loaded can be
resolved through the class loader of `CdpVersionFinder`.
@dennisoelkers dennisoelkers changed the title Use class' class loader when looking up HttpClient factory. Apr 5, 2022
@pujagani
Copy link
Contributor

pujagani commented Apr 6, 2022

Thank you @dennisoelkers!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 6, 2022

@diemol diemol merged commit 10c712d into SeleniumHQ:trunk Apr 6, 2022
@dennisoelkers dennisoelkers deleted the fix/use-class-classloader-for-cdp-lookup branch April 7, 2022 06:47
elgatov pushed a commit to elgatov/selenium that referenced this pull request Jun 27, 2022
…umHQ#10509)

Prior to this change, `CdpVersionFinder` was using `ServiceLoader.load` without
passing in a class loader explicitly to find CDP implementations. In
this case, `ServiceLoader` is using the current thread's default class
loader to resolve the specified class name's service.

In projects with a plugin system, this can lead to problems, when
selenium dependencies are part of a jar file that is not on the class
path the application was started with. For these projects, using
selenium results in a `ClassNotFoundException` when connecting to a
remote webdriver.

To fix this, using the class loader of the class itself would be
beneficial. In projects without a plugin architecture, it will mostly be
identical with the class loader of the current thread, in other cases
there is a very high chance that the service which is to be loaded can be
resolved through the class loader of `CdpVersionFinder`.

Co-authored-by: Puja Jagani <puja.jagani93@gmail.com>
Co-authored-by: Diego Molina <diemol@users.noreply.github.com>

[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants