You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2022/04/04 15:53:48 UTC
[solr] branch branch_9_0 updated: SOLR-16131: Fix class loader for jdbcStream (#783)
This is an automated email from the ASF dual-hosted git repository.
houston pushed a commit to branch branch_9_0
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9_0 by this push:
new 332485f92f3 SOLR-16131: Fix class loader for jdbcStream (#783)
332485f92f3 is described below
commit 332485f92f3c1110efe14f0144a1ffdfc68d65e3
Author: Houston Putman <ho...@apache.org>
AuthorDate: Mon Apr 4 11:46:13 2022 -0400
SOLR-16131: Fix class loader for jdbcStream (#783)
This sets the jetty thread's contextClassLoader to the Solr core's resourceLoader's classLoader for all Solr requests.
(cherry picked from commit b7bd00188e33330a693619a0ae9f7a54dc6d73ea)
---
solr/CHANGES.txt | 4 +++
.../src/java/org/apache/solr/api/V2HttpCall.java | 6 ++++
.../src/java/org/apache/solr/core/PluginBag.java | 2 +-
.../java/org/apache/solr/servlet/HttpSolrCall.java | 6 ++++
.../apache/solr/servlet/SolrDispatchFilter.java | 5 +++
.../src/java/org/apache/solr/util/SolrCLI.java | 4 +--
.../apache/solr/handler/sql/CalciteJDBCStream.java | 6 ++++
solr/packaging/test/test_modules.bats | 37 ++++++++++++++++++++++
.../solr/client/solrj/io/stream/JDBCStream.java | 22 ++++++++++---
9 files changed, 84 insertions(+), 8 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c9af159cc1c..78323221c73 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -244,6 +244,10 @@ Improvements
* SOLR-15590: Start CoreContainer via ServletContextListener (Gus Heck)
+* SOLR-16131: Jetty threads' contextClassLoaders are set to the Solr core's resourceLoader's classLoader.
+ This allows for custom plugins and Solr modules to rely on 3rd part dependencies that use the thread contextClassLoader.
+ (Houston Putman)
+
Build
---------------------
* LUCENE-9077 LUCENE-9433: Support Gradle build, remove Ant support from trunk (Dawid Weiss, Erick Erickson, Uwe Schindler et.al.)
diff --git a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
index a463ce30d06..b7e44e369ae 100644
--- a/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
+++ b/solr/core/src/java/org/apache/solr/api/V2HttpCall.java
@@ -37,6 +37,7 @@ import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.util.CommandOperation;
import org.apache.solr.common.util.JsonSchemaValidator;
import org.apache.solr.common.util.PathTrie;
+import org.apache.solr.common.util.SuppressForbidden;
import org.apache.solr.common.util.ValidatingJsonMap;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.PluginBag;
@@ -72,6 +73,9 @@ public class V2HttpCall extends HttpSolrCall {
super(solrDispatchFilter, cc, request, response, retry);
}
+ @SuppressForbidden(
+ reason =
+ "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control")
protected void init() throws Exception {
queryParams = SolrRequestParsers.parseQueryString(req.getQueryString());
String path = this.path;
@@ -152,6 +156,8 @@ public class V2HttpCall extends HttpSolrCall {
SolrException.ErrorCode.NOT_FOUND,
"no core retrieved for core name: " + origCorename + ". Path : " + path);
}
+ } else {
+ Thread.currentThread().setContextClassLoader(core.getResourceLoader().getClassLoader());
}
this.path = path = path.substring(prefix.length() + pathSegments.get(1).length() + 2);
diff --git a/solr/core/src/java/org/apache/solr/core/PluginBag.java b/solr/core/src/java/org/apache/solr/core/PluginBag.java
index 6787144e101..2494293efc6 100644
--- a/solr/core/src/java/org/apache/solr/core/PluginBag.java
+++ b/solr/core/src/java/org/apache/solr/core/PluginBag.java
@@ -128,7 +128,7 @@ public class PluginBag<T> implements AutoCloseable {
(Class<T>) meta.clazz,
meta.getCleanTag(),
null,
- core.getResourceLoader(info.pkgName));
+ core.getResourceLoader());
initInstance(inst, info);
return new PluginHolder<>(info, inst);
}
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index f4528c9a3f4..50b0527f1f4 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -99,6 +99,7 @@ import org.apache.solr.common.util.JsonSchemaValidator;
import org.apache.solr.common.util.NamedList;
import org.apache.solr.common.util.SimpleOrderedMap;
import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.common.util.SuppressForbidden;
import org.apache.solr.common.util.TimeSource;
import org.apache.solr.common.util.Utils;
import org.apache.solr.common.util.ValidatingJsonMap;
@@ -231,6 +232,9 @@ public class HttpSolrCall {
return collectionsList != null ? collectionsList : Collections.emptyList();
}
+ @SuppressForbidden(
+ reason =
+ "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control")
protected void init() throws Exception {
// check for management path
String alternate = cores.getManagementPath();
@@ -323,6 +327,8 @@ public class HttpSolrCall {
// With a valid core...
if (core != null) {
+ Thread.currentThread().setContextClassLoader(core.getResourceLoader().getClassLoader());
+
config = core.getSolrConfig();
// get or create/cache the parser for the core
SolrRequestParsers parser = config.getRequestParsers();
diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
index 853c628cbf3..ea53b5ac06c 100644
--- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
+++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java
@@ -46,6 +46,7 @@ import org.apache.solr.api.V2HttpCall;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.SuppressForbidden;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.core.SolrCore;
import org.apache.solr.logging.MDCLoggingContext;
@@ -169,12 +170,16 @@ public class SolrDispatchFilter extends BaseSolrFilter implements PathExcluder {
}
@Override
+ @SuppressForbidden(
+ reason =
+ "Set the thread contextClassLoader for all 3rd party dependencies that we cannot control")
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
try (var mdcSnapshot = MDCSnapshot.create()) {
assert null != mdcSnapshot; // prevent compiler warning
MDCLoggingContext.reset();
MDCLoggingContext.setNode(getCores());
+ Thread.currentThread().setContextClassLoader(getCores().getResourceLoader().getClassLoader());
doFilter(request, response, chain, false);
}
diff --git a/solr/core/src/java/org/apache/solr/util/SolrCLI.java b/solr/core/src/java/org/apache/solr/util/SolrCLI.java
index b9cbbc19b2d..e4f54cf528e 100755
--- a/solr/core/src/java/org/apache/solr/util/SolrCLI.java
+++ b/solr/core/src/java/org/apache/solr/util/SolrCLI.java
@@ -713,10 +713,10 @@ public class SolrCLI implements CLIO {
HttpClientUtil.createNewHttpClientRequestContext());
// check the response JSON from Solr to see if it is an error
Long statusCode = asLong("/responseHeader/status", json);
- if (statusCode == -1) {
+ if (statusCode != null && statusCode == -1) {
throw new SolrServerException(
"Unable to determine outcome of GET request to: " + getUrl + "! Response: " + json);
- } else if (statusCode != 0) {
+ } else if (statusCode != null && statusCode != 0) {
String errMsg = asString("/error/msg", json);
if (errMsg == null) errMsg = String.valueOf(json);
throw new SolrServerException(errMsg);
diff --git a/solr/modules/sql/src/java/org/apache/solr/handler/sql/CalciteJDBCStream.java b/solr/modules/sql/src/java/org/apache/solr/handler/sql/CalciteJDBCStream.java
index b88ce586292..c53e4a25c7b 100644
--- a/solr/modules/sql/src/java/org/apache/solr/handler/sql/CalciteJDBCStream.java
+++ b/solr/modules/sql/src/java/org/apache/solr/handler/sql/CalciteJDBCStream.java
@@ -18,6 +18,7 @@ package org.apache.solr.handler.sql;
import java.io.IOException;
import java.sql.Array;
+import java.sql.Driver;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
@@ -78,4 +79,9 @@ public class CalciteJDBCStream extends JDBCStream {
}
return valueSelector;
}
+
+ @Override
+ protected Driver getDriver() {
+ return CalciteSolrDriver.INSTANCE;
+ }
}
diff --git a/solr/packaging/test/test_modules.bats b/solr/packaging/test/test_modules.bats
new file mode 100644
index 00000000000..9f8d08ad59b
--- /dev/null
+++ b/solr/packaging/test/test_modules.bats
@@ -0,0 +1,37 @@
+#!/usr/bin/env bats
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+load bats_helper
+
+setup() {
+ common_setup
+}
+
+teardown() {
+ delete_all_collections
+ solr stop -all >/dev/null 2>&1
+}
+
+@test "SQL Module" {
+ run solr start -c -Dsolr.modules=sql
+ run solr create_collection -c COLL_NAME
+ run solr api -get http://localhost:8983/solr/COLL_NAME/sql?stmt=select+id+from+COLL_NAME+limit+10
+ assert_output --partial '"docs":'
+ assert_output --partial '"EOF":true'
+ assert_output --partial '"RESPONSE_TIME":'
+ refute_output --partial '"EXCEPTION"'
+}
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/JDBCStream.java b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/JDBCStream.java
index 368e756de94..9f78e945f43 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/JDBCStream.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/JDBCStream.java
@@ -23,6 +23,7 @@ import java.math.BigDecimal;
import java.sql.Clob;
import java.sql.Connection;
import java.sql.Date;
+import java.sql.Driver;
import java.sql.DriverManager;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
@@ -275,9 +276,15 @@ public class JDBCStream extends TupleStream implements Expressible {
this.streamContext = context;
}
- /** Opens the JDBCStream */
- public void open() throws IOException {
-
+ protected Driver getDriver() throws IOException {
+ // The DriverManager uses this class's ClassLoader to determine if it can load the driver given
+ // by driverClassName. Therefore, if you are loading in a driver from a separate ClassLoader
+ // than java was started with, it will likely not work. Instead, create a class that inherits
+ // this class and override this getDriver() method.
+ // Unfortunately it is impossible to use a custom ClassLoader with DriverManager, so we would
+ // need to remove our use of this class in order to support JDBC drivers loaded in via solr's
+ // additional library methods. This comment is relevant for JDBC drivers loaded in via custom
+ // plugins and even Solr modules.
try {
if (null != driverClassName) {
Class.forName(driverClassName);
@@ -291,9 +298,11 @@ public class JDBCStream extends TupleStream implements Expressible {
// likely want to provide the driverClassName. Not being able to find a driver generally means
// the driver has not been loaded.
try {
- if (null == DriverManager.getDriver(connectionUrl)) {
+ Driver driver = DriverManager.getDriver(connectionUrl);
+ if (null == driver) {
throw new SQLException("DriverManager.getDriver(url) returned null");
}
+ return driver;
} catch (SQLException e) {
throw new IOException(
String.format(
@@ -303,9 +312,12 @@ public class JDBCStream extends TupleStream implements Expressible {
connectionUrl),
e);
}
+ }
+ /** Opens the JDBCStream */
+ public void open() throws IOException {
try {
- connection = DriverManager.getConnection(connectionUrl, connectionProperties);
+ connection = getDriver().connect(connectionUrl, connectionProperties);
} catch (SQLException e) {
throw new IOException(
String.format(Locale.ROOT, "Failed to open JDBC connection to '%s'", connectionUrl), e);