[
https://issues.apache.org/jira/browse/TIKA-4755?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18086741#comment-18086741
]
ASF GitHub Bot commented on TIKA-4755:
--------------------------------------
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.
> Allow extras directory for users to add jars for tika-app and tika-server
> -------------------------------------------------------------------------
>
> Key: TIKA-4755
> URL: https://issues.apache.org/jira/browse/TIKA-4755
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Minor
>
> We do this for tika-server in docker. We should do the same for tika-app and
> regular tika-server.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
