diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 0142f465af4..8365017c240 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -151,6 +151,11 @@ Other current classloader that defined the interface class (lucene-core.jar). See MIGRATE.txt for more information! (Uwe Schindler, Dawid Weiss) +* LUCENE-7883: Lucene no longer uses the context class loader when resolving + resources in CustomAnalyzer or ClassPathResourceLoader. Resources are only + resolved against Lucene's class loader by default. Please use another builder + method to change to a custom classloader. (Uwe Schindler) + ======================= Lucene 6.7.0 ======================= New Features diff --git a/lucene/MIGRATE.txt b/lucene/MIGRATE.txt index 089d196ba9a..e7edcc53a1e 100644 --- a/lucene/MIGRATE.txt +++ b/lucene/MIGRATE.txt @@ -35,12 +35,19 @@ If you are migrating your project to Java 9 Jigsaw module system, keep in mind that Lucene currently does not yet support `module-info.java` declarations of service provider impls (`provides` statement). It is therefore recommended to keep all of Lucene in one Uber-Module and not try to split Lucene into -several modules. As soon as Lucene will migrate to Java 9 as minimum requirement, -we will work on improving that. +several modules. As soon as Lucene will migrate to Java 9 as minimum +requirement, we will work on improving that. For OSGI, the same applies. You have to create a bundle with all of Lucene for SPI to work correctly. +## CustomAnalyzer resources (LUCENE-7883)## + +Lucene no longer uses the context class loader when resolving resources in +CustomAnalyzer or ClassPathResourceLoader. Resources are only resolved +against Lucene's class loader by default. Please use another builder method +to change to a custom classloader. + ## Query.hashCode and Query.equals are now abstract methods (LUCENE-7277) Any custom query subclasses should redeclare equivalence relationship according diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/custom/CustomAnalyzer.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/custom/CustomAnalyzer.java index 1cfdfe37979..a697cced51a 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/custom/CustomAnalyzer.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/custom/CustomAnalyzer.java @@ -76,17 +76,21 @@ import org.apache.lucene.util.Version; */ public final class CustomAnalyzer extends Analyzer { - /** Returns a builder for custom analyzers that loads all resources from classpath. - * All path names given must be absolute with package prefixes. */ + /** + * Returns a builder for custom analyzers that loads all resources from + * Lucene's classloader. All path names given must be absolute with package prefixes. + */ public static Builder builder() { - return builder(new ClasspathResourceLoader()); + return builder(new ClasspathResourceLoader(CustomAnalyzer.class.getClassLoader())); } - /** Returns a builder for custom analyzers that loads all resources from the given + /** + * Returns a builder for custom analyzers that loads all resources from the given * file system base directory. Place, e.g., stop word files there. - * Files that are not in the given directory are loaded from classpath. */ + * Files that are not in the given directory are loaded from Lucene's classloader. + */ public static Builder builder(Path configDir) { - return builder(new FilesystemResourceLoader(configDir)); + return builder(new FilesystemResourceLoader(configDir, CustomAnalyzer.class.getClassLoader())); } /** Returns a builder for custom analyzers that loads all resources using the given {@link ResourceLoader}. */ diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/ClasspathResourceLoader.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/ClasspathResourceLoader.java index 3653e7f5ffe..4ee9212949a 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/ClasspathResourceLoader.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/ClasspathResourceLoader.java @@ -20,6 +20,8 @@ package org.apache.lucene.analysis.util; import java.io.IOException; import java.io.InputStream; +import org.apache.lucene.util.SuppressForbidden; + /** * Simple {@link ResourceLoader} that uses {@link ClassLoader#getResourceAsStream(String)} * and {@link Class#forName(String,boolean,ClassLoader)} to open resources and @@ -30,9 +32,17 @@ public final class ClasspathResourceLoader implements ResourceLoader { private final ClassLoader loader; /** - * Creates an instance using the context classloader to load Resources and classes. + * Creates an instance using the context classloader to load resources and classes. * Resource paths must be absolute. + * + * @deprecated You should not use this ctor, because it uses the thread's context + * class loader, which is bad programming style. Please specify a reference class or + * a {@link ClassLoader} instead. + * @see #ClasspathResourceLoader(ClassLoader) + * @see #ClasspathResourceLoader(Class) */ + @Deprecated + @SuppressForbidden(reason = "Deprecated method uses thread's context classloader, but there for backwards compatibility") public ClasspathResourceLoader() { this(Thread.currentThread().getContextClassLoader()); } diff --git a/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/FilesystemResourceLoader.java b/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/FilesystemResourceLoader.java index 39703865029..4fbaa6d94b5 100644 --- a/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/FilesystemResourceLoader.java +++ b/lucene/analysis/common/src/java/org/apache/lucene/analysis/util/FilesystemResourceLoader.java @@ -46,11 +46,27 @@ public final class FilesystemResourceLoader implements ResourceLoader { * base directory (may be {@code null} to refer to CWD). * Files not found in file system and class lookups are delegated to context * classloader. + * + * @deprecated You should not use this ctor, because it uses the thread's context + * class loader as fallback for resource lookups, which is bad programming style. + * Please specify a {@link ClassLoader} instead. + * @see #FilesystemResourceLoader(Path, ClassLoader) */ + @Deprecated public FilesystemResourceLoader(Path baseDirectory) { this(baseDirectory, new ClasspathResourceLoader()); } + /** + * Creates a resource loader that resolves resources against the given + * base directory (may be {@code null} to refer to CWD). + * Files not found in file system and class lookups are delegated to context + * classloader. + */ + public FilesystemResourceLoader(Path baseDirectory, ClassLoader delegate) { + this(baseDirectory, new ClasspathResourceLoader(delegate)); + } + /** * Creates a resource loader that resolves resources against the given * base directory (may be {@code null} to refer to CWD). diff --git a/lucene/tools/forbiddenApis/base.txt b/lucene/tools/forbiddenApis/base.txt index 1bb8118de15..5eb3d3868b5 100644 --- a/lucene/tools/forbiddenApis/base.txt +++ b/lucene/tools/forbiddenApis/base.txt @@ -27,6 +27,10 @@ java.util.Properties#load(java.io.InputStream) java.util.Properties#save(java.io.OutputStream,java.lang.String) java.util.Properties#store(java.io.OutputStream,java.lang.String) +@defaultMessage The context classloader should never be used for resource lookups, unless there is a 3rd party library that needs it. Always pass a classloader down as method parameters. +java.lang.Thread#getContextClassLoader() +java.lang.Thread#setContextClassLoader(java.lang.ClassLoader) + java.lang.Character#codePointBefore(char[],int) @ Implicit start offset is error-prone when the char[] is a buffer and the first chars are random chars java.lang.Character#codePointAt(char[],int) @ Implicit end offset is error-prone when the char[] is a buffer and the last chars are random chars diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8494aeec80d..71dfa6a46f9 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -309,6 +309,10 @@ Other Changes option to PointsField to ignore 'precisionStep' attribute. This change also begins to attempt to randomize 'docValues' on numeric field types unless tests explicity enable/disable them. (hossman, Steve Rowe) +* LUCENE-7883: Solr no longer uses the context class loader when resolving + resources, they are only resolved against Solr's own or "core" class loader + by default. (Uwe Schindler) + ================== 6.7.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/contrib/clustering/src/java/org/apache/solr/handler/clustering/carrot2/CarrotClusteringEngine.java b/solr/contrib/clustering/src/java/org/apache/solr/handler/clustering/carrot2/CarrotClusteringEngine.java index 951cce5c4b0..33cbb64dd0f 100644 --- a/solr/contrib/clustering/src/java/org/apache/solr/handler/clustering/carrot2/CarrotClusteringEngine.java +++ b/solr/contrib/clustering/src/java/org/apache/solr/handler/clustering/carrot2/CarrotClusteringEngine.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.function.Supplier; import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang.StringUtils; @@ -40,6 +41,7 @@ import org.apache.solr.common.params.HighlightParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; +import org.apache.solr.common.util.SuppressForbidden; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.clustering.ClusteringEngine; import org.apache.solr.handler.clustering.SearchClusteringEngine; @@ -162,21 +164,17 @@ public class CarrotClusteringEngine extends SearchClusteringEngine { + Arrays.toString(attributeXmls)); } - Thread ct = Thread.currentThread(); - ClassLoader prev = ct.getContextClassLoader(); - try { - ct.setContextClassLoader(core.getResourceLoader().getClassLoader()); - - AttributeValueSets avs = AttributeValueSets.deserialize(attributeXmls[0].open()); - AttributeValueSet defaultSet = avs.getDefaultAttributeValueSet(); - initAttributes.putAll(defaultSet.getAttributeValues()); - } catch (Exception e) { - throw new SolrException(ErrorCode.SERVER_ERROR, - "Could not read attributes XML for clustering component: " - + componentName, e); - } finally { - ct.setContextClassLoader(prev); - } + withContextClassLoader(core.getResourceLoader().getClassLoader(), () -> { + try { + AttributeValueSets avs = AttributeValueSets.deserialize(attributeXmls[0].open()); + AttributeValueSet defaultSet = avs.getDefaultAttributeValueSet(); + initAttributes.putAll(defaultSet.getAttributeValues()); + } catch (Exception e) { + throw new SolrException(ErrorCode.SERVER_ERROR, + "Could not read attributes XML for clustering component: " + componentName, e); + } + return null; + }); } } @@ -205,14 +203,7 @@ public class CarrotClusteringEngine extends SearchClusteringEngine { // certain classes (e.g. custom tokenizer/stemmer) at initialization time. // To make sure classes from contrib JARs are available, // we swap the context class loader for the time of clustering. - Thread ct = Thread.currentThread(); - ClassLoader prev = ct.getContextClassLoader(); - try { - ct.setContextClassLoader(core.getResourceLoader().getClassLoader()); - this.controller.init(initAttributes); - } finally { - ct.setContextClassLoader(prev); - } + withContextClassLoader(core.getResourceLoader().getClassLoader(), () -> this.controller.init(initAttributes)); SchemaField uniqueField = core.getLatestSchema().getUniqueKeyField(); if (uniqueField == null) { @@ -246,15 +237,9 @@ public class CarrotClusteringEngine extends SearchClusteringEngine { // certain classes (e.g. custom tokenizer/stemmer) at runtime. // To make sure classes from contrib JARs are available, // we swap the context class loader for the time of clustering. - Thread ct = Thread.currentThread(); - ClassLoader prev = ct.getContextClassLoader(); - try { - ct.setContextClassLoader(core.getResourceLoader().getClassLoader()); - return clustersToNamedList(controller.process(attributes, - clusteringAlgorithmClass).getClusters(), sreq.getParams()); - } finally { - ct.setContextClassLoader(prev); - } + return withContextClassLoader(core.getResourceLoader().getClassLoader(), + () -> clustersToNamedList(controller.process(attributes, + clusteringAlgorithmClass).getClusters(), sreq.getParams())); } catch (Exception e) { log.error("Carrot2 clustering failed", e); throw new SolrException(ErrorCode.SERVER_ERROR, "Carrot2 clustering failed", e); @@ -562,4 +547,17 @@ public class CarrotClusteringEngine extends SearchClusteringEngine { } } } + + @SuppressForbidden(reason = "Uses context class loader as a workaround to inject correct classloader to 3rd party libs") + private static T withContextClassLoader(ClassLoader loader, Supplier action) { + Thread ct = Thread.currentThread(); + ClassLoader prev = ct.getContextClassLoader(); + try { + ct.setContextClassLoader(loader); + return action.get(); + } finally { + ct.setContextClassLoader(prev); + } + } + } diff --git a/solr/contrib/dataimporthandler-extras/src/java/org/apache/solr/handler/dataimport/MailEntityProcessor.java b/solr/contrib/dataimporthandler-extras/src/java/org/apache/solr/handler/dataimport/MailEntityProcessor.java index d4418daec81..f7ad22f7e4b 100644 --- a/solr/contrib/dataimporthandler-extras/src/java/org/apache/solr/handler/dataimport/MailEntityProcessor.java +++ b/solr/contrib/dataimporthandler-extras/src/java/org/apache/solr/handler/dataimport/MailEntityProcessor.java @@ -18,6 +18,7 @@ package org.apache.solr.handler.dataimport; import com.sun.mail.imap.IMAPMessage; +import org.apache.solr.common.util.SuppressForbidden; import org.apache.solr.handler.dataimport.config.ConfigNameConstants; import org.apache.solr.util.RTimer; import org.apache.tika.Tika; @@ -37,6 +38,7 @@ import java.lang.invoke.MethodHandles; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.*; +import java.util.function.Supplier; import com.sun.mail.gimap.GmailFolder; import com.sun.mail.gimap.GmailRawSearchTerm; @@ -213,7 +215,14 @@ public class MailEntityProcessor extends EntityProcessorBase { private Message getNextMail() { if (!connected) { - if (!connectToMailBox()) return null; + // this is needed to load the activation mail stuff correctly + // otherwise, the JavaMail multipart support doesn't get configured + // correctly, which leads to a class cast exception when processing + // multipart messages: IMAPInputStream cannot be cast to + // javax.mail.Multipart + if (false == withContextClassLoader(getClass().getClassLoader(), this::connectToMailBox)) { + return null; + } connected = true; } if (folderIter == null) { @@ -358,13 +367,6 @@ public class MailEntityProcessor extends EntityProcessorBase { } private boolean connectToMailBox() { - // this is needed to load the activation mail stuff correctly - // otherwise, the JavaMail multipart support doesn't get configured - // correctly, which leads to a class cast exception when processing - // multipart messages: IMAPInputStream cannot be cast to - // javax.mail.Multipart - Thread.currentThread().setContextClassLoader(getClass().getClassLoader()); - try { Properties props = new Properties(); if (System.getProperty("mail.debug") != null) @@ -870,4 +872,17 @@ public class MailEntityProcessor extends EntityProcessorBase { } return v; } + + @SuppressForbidden(reason = "Uses context class loader as a workaround to inject correct classloader to 3rd party libs") + private static T withContextClassLoader(ClassLoader loader, Supplier action) { + Thread ct = Thread.currentThread(); + ClassLoader prev = ct.getContextClassLoader(); + try { + ct.setContextClassLoader(loader); + return action.get(); + } finally { + ct.setContextClassLoader(prev); + } + } + } diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java index 17cdbbc2919..ef79875071d 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java @@ -145,7 +145,7 @@ public class SolrResourceLoader implements ResourceLoader,Closeable /** *

