This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-configuration.git
The following commit(s) were added to refs/heads/master by this push:
new d14950e02 Extend scheme validation to inner schemes of jar: URLs (#636)
d14950e02 is described below
commit d14950e026bc007d4af73964dca7aeabddc9d915
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Tue May 12 13:33:30 2026 +0200
Extend scheme validation to inner schemes of jar: URLs (#636)
* Extend scheme validation to inner schemes of jar: URLs
Builds on #633 by recursively validating the inner URL of a jar: URL
against the same scheme and host allow-lists.
This deliberately changes the previous semantics: for `jar:http://host/...`
to be accepted, both `jar` and `http` must appear in the allow-list, and the
inner host must satisfy the host allow-list.
An alternative considered was the grammar documented by
[`XMLConstants`](https://docs.oracle.com/en/java/javase/25/docs/api/java.xml/javax/xml/XMLConstants.html),
where tokens like `jar:file` or `jar:http` would explicitly allow specific
inner schemes. That grammar is documented but not honored by the JDK reference
implementation: `jdk.xml.internal.SecuritySupport.checkAccess`
(verified on JDK 8, 17 and 25) strips the `jar:` prefix and matches only
the inner scheme as a bare token, so a `jar:http` entry in the allow-list never
matches anything. Aligning with the documented spec would have added
marginal expressiveness at the cost of diverging from what JDKs actually do.
* fix: use `anyMatch`
* fix: useless `String.format`
* fix: anyMatch -> noneMatch
The classical inversion bug. :wink:
* fix: import and formatting errors
* fix: no parameterized test `name`
* fix: apply review comments
---
.../ex/ConfigurationDeniedException.java | 12 ++++
.../io/AbstractFileLocationStrategy.java | 68 +++++++++++++-----
.../io/TestAbstractFileLocationStrategy.java | 84 ++++++++++++++++++++++
3 files changed, 147 insertions(+), 17 deletions(-)
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 69ef3ca25..c31c27007 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 1eb5716a5..5173e424f 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 abstract class AbstractFileLocationStrategy
implements FileLocationStrate
*/
private static final String KEY_SCHEMES =
"org.apache.commons.configuration2.io.FileLocationStrategy.schemes";
+ private static void checkHost(final String value, final Set<Pattern>
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<String>
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<String> validSchemes, final
Set<Pattern> 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<String> getSchemesProperty() {
final Set<String> set = new LinkedHashSet<>();
final String[] split = System.getProperty(KEY_SCHEMES,
DEFAULT_SCHEMES).split(",");
@@ -247,27 +297,11 @@ public abstract class AbstractFileLocationStrategy
implements FileLocationStrate
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<Pattern> 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<String> 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 5ceb2e799..ac915278a 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<String> schemes(final String... values) {
+ return new LinkedHashSet<>(Arrays.asList(values));
+ }
+
+ private static Set<Pattern> hosts(final String... regexes) {
+ final LinkedHashSet<Pattern> set = new LinkedHashSet<>();
+ for (final String r : regexes) {
+ set.add(Pattern.compile(r, Pattern.CASE_INSENSITIVE));
+ }
+ return set;
+ }
+
+ static Stream<Arguments> 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<Arguments> 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<String> validSchemes,
final Set<Pattern> validHosts) {
+ assertDoesNotThrow(() -> AbstractFileLocationStrategy.checkUrl(url,
validSchemes, validHosts));
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ void testCheckUrlRejects(final URL url, final Set<String> validSchemes,
final Set<Pattern> validHosts) {
+ assertThrows(ConfigurationDeniedException.class, () ->
AbstractFileLocationStrategy.checkUrl(url, validSchemes, validHosts));
+ }
+
}
