Copilot commented on code in PR #2880: URL: https://github.com/apache/tika/pull/2880#discussion_r3369636827
########## tika-core/src/main/java/org/apache/tika/config/TikaExtras.java: ########## @@ -0,0 +1,158 @@ +/* + * 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. + */ +package org.apache.tika.config; + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Opt-in mechanism for adding user-supplied "extras" jars (extra + * {@code EncodingDetector}s, {@code Parser}s, etc.) to Tika's SPI discovery + * without repackaging the application. + * + * <p><b>Off by default.</b> Nothing is loaded unless the + * {@value #EXTRAS_DIR_PROPERTY} system property points at a directory; then every + * {@code *.jar} in it is made visible to service-loading. There is no implicit + * directory, and the working directory is never used. Review Comment: The class-level javadoc says “the working directory is never used”, but `extrasDir()` accepts relative paths (and README examples use `-Dtika.extras.dir=my-jars`), which will be resolved against the process working directory. This is a bit misleading; please clarify that there is no implicit directory, but relative paths still use the working directory for resolution. ########## tika-server/tika-server-core/src/main/java/org/apache/tika/server/core/TikaServerProcess.java: ########## @@ -120,6 +121,10 @@ private static Options getOptions() { } public static void main(String[] args) throws Exception { + // Install any tika.extras.dir jars before loading components, so the + // server's in-process SPI discovery sees them too (forked PipesServer + // children get them via the classpath in *ServerManager.writeArgFile). Review Comment: This comment references “*ServerManager.writeArgFile” (with a leading `*`), which reads like a formatting artifact and makes it harder to grep/search for the referenced method. Consider removing the `*`. ########## tika-core/src/main/java/org/apache/tika/config/TikaExtras.java: ########## @@ -0,0 +1,158 @@ +/* + * 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. + */ +package org.apache.tika.config; + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Opt-in mechanism for adding user-supplied "extras" jars (extra + * {@code EncodingDetector}s, {@code Parser}s, etc.) to Tika's SPI discovery + * without repackaging the application. + * + * <p><b>Off by default.</b> Nothing is loaded unless the + * {@value #EXTRAS_DIR_PROPERTY} system property points at a directory; then every + * {@code *.jar} in it is made visible to service-loading. There is no implicit + * directory, and the working directory is never used. + * + * <p><b>Security:</b> this is a trusted code directory — anything in it runs with + * the full privileges of the Tika process. Treat write access to it exactly like + * write access to {@code lib/}; it must not be writable by less-trusted principals + * (for servers, not reachable by request handling). Being opt-in keeps "we are + * now loading extra code" an explicit, auditable choice. + */ +public final class TikaExtras { + + /** System property naming the extras directory. Unset = feature off. */ + public static final String EXTRAS_DIR_PROPERTY = "tika.extras.dir"; + + private static final Logger LOG = LoggerFactory.getLogger(TikaExtras.class); + + private TikaExtras() { + } + + /** + * If {@value #EXTRAS_DIR_PROPERTY} is set, installs a classloader over the + * {@code *.jar} files in that directory as the thread + Tika + * {@link ServiceLoader} context classloader, so they join SPI discovery. + * No-op (returns {@code null}) when the property is unset or the directory is + * missing/empty. Call once, before any Tika component is loaded. + * + * @return the installed classloader, or {@code null} if extras are off/empty + */ + public static ClassLoader install() { + List<Path> jars = extraJars(); + if (jars.isEmpty()) { + return null; + } Review Comment: `install()` can be called multiple times and will create a new `URLClassLoader` each time, overriding the global `ServiceLoader` context classloader and potentially leaking open handles to the original jars (notably on Windows). Since this is a public utility in `tika-core`, consider making it idempotent (e.g., cache and return the previously installed classloader) or otherwise guard against repeated installation. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