- * This loader will delegate to the context classloader when possible, + * This loader will delegate to Solr's classloader when possible, * otherwise it will attempt to resolve resources using any jar files * found in the "lib/" directory in the specified instance directory. *

@@ -162,9 +162,10 @@ public class SolrResourceLoader implements ResourceLoader,Closeable log.debug("new SolrResourceLoader for directory: '{}'", this.instanceDir); } - if (parent == null) - parent = Thread.currentThread().getContextClassLoader(); - this.classLoader = new URLClassLoader(new URL[0], parent); + if (parent == null) { + parent = getClass().getClassLoader(); + } + this.classLoader = URLClassLoader.newInstance(new URL[0], parent); /* * Skip the lib subdirectory when we are loading from the solr home. diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index 9a3fe00fd2d..3f2eb5f048c 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -357,11 +357,11 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission try { String path = ZkStateReader.CONFIGS_ZKNODE + "/" + SYSTEM_COLL + "/schema.xml"; - byte[] data = IOUtils.toByteArray(Thread.currentThread().getContextClassLoader().getResourceAsStream("SystemCollectionSchema.xml")); + byte[] data = IOUtils.toByteArray(CollectionsHandler.class.getResourceAsStream("/SystemCollectionSchema.xml")); assert data != null && data.length > 0; cmdExecutor.ensureExists(path, data, CreateMode.PERSISTENT, zk); path = ZkStateReader.CONFIGS_ZKNODE + "/" + SYSTEM_COLL + "/solrconfig.xml"; - data = IOUtils.toByteArray(Thread.currentThread().getContextClassLoader().getResourceAsStream("SystemCollectionSolrConfig.xml")); + data = IOUtils.toByteArray(CollectionsHandler.class.getResourceAsStream("/SystemCollectionSolrConfig.xml")); assert data != null && data.length > 0; cmdExecutor.ensureExists(path, data, CreateMode.PERSISTENT, zk); } catch (IOException e) { 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 e58a3d86da8..cdf3ce855f2 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -265,7 +265,7 @@ public class SolrDispatchFilter extends BaseSolrFilter { */ public static NodeConfig loadNodeConfig(Path solrHome, Properties nodeProperties) { - SolrResourceLoader loader = new SolrResourceLoader(solrHome, null, nodeProperties); + SolrResourceLoader loader = new SolrResourceLoader(solrHome, SolrDispatchFilter.class.getClassLoader(), nodeProperties); if (!StringUtils.isEmpty(System.getProperty("solr.solrxml.location"))) { log.warn("Solr property solr.solrxml.location is no longer supported. " + "Will automatically load solr.xml from ZooKeeper if it exists"); 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 51ab5d7f5dd..c207dcf1c4c 100644 --- a/solr/core/src/java/org/apache/solr/util/SolrCLI.java +++ b/solr/core/src/java/org/apache/solr/util/SolrCLI.java @@ -486,7 +486,7 @@ public class SolrCLI { private static List> findToolClassesInPackage(String packageName) { List> toolClasses = new ArrayList>(); try { - ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader classLoader = SolrCLI.class.getClassLoader(); String path = packageName.replace('.', '/'); Enumeration resources = classLoader.getResources(path); Set classes = new TreeSet(); diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java index 2949e2e5bc4..cbef887f2a2 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java +++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java @@ -241,8 +241,8 @@ public class TestCoreContainer extends SolrTestCaseJ4 { final CoreContainer cc = init(CONFIGSETS_SOLR_XML); try { ClassLoader sharedLoader = cc.loader.getClassLoader(); - ClassLoader contextLoader = Thread.currentThread().getContextClassLoader(); - assertSame(contextLoader, sharedLoader.getParent()); + ClassLoader baseLoader = SolrResourceLoader.class.getClassLoader(); + assertSame(baseLoader, sharedLoader.getParent()); SolrCore core1 = cc.create("core1", ImmutableMap.of("configSet", "minimal")); ClassLoader coreLoader = core1.getResourceLoader().getClassLoader(); diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java index 1da5af60361..fbfc4a120e7 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java @@ -193,8 +193,7 @@ public class Utils { } public static Object fromJSONResource(String resourceName){ - return fromJSON(Thread.currentThread() - .getContextClassLoader().getResourceAsStream(resourceName)); + return fromJSON(Utils.class.getClassLoader().getResourceAsStream(resourceName)); } public static JSONParser getJSONParser(Reader reader){ diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java b/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java index eef3823f2f4..4edef90b43d 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/ValidatingJsonMap.java @@ -313,7 +313,7 @@ public class ValidatingJsonMap implements Map { } public static ValidatingJsonMap parse(String resourceName, String includeLocation) { - InputStream is = Thread.currentThread().getContextClassLoader().getResourceAsStream(resourceName); + InputStream is = ValidatingJsonMap.class.getClassLoader().getResourceAsStream(resourceName); if (is == null) throw new RuntimeException("invalid API spec: " + resourceName); ValidatingJsonMap map = null; diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 149745770f3..09a7ca19845 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -1891,7 +1891,7 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { * {@link Class#getResourceAsStream} using {@code this.getClass()}. */ public static File getFile(String name) { - final URL url = Thread.currentThread().getContextClassLoader().getResource(name.replace(File.separatorChar, '/')); + final URL url = SolrTestCaseJ4.class.getClassLoader().getResource(name.replace(File.separatorChar, '/')); if (url != null) { try { return new File(url.toURI()); diff --git a/solr/test-framework/src/java/org/apache/solr/util/ExternalPaths.java b/solr/test-framework/src/java/org/apache/solr/util/ExternalPaths.java index e5a83efc4aa..1ebb2e962cc 100644 --- a/solr/test-framework/src/java/org/apache/solr/util/ExternalPaths.java +++ b/solr/test-framework/src/java/org/apache/solr/util/ExternalPaths.java @@ -59,7 +59,7 @@ public class ExternalPaths { try { file = new File("solr/conf"); if (!file.exists()) { - file = new File(Thread.currentThread().getContextClassLoader().getResource("solr/conf").toURI()); + file = new File(ExternalPaths.class.getClassLoader().getResource("solr/conf").toURI()); } } catch (Exception e) { // If there is no "solr/conf" in the classpath, fall back to searching from the current directory.