[
https://issues.apache.org/jira/browse/TIKA-4931?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18119236#comment-18119236
]
ASF GitHub Bot commented on TIKA-4931:
--------------------------------------
Copilot commented on code in PR #3257:
URL: https://github.com/apache/tika/pull/3257#discussion_r4105831360
##########
tika-pipes/tika-pipes-fork-parser/src/main/java/org/apache/tika/pipes/fork/PipesForkParser.java:
##########
@@ -409,11 +411,15 @@ private ConfigMerger.MergeResult createTikaConfigFile()
throws IOException {
// Use null ID to trigger UUID generation
.addFetcher(null, "file-system-fetcher",
Map.of("allowAbsolutePaths", true))
- // Set pipes configuration
+ // Set pipes configuration. socketTimeoutMillis/javaPath only
when set in
+ // code (TIKA-4931): writing the default would clobber a user
config's value.
.setPipesConfig(
pc.getNumClients(),
pc.getMaxFilesProcessedPerProcess(),
- pc.getForkedJvmArgs())
+ pc.getForkedJvmArgs(),
+ pc.getSocketTimeoutMillis() ==
PipesConfig.DEFAULT_SOCKET_TIMEOUT_MILLIS
+ ? -1 : pc.getSocketTimeoutMillis(),
+ DEFAULT_JAVA_PATH.equals(pc.getJavaPath()) ? null :
pc.getJavaPath())
Review Comment:
Comparing against the default value cannot tell whether the caller
explicitly set these values. With a user config containing `javaPath:
"/file/java"` (or a non-default socket timeout), calling `setJavaPath("java")`
(or explicitly setting the default 60000 timeout through `getPipesConfig()`) is
silently discarded, so the fork still uses the user-configured value. Track
whether each setter was invoked and pass that override state to `ConfigMerger`,
rather than using the value as the signal.
> PipesForkParser drops socketTimeoutMillis and javaPath set on
> PipesForkParserConfig
> -----------------------------------------------------------------------------------
>
> Key: TIKA-4931
> URL: https://issues.apache.org/jira/browse/TIKA-4931
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Trivial
>
> From a :robot: :
> PipesForkParser writes its config to a JSON file with
> ConfigMerger.mergeOrCreate(), then loads it with PipesParser.load(path).
> ConfigOverrides.PipesConfigOverride only carries numClients,
> maxFilesProcessedPerProcess and forkedJvmArgs. Anything else set on
> PipesForkParserConfig.getPipesConfig() never reaches the file, including
> socketTimeoutMillis and javaPath (the latter set through
> setJavaPath).
> As a result, the fork runs java from the PATH instead of the configured
> path, and the socket timeout stays at the 60s default. There's no error or
> warning.
> Fix: carry socketTimeoutMillis and javaPath through ConfigOverrides and
> ConfigMerger, and review the other PipesConfig setters for the same gap. Add
> a round-trip test: set values on the config,
> build the parser, reload the merged file, and check they match.
> Found by [~dpol1] on apache/stormcrawler#2183.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
