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