diff --git a/src/main/java/org/apache/commons/configuration2/ex/ConfigurationDeniedException.java b/src/main/java/org/apache/commons/configuration2/ex/ConfigurationDeniedException.java index 69ef3ca25d..c31c27007f 100644 --- a/src/main/java/org/apache/commons/configuration2/ex/ConfigurationDeniedException.java +++ b/src/main/java/org/apache/commons/configuration2/ex/ConfigurationDeniedException.java @@ -36,4 +36,16 @@ public class ConfigurationDeniedException extends ConfigurationRuntimeException public ConfigurationDeniedException(final String message, final Object... args) { super(message, args); } + + /** + * Constructs a new {@code ConfigurationDeniedException} with specified detail message using {@link String#format(String,Object...)} and cause. + * + * @param cause the cause. + * @param message the error message. + * @param args arguments to the error message. + * @see String#format(String,Object...) + */ + public ConfigurationDeniedException(final Throwable cause, final String message, final Object... args) { + super(cause, message, args); + } } diff --git a/src/main/java/org/apache/commons/configuration2/io/AbstractFileLocationStrategy.java b/src/main/java/org/apache/commons/configuration2/io/AbstractFileLocationStrategy.java index 1eb5716a56..5173e424ff 100644 --- a/src/main/java/org/apache/commons/configuration2/io/AbstractFileLocationStrategy.java +++ b/src/main/java/org/apache/commons/configuration2/io/AbstractFileLocationStrategy.java @@ -17,6 +17,7 @@ package org.apache.commons.configuration2.io; +import java.net.MalformedURLException; import java.net.URL; import java.util.Collections; import java.util.LinkedHashSet; @@ -204,6 +205,55 @@ public T get() { */ private static final String KEY_SCHEMES = "org.apache.commons.configuration2.io.FileLocationStrategy.schemes"; + private static void checkHost(final String value, final Set validSet) { + final String lowerCase = StringUtils.toRootLowerCase(value); + if (!validSet.isEmpty() && StringUtils.isNotEmpty(lowerCase) && validSet.stream().noneMatch(p -> p.matcher(lowerCase).matches())) { + throw new ConfigurationDeniedException("URL host is not enabled: %s; must be one of %s", value, validSet); + } + } + + /** + * Checks if the scheme is allowed. + * + * @param value A URL scheme, never empty or {@code null}. + * @param validSet the scheme valid-set. + */ + private static void checkScheme(final String value, final Set validSet) { + if (!validSet.isEmpty() && !validSet.contains(StringUtils.toRootLowerCase(value))) { + throw new ConfigurationDeniedException("URL scheme \"%s\" is not enabled, must be one of %s, override defaults with the system property \"%s\", " + + "complete set: \"file,http,https,jar\"", value, validSet, KEY_SCHEMES); + } + } + + /** + * Validates {@code url} against the scheme and host allow-lists. + * + * @param url the URL to check. + * @param validSchemes the scheme valid-set. + * @param validHosts the host valid-set. + * @throws ConfigurationDeniedException if the URL or any embedded URL fails the check, or a {@code jar:} URL is malformed. + */ + static void checkUrl(final URL url, final Set validSchemes, final Set validHosts) { + String scheme = url.getProtocol(); + checkScheme(scheme, validSchemes); + if ("jar".equalsIgnoreCase(scheme)) { + try { + // Follows the logic of JarURLConnection#parseSpecs without the cost of opening a connection. + final String spec = url.getFile(); + final int sep = spec.lastIndexOf("!/"); + if (sep < 0) { + throw new MalformedURLException("no !/ found in url spec:" + spec); + } + final URL inner = new URL(spec.substring(0, sep)); + checkUrl(inner, validSchemes, validHosts); + } catch (final MalformedURLException e) { + throw new ConfigurationDeniedException(e, "Malformed 'jar:' URL: %s", url); + } + } else { + checkHost(url.getHost(), validHosts); + } + } + private static Set getSchemesProperty() { final Set set = new LinkedHashSet<>(); final String[] split = System.getProperty(KEY_SCHEMES, DEFAULT_SCHEMES).split(","); @@ -247,27 +297,11 @@ private static Set getSchemesProperty() { URL check(final URL url) { if (url != null) { - checkScheme("scheme", url, url.getProtocol(), schemes); - checkHost("host", url, url.getHost(), hosts); + checkUrl(url, schemes, hosts); } return url; } - void checkHost(final String type, final URL url, final String value, final Set validSet) { - if (!validSet.isEmpty() && StringUtils.isNotEmpty(value)) { - hosts.stream().filter(p -> p.matcher(StringUtils.toRootLowerCase(value)).matches()).findFirst() - .orElseThrow(() -> new ConfigurationDeniedException(String.format("URL %s is not enabled: %s; must be one of %s", type, value, validSet))); - } - } - - void checkScheme(final String type, final URL url, final String value, final Set validSet) { - if (!validSet.isEmpty() && value != null && !validSet.contains(StringUtils.toRootLowerCase(value))) { - throw new ConfigurationDeniedException(String.format( - "URL %s \"%s\" is not enabled, must be one of %s, override defaults with the system property \"%s\", complete set: \"file,http,https,jar\"", - type, value, validSet, KEY_SCHEMES)); - } - } - /** * Gets the enabled hosts. * diff --git a/src/test/java/org/apache/commons/configuration2/io/TestAbstractFileLocationStrategy.java b/src/test/java/org/apache/commons/configuration2/io/TestAbstractFileLocationStrategy.java index 5ceb2e7997..ac915278a7 100644 --- a/src/test/java/org/apache/commons/configuration2/io/TestAbstractFileLocationStrategy.java +++ b/src/test/java/org/apache/commons/configuration2/io/TestAbstractFileLocationStrategy.java @@ -17,17 +17,101 @@ package org.apache.commons.configuration2.io; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.net.URL; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.regex.Pattern; +import java.util.stream.Stream; + +import org.apache.commons.configuration2.ex.ConfigurationDeniedException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Tests {@link AbstractFileLocationStrategy}. */ public class TestAbstractFileLocationStrategy { + private static URL url(final String spec) throws Exception { + return new URL(spec); + } + + // Bypasses the validation of the single-arg constructor + private static URL jarUrl(final String spec) throws Exception { + return new URL("jar", null, spec); + } + + private static Set schemes(final String... values) { + return new LinkedHashSet<>(Arrays.asList(values)); + } + + private static Set hosts(final String... regexes) { + final LinkedHashSet set = new LinkedHashSet<>(); + for (final String r : regexes) { + set.add(Pattern.compile(r, Pattern.CASE_INSENSITIVE)); + } + return set; + } + + static Stream testCheckUrlAccepts() throws Exception { + return Stream.of( + // Empty scheme allows all. + Arguments.of(url("file:/tmp/x.properties"), schemes(), hosts()), + Arguments.of(url("https://example.com/x.properties"), schemes(), hosts()), + // Bare schemes that match the allow-set. + Arguments.of(url("file:/tmp/x.properties"), schemes("file"), hosts()), + Arguments.of(url("https://example.com/x.properties"), schemes("https"), hosts()), + // jar: unwraps to the inner scheme, which is in the allow-set. + Arguments.of(url("jar:file:/tmp/x.jar!/y.properties"), schemes("file", "jar"), hosts()), + Arguments.of(url("jar:https://example.com/x.jar!/y.properties"), schemes("https", "jar"), hosts()), + // Empty host allow-set means "any host". + Arguments.of(url("file:///tmp/x.properties"), schemes("file"), hosts()), + Arguments.of(url("http://anything.example/x.properties"), schemes("http"), hosts()), + Arguments.of(url("jar:https://anything.example/x.jar!/y.properties"), schemes("https", "jar"), hosts()), + // Host satisfies allow-set + Arguments.of(url("file:///tmp/x.properties"), schemes("file"), hosts("trusted\\.example")), + Arguments.of(url("https://trusted.example/x.properties"), schemes("https", "jar"), hosts("trusted\\.example")), + Arguments.of(url("jar:https://trusted.example/x.jar!/y.properties"), schemes("https", "jar"), hosts("trusted\\.example")) + ); + } + + static Stream testCheckUrlRejects() throws Exception { + return Stream.of( + // Plain scheme not in the allow-set. + Arguments.of(url("http://example.com/x.properties"), schemes("file", "jar"), hosts()), + // jar: is allowed but the inner scheme is not. + Arguments.of(url("jar:file:/tmp/x.jar!/y.properties"), schemes("jar"), hosts()), + Arguments.of(url("jar:https://example.com/x.jar!/y.properties"), schemes("jar"), hosts()), + // Invalid jar URL + Arguments.of(jarUrl("file:/tmp/x.properties"), schemes("file", "jar"), hosts()), + Arguments.of(jarUrl("invalid url!/y.properties"), schemes("file", "jar"), hosts()), + // Host is not allowed + Arguments.of(url("https://evilhost/x.properties"), schemes(), hosts("trusted\\.example")), + Arguments.of(url("jar:https://evilhost/x.jar!/y.properties"), schemes(), hosts("trusted\\.example")) + ); + } + @Test void testBuilder() { assertThrows(NullPointerException.class, () -> new AbstractFileLocationStrategy.StrategyBuilder<>(null)); } + + @ParameterizedTest + @MethodSource + void testCheckUrlAccepts(final URL url, final Set validSchemes, final Set validHosts) { + assertDoesNotThrow(() -> AbstractFileLocationStrategy.checkUrl(url, validSchemes, validHosts)); + } + + @ParameterizedTest + @MethodSource + void testCheckUrlRejects(final URL url, final Set validSchemes, final Set validHosts) { + assertThrows(ConfigurationDeniedException.class, () -> AbstractFileLocationStrategy.checkUrl(url, validSchemes, validHosts)); + } + }