diff --git a/src/main/java/org/codelibs/fess/helper/PathMappingHelper.java b/src/main/java/org/codelibs/fess/helper/PathMappingHelper.java
index 9269a8d42..62230dd31 100644
--- a/src/main/java/org/codelibs/fess/helper/PathMappingHelper.java
+++ b/src/main/java/org/codelibs/fess/helper/PathMappingHelper.java
@@ -264,9 +264,16 @@ public BiFunction createPathMatcher(final Matcher match
*
* A replacement containing a colon is far more often an ordinary path mapping such as
* file: 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.
+ * the plain replacement, with $1 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.
+ *
+ * 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 ${...} 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.
*
* @param engineName the prefix before the first colon
* @param template the script text after the first colon
@@ -288,8 +295,9 @@ protected BiFunction 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);
}
diff --git a/src/test/java/org/codelibs/fess/helper/PathMappingHelperTest.java b/src/test/java/org/codelibs/fess/helper/PathMappingHelperTest.java
index 4198d4798..105b902df 100644
--- a/src/test/java/org/codelibs/fess/helper/PathMappingHelperTest.java
+++ b/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 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 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 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 list = new ArrayList<>();
+ list.add(pathMapping);
+
+ assertEquals("https://files.example.com/a/b.txt", helper.replaceUrl(list, "file:/share/a/b.txt"));
}
@Test