Skip to content

Conversation

@vinodkc
Copy link
Contributor

@vinodkc vinodkc commented Apr 22, 2015

No description provided.

@marmbrus
Copy link
Contributor

ok to test

@SparkQA
Copy link

SparkQA commented Apr 22, 2015

Test build #30791 has finished for PR 5633 at commit 73c5380.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@pwendell
Copy link
Contributor

This is how we usually do class loading, but IIRC, there is an issue with certain JDBC drivers where they need to be loaded from the primordial classloader or else there is some security issue. /cc @tmyklebu

@tmyklebu
Copy link
Contributor

Do the MySQL and Postgres integration tests both pass with this change? (You'll need Docker installed and you'll need to be on a Linux box to find out for yourself.)

It's not a security issue per se. The trouble is that JDBC's security stuff "fails closed" and it won't be able to find the driver if it wasn't loaded with the right class loader. I believe it does this so that untrusted code can't compromise other code by polluting the driver cache.

@vinodkc
Copy link
Contributor Author

vinodkc commented Apr 23, 2015

@marmbrus,
Have you tried MySQL and Postgres integration tests for this PR ?
#5543
I also wish to run MySQL and Postgres integration tests to verify my PR

@marmbrus
Copy link
Contributor

Unfortunately we can't run the docker tests on Jenkins and they cause issues with dependencies during the release so we temporarily removed them. I can try running them manually.

@marmbrus
Copy link
Contributor

Docker integration tests still pass. This is likely because classloaders (almost) always delegate to their parent to load, and thus even though we are allowing it to possibly find the class from the users jars it will still default to the system classloader when it can find the class.

Merging to master.

@marmbrus
Copy link
Contributor

Thanks for fixing this!

@asfgit asfgit closed this in c1213e6 Apr 23, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request May 14, 2015
…etConnector

Author: Vinod K C <vinod.kc@huawei.com>

Closes apache#5633 from vinodkc/use_correct_classloader_driverload and squashes the following commits:

73c5380 [Vinod K C] Use correct ClassLoader for JDBC Driver
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
…etConnector

Author: Vinod K C <vinod.kc@huawei.com>

Closes apache#5633 from vinodkc/use_correct_classloader_driverload and squashes the following commits:

73c5380 [Vinod K C] Use correct ClassLoader for JDBC Driver
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.

5 participants