fix(pathmap): leave the URL unchanged when a script mapping has no engine

src/main/java/org/codelibs/fess/helper/PathMappingHelper.java

@@ -264,9 +264,16 @@ public BiFunction<String, Matcher, String> createPathMatcher(final Matcher match
      *
      * <p>A replacement containing a colon is far more often an ordinary path mapping such as
      * <code>file:</code> than a script, so every way of failing to reach an engine has to end in
-     * the plain replacement. Throwing here would leave {@code PathMapping#process} logging a
-     * warning and returning the raw URL, which indexes the wrong URL for a mapping that never
-     * wanted an engine in the first place.</p>
+     * the plain replacement, with <code>$1</code> group references as before. Throwing here would
+     * leave {@code PathMapping#process} logging a warning and returning the raw URL, which indexes
+     * the wrong URL for a mapping that never wanted an engine in the first place.</p>
+     *
+     * <p>A prefix in {@link #SCRIPT_ENGINE_NAMES} is the exception: that replacement is a script,
+     * typically a Groovy one saved before its engine became a plugin. Read as a regular-expression
+     * replacement, its <code>${...}</code> is a group reference that does not exist and every URL
+     * throws; read literally, every matching URL becomes the script text, which a crawl-time
+     * mapping would index. Neither is a mapping anyone wrote, so the URL is left unchanged and the
+     * missing engine is reported once, when the mapping is first used.</p>
      *
      * @param engineName the prefix before the first colon
      * @param template the script text after the first colon
@@ -288,8 +295,9 @@ protected BiFunction<String, Matcher, String> resolveEngineMatcher(final String
         }
         if (scriptEngine == null) {
             if (SCRIPT_ENGINE_NAMES.contains(engineName.toLowerCase(Locale.ROOT))) {
-                logger.warn("No script engine is registered for {}, so \"{}\" is used as a plain replacement."
+                logger.warn("No script engine is registered for {}, so the path mapping \"{}\" is not applied and URLs are left unchanged."
                         + " Install the plugin providing it, such as fess-script-groovy for groovy.", engineName, replacement);
+                return (u, m) -> u;
             }
             return (u, m) -> m.replaceAll(replacement);
         }

src/test/java/org/codelibs/fess/helper/PathMappingHelperTest.java

@@ -22,10 +22,12 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.logging.log4j.Level;
 import org.codelibs.fess.Constants;
 import org.codelibs.fess.opensearch.config.exentity.PathMapping;
 import org.codelibs.fess.script.ScriptEngine;
 import org.codelibs.fess.script.ScriptEngineFactory;
+import org.codelibs.fess.unit.LogCapturingAppender;
 import org.codelibs.fess.unit.UnitFessTestCase;
 import org.codelibs.fess.util.ComponentUtil;
 import org.junit.jupiter.api.Test;
@@ -456,7 +458,7 @@ public void test_replaceUrl_groovyPrefix() {
     }
 
     @Test
-    public void test_replaceUrl_missingGroovyEngineIsPlainReplacement() {
+    public void test_replaceUrl_missingGroovyEngineLeavesUrlUnchanged() {
         ComponentUtil.register(new ScriptEngineFactory(), "scriptEngineFactory");
 
         final PathMappingHelper helper = new PathMappingHelper();
@@ -466,7 +468,61 @@ public void test_replaceUrl_missingGroovyEngineIsPlainReplacement() {
         final List<PathMapping> list = new ArrayList<>();
         list.add(pathMapping);
 
-        assertEquals("groovy:urla.html", helper.replaceUrl(list, "http://example.com/a.html"));
+        assertEquals("http://example.com/a.html", helper.replaceUrl(list, "http://example.com/a.html"));
+    }
+
+    @Test
+    public void test_createPathMatcher_missingGroovyEngineDoesNotThrowOnScript() {
+        ComponentUtil.register(new ScriptEngineFactory(), "scriptEngineFactory");
+
+        final PathMappingHelper helper = new PathMappingHelper();
+        final String url = "http://localhost/docs/en/intro.html";
+        final Matcher matcher = Pattern.compile("http://localhost/docs/en/(.*)").matcher(url);
+        assertTrue(matcher.find());
+        final BiFunction<String, Matcher, String> pathMatcher =
+                helper.createPathMatcher(matcher, "groovy:\"http://mapped.invalid/en-${matcher.group(1)}\"");
+
+        assertEquals(url, pathMatcher.apply(url, matcher));
+    }
+
+    @Test
+    public void test_replaceUrl_missingGroovyEngineScriptLeavesUrlAndLogsNoFailure() {
+        ComponentUtil.register(new ScriptEngineFactory(), "scriptEngineFactory");
+
+        final PathMappingHelper helper = new PathMappingHelper();
+        final PathMapping pathMapping = new PathMapping();
+        pathMapping.setRegex("http://localhost/docs/en/(.*)");
+        pathMapping.setReplacement("groovy:\"http://mapped.invalid/en-${matcher.group(1)}\"");
+        final List<PathMapping> list = new ArrayList<>();
+        list.add(pathMapping);
+
+        final LogCapturingAppender pathMappingLog = LogCapturingAppender.attach(PathMapping.class);
+        final LogCapturingAppender helperLog = LogCapturingAppender.attach(PathMappingHelper.class);
+        try {
+            assertEquals("http://localhost/docs/en/intro.html", helper.replaceUrl(list, "http://localhost/docs/en/intro.html"));
+            assertEquals("http://localhost/docs/en/guide.html", helper.replaceUrl(list, "http://localhost/docs/en/guide.html"));
+
+            assertTrue(pathMappingLog.eventsAt(Level.WARN).isEmpty(), "unexpected warnings: " + pathMappingLog.renderedEvents());
+            assertEquals(1, helperLog.warnings().size());
+            assertTrue(helperLog.warnings().get(0).contains("fess-script-groovy"), helperLog.warnings().toString());
+        } finally {
+            pathMappingLog.detach();
+            helperLog.detach();
+        }
+    }
+
+    @Test
+    public void test_replaceUrl_unregisteredPrefixKeepsGroupReferences() {
+        ComponentUtil.register(new ScriptEngineFactory(), "scriptEngineFactory");
+
+        final PathMappingHelper helper = new PathMappingHelper();
+        final PathMapping pathMapping = new PathMapping();
+        pathMapping.setRegex("^file:/share/(.*)");
+        pathMapping.setReplacement("https://files.example.com/$1");
+        final List<PathMapping> list = new ArrayList<>();
+        list.add(pathMapping);
+
+        assertEquals("https://files.example.com/a/b.txt", helper.replaceUrl(list, "file:/share/a/b.txt"));
     }
 
     @Test