TIKA-4834: allow comments in json configs throughout

Author: tballison

CHANGES.txt

@@ -1,5 +1,11 @@
 Release 4.1.0 - unreleased
 
+   * tika-server and tika-async-cli now start from a config that contains
+     // or /* */ comments, as the configuration docs have always said they
+     may. The main loader accepted them; the steps that re-read the user's
+     file to merge in server/CLI overrides (ConfigMerger, ensurePluginRoots)
+     used a strict parser and refused the whole file (TIKA-4834).
+
    * The Kafka pipes iterator no longer stops at the first empty poll. A newly
      subscribed consumer spends its first poll(s) joining the group and returns
      empty even when the topic has a backlog, so the iterator could enqueue zero

tika-pipes/tika-async-cli/src/main/java/org/apache/tika/async/cli/TikaAsyncCLI.java

@@ -25,6 +25,7 @@
 import java.util.Optional;
 import java.util.concurrent.TimeoutException;
 
+import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -445,6 +446,8 @@ static String resolveDefaultPluginsDir() {
      */
     static Path ensurePluginRoots(Path originalConfigPath, String pluginsDir) throws IOException {
         ObjectMapper mapper = new ObjectMapper();
+        // The user's file may carry the // and /* */ comments the config docs permit.
+        mapper.enable(JsonParser.Feature.ALLOW_COMMENTS);
         JsonNode rootNode = mapper.readTree(originalConfigPath.toFile());
 
         if (rootNode.has("plugin-roots")) {

tika-pipes/tika-async-cli/src/test/java/org/apache/tika/async/cli/AsyncCliParserTest.java

@@ -159,6 +159,23 @@ public void testFileListDefaultsOutputDir(@TempDir Path tmp) throws Exception {
     @TempDir
     Path tempDir;
 
+    /** TIKA-4834: the config docs permit comments. */
+    @Test
+    public void testEnsurePluginRootsAcceptsComments() throws Exception {
+        Path configPath = tempDir.resolve("config-comments.json");
+        Files.writeString(configPath, """
+            // leading comment
+            {
+              /* block */
+              "pipes": { "numClients": 3 } // trailing
+            }
+            """);
+        Path result = TikaAsyncCLI.ensurePluginRoots(configPath, null);
+        JsonNode root = new ObjectMapper().readTree(result.toFile());
+        assertTrue(root.has("plugin-roots"));
+        assertEquals(3, root.get("pipes").get("numClients").asInt());
+    }
+
     @Test
     public void testEnsurePluginRootsAddsDefault() throws Exception {
         // Create a config without plugin-roots

tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/config/ConfigMerger.java

@@ -25,6 +25,7 @@
 import java.util.Map;
 import java.util.UUID;
 
+import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationFeature;
@@ -92,6 +93,8 @@ public static MergeResult mergeOrCreate(Path existingConfig, ConfigOverrides ove
             throws IOException {
         ObjectMapper mapper = new ObjectMapper();
         mapper.enable(SerializationFeature.INDENT_OUTPUT);
+        // The user's file may carry the // and /* */ comments the config docs permit.
+        mapper.enable(JsonParser.Feature.ALLOW_COMMENTS);
 
         ObjectNode root;
         if (existingConfig != null && Files.exists(existingConfig)) {

tika-pipes/tika-pipes-core/src/test/java/org/apache/tika/pipes/core/config/ConfigMergerTest.java

@@ -82,6 +82,40 @@ public void testCreateNewConfig() throws IOException {
         Files.deleteIfExists(result.configPath());
     }
 
+    /** TIKA-4834: the config docs permit comments; the merge step must not reject them. */
+    @Test
+    public void testMergeWithCommentedConfig() throws IOException {
+        String existingConfig = """
+                // leading line comment
+                {
+                    /* block comment */
+                    "fetchers": {
+                        "existing-fetcher": { // trailing comment
+                            "file-system-fetcher": {
+                                "basePath": "/existing/path"
+                            }
+                        }
+                    },
+                    "plugin-roots": "existing-plugins"
+                }
+                """;
+        Path existingPath = tempDir.resolve("commented-config.json");
+        Files.writeString(existingPath, existingConfig);
+
+        ConfigOverrides overrides = ConfigOverrides.builder()
+                .addFetcher("new-fetcher", "file-system-fetcher", Map.of("basePath", "/new/path"))
+                .build();
+        ConfigMerger.MergeResult result = ConfigMerger.mergeOrCreate(existingPath, overrides);
+
+        // The merged file is plain JSON: readable by a strict mapper, content intact.
+        JsonNode root = new ObjectMapper().readTree(result.configPath().toFile());
+        assertEquals("/existing/path", root.get("fetchers").get("existing-fetcher")
+                .get("file-system-fetcher").get("basePath").asText());
+        assertTrue(root.get("fetchers").has("new-fetcher"));
+        assertEquals("existing-plugins", root.get("plugin-roots").asText());
+        Files.deleteIfExists(result.configPath());
+    }
+
     @Test
     public void testMergeWithExistingConfig() throws IOException {
         // Create existing config

tika-server/tika-server-core/src/test/java/org/apache/tika/server/core/TikaServerIntegrationTest.java

@@ -100,6 +100,18 @@ public void testBasic() throws Exception {
         testBaseline();
     }
 
+    /** TIKA-4834: the docs permit comments in the config; the server must start from one. */
+    @Test
+    public void testCommentedConfig() throws Exception {
+        startProcess(new String[]{"-config", getConfig("tika-config-server-comments.json")});
+        awaitServerStartup();
+        Response response = WebClient
+                .create(endPoint + RMETA_PATH)
+                .accept("application/json")
+                .put(ClassLoader.getSystemResourceAsStream(TEST_HELLO_WORLD));
+        assertEquals(200, response.getStatus());
+    }
+
     @Test
     public void testBasicWithPipes() throws Exception {
         // Test that pipes-based parsing works for normal documents

tika-server/tika-server-core/src/test/resources/configs/tika-config-server-comments.json

@@ -0,0 +1,20 @@
+// TIKA-4834: comments everywhere the docs say they are allowed
+{
+  /* block comment before a key */
+  "server": {
+    "port": 9999, // trailing comment
+    "allowPipes": true,
+    "endpoints": [
+      "rmeta",
+      // comment inside an array
+      "tika"
+    ]
+  },
+  "pipes": {
+    "numClients": 1,
+    "forkedJvmArgs": [
+      "-Xmx256m"
+    ]
+  },
+  "plugin-roots": "target/plugins"
+}