Skip to content

Commit 3fb0fbc

Browse files
duncdrumclaude
andcommitted
[ignore] Fix Codacy field-placement and setAccessible() findings
Move XSD11_DETECTION_CACHE/XSD11_DETECTION_CACHE_MAX_ENTRIES to the top of Jaxp, alongside its other fields, instead of declaring them mid-class next to the methods that use them. Make isXsd11Schema package-private instead of private, the same pattern already used for clearXsd11DetectionCache(), so JaxpSchemaLocationSecurityTest/JaxpXsd11DetectionCacheTest (both in the same package) can call it directly instead of via setAccessible(true). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 964ba38 commit 3fb0fbc

3 files changed

Lines changed: 37 additions & 46 deletions

File tree

exist-core/src/main/java/org/exist/xquery/functions/validation/Jaxp.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,25 @@ public class Jaxp extends BasicFunction {
111111
private static final String XSI_NS = "http://www.w3.org/2001/XMLSchema-instance";
112112
private static final String XSD_VERSIONING_NS = "http://www.w3.org/2007/XMLSchema-versioning";
113113

114+
/**
115+
* Bound on the size of {@link #XSD11_DETECTION_CACHE} (see there for what it caches).
116+
*/
117+
private static final int XSD11_DETECTION_CACHE_MAX_ENTRIES = 256;
118+
119+
/**
120+
* Bounded (see {@link #XSD11_DETECTION_CACHE_MAX_ENTRIES}), LRU-evicted cache of
121+
* "does the schema at this resolved URI declare vc:minVersion 1.1?", so that validating many
122+
* documents against the same schema doesn't re-fetch and re-peek it every time. Cleared by
123+
* {@code validation:clear-grammar-cache()} (see {@link GrammarTooling}) alongside the Xerces
124+
* grammar pool, so operators have one function to clear every validation-related cache.
125+
*/
126+
private static final Map<String, Boolean> XSD11_DETECTION_CACHE = new LinkedHashMap<>(16, 0.75f, true) {
127+
@Override
128+
protected boolean removeEldestEntry(final Map.Entry<String, Boolean> eldest) {
129+
return size() > XSD11_DETECTION_CACHE_MAX_ENTRIES;
130+
}
131+
};
132+
114133
private static final String simpleFunctionTxt = """
115134
Validate document by parsing $instance. Optionally \
116135
grammar caching can be enabled. Supported grammars types \
@@ -593,6 +612,8 @@ private static boolean detectXsd11ViaSchemaLocation(final InputSource peekInstan
593612
* and checks whether its root element declares {@code vc:minVersion} containing "1.1".
594613
* Returns {@code false} for any resolution/read failure -- this is a best-effort peek, not
595614
* a substitute for the real catalog-aware resolution the actual validation pass performs.
615+
* Package-private (not {@code private}) so {@code JaxpSchemaLocationSecurityTest}/
616+
* {@code JaxpXsd11DetectionCacheTest}, both in this package, can call it directly.
596617
*
597618
* <p>{@code location} is the literal, attacker/document-author-controlled value of the
598619
* instance's own {@code xsi:schemaLocation}/{@code noNamespaceSchemaLocation} hint -- if it's
@@ -619,7 +640,7 @@ private static boolean detectXsd11ViaSchemaLocation(final InputSource peekInstan
619640
* {@code util:} Java-interop functions to construct that object in the first place -- a
620641
* separate, pre-existing privilege boundary this peek doesn't change either way.</p>
621642
*/
622-
private static boolean isXsd11Schema(final String baseUri, final String location) {
643+
static boolean isXsd11Schema(final String baseUri, final String location) {
623644
try {
624645
final URI baseUriNormalized = new URI(ResolverFactory.fixupExistCatalogUri(baseUri));
625646
final URI resolvedUri = baseUriNormalized.resolve(location);
@@ -661,22 +682,6 @@ private static boolean isXsd11Schema(final String baseUri, final String location
661682
}
662683
}
663684

664-
/**
665-
* Bounded (see {@link #XSD11_DETECTION_CACHE_MAX_ENTRIES}), LRU-evicted cache of
666-
* "does the schema at this resolved URI declare vc:minVersion 1.1?", so that validating many
667-
* documents against the same schema doesn't re-fetch and re-peek it every time. Cleared by
668-
* {@code validation:clear-grammar-cache()} (see {@link GrammarTooling}) alongside the Xerces
669-
* grammar pool, so operators have one function to clear every validation-related cache.
670-
*/
671-
private static final int XSD11_DETECTION_CACHE_MAX_ENTRIES = 256;
672-
673-
private static final Map<String, Boolean> XSD11_DETECTION_CACHE = new LinkedHashMap<>(16, 0.75f, true) {
674-
@Override
675-
protected boolean removeEldestEntry(final Map.Entry<String, Boolean> eldest) {
676-
return size() > XSD11_DETECTION_CACHE_MAX_ENTRIES;
677-
}
678-
};
679-
680685
@Nullable
681686
private static Boolean getCachedXsd11Detection(final String resolvedUri) {
682687
synchronized (XSD11_DETECTION_CACHE) {

exist-core/src/test/java/org/exist/xquery/functions/validation/JaxpSchemaLocationSecurityTest.java

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.junit.BeforeClass;
2626
import org.junit.Test;
2727

28-
import java.lang.reflect.Method;
2928
import java.nio.charset.StandardCharsets;
3029
import java.nio.file.Files;
3130
import java.nio.file.Path;
@@ -40,8 +39,8 @@
4039
* own base URI, since the hint is document-author-controlled and the peek runs unconditionally,
4140
* before any catalog/permission check would otherwise govern the fetch.
4241
*
43-
* <p>Tests the method directly (via reflection, since it's {@code private static}) rather than
44-
* through the full {@code validation:jaxp()} pipeline: going through the pipeline would also
42+
* <p>Tests {@code Jaxp.isXsd11Schema} directly rather than through the full {@code
43+
* validation:jaxp()} pipeline: going through the pipeline would also
4544
* exercise the (separate, pre-existing, unrelated) default Xerces resolution that runs when the
4645
* peek declines and falls through -- which would independently attempt the same kind of fetch,
4746
* confounding any attempt to observe whether the peek itself made a network call.</p>
@@ -77,43 +76,37 @@ public static void teardown() throws Exception {
7776
}
7877

7978
@Test
80-
public void sameOriginRelativeXsd11SchemaIsDetected() throws Exception {
81-
assertTrue(invokeIsXsd11Schema(fileBaseUri, "schema11.xsd"));
79+
public void sameOriginRelativeXsd11SchemaIsDetected() {
80+
assertTrue(Jaxp.isXsd11Schema(fileBaseUri, "schema11.xsd"));
8281
}
8382

8483
@Test
85-
public void sameOriginRelativeXsd10SchemaIsNotDetectedAsXsd11() throws Exception {
86-
assertFalse(invokeIsXsd11Schema(fileBaseUri, "schema10.xsd"));
84+
public void sameOriginRelativeXsd10SchemaIsNotDetectedAsXsd11() {
85+
assertFalse(Jaxp.isXsd11Schema(fileBaseUri, "schema10.xsd"));
8786
}
8887

8988
@Test(timeout = 5000)
90-
public void crossOriginHttpLocationIsRefused() throws Exception {
89+
public void crossOriginHttpLocationIsRefused() {
9190
// A real instance would never have a `file://` base URI reachable from an unprivileged
9291
// caller (only Java-object-backed items do, which already requires elevated capability) --
9392
// this is just the most convenient same-scheme baseline to contrast against. The case that
9493
// matters is: whatever the base URI's origin, an absolute, different-origin location must
9594
// never be followed.
96-
assertFalse(invokeIsXsd11Schema(fileBaseUri, "http://203.0.113.1:1/evil.xsd"));
95+
assertFalse(Jaxp.isXsd11Schema(fileBaseUri, "http://203.0.113.1:1/evil.xsd"));
9796
}
9897

9998
@Test(timeout = 5000)
100-
public void crossOriginHttpLocationIsRefusedForDatabaseBaseUri() throws Exception {
99+
public void crossOriginHttpLocationIsRefusedForDatabaseBaseUri() {
101100
// The realistic, unprivileged case: a document stored in the database (xmldb:// base URI)
102101
// with an absolute http:// schemaLocation hint pointing out to an attacker-controlled host.
103-
assertFalse(invokeIsXsd11Schema("xmldb://db/test/instance.xml", "http://203.0.113.1:1/evil.xsd"));
102+
assertFalse(Jaxp.isXsd11Schema("xmldb://db/test/instance.xml", "http://203.0.113.1:1/evil.xsd"));
104103
}
105104

106105
@Test(timeout = 5000)
107-
public void crossHostXmldbLocationIsRefused() throws Exception {
106+
public void crossHostXmldbLocationIsRefused() {
108107
// An absolute xmldb:// location naming a different host is not "same scheme" enough --
109108
// XmldbURL.isEmbedded() treats a non-empty host as a remote XML-RPC target
110109
// (EmbeddedURLConnection -> XmlrpcInputStream), so this must be refused too.
111-
assertFalse(invokeIsXsd11Schema("xmldb://db/test/instance.xml", "xmldb://203.0.113.1:1/db/evil.xsd"));
112-
}
113-
114-
private static boolean invokeIsXsd11Schema(final String baseUri, final String location) throws Exception {
115-
final Method method = Jaxp.class.getDeclaredMethod("isXsd11Schema", String.class, String.class);
116-
method.setAccessible(true);
117-
return (boolean) method.invoke(null, baseUri, location);
110+
assertFalse(Jaxp.isXsd11Schema("xmldb://db/test/instance.xml", "xmldb://203.0.113.1:1/db/evil.xsd"));
118111
}
119112
}

exist-core/src/test/java/org/exist/xquery/functions/validation/JaxpXsd11DetectionCacheTest.java

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.junit.After;
2525
import org.junit.Test;
2626

27-
import java.lang.reflect.Method;
2827
import java.nio.charset.StandardCharsets;
2928
import java.nio.file.Files;
3029
import java.nio.file.Path;
@@ -64,30 +63,24 @@ public void cachesResultAcrossCallsAndClearCacheInvalidatesIt() throws Exception
6463
final String baseUri = instance.toUri().toString();
6564

6665
// First call: reads the real (XSD 1.1) content from disk.
67-
assertTrue(invokeIsXsd11Schema(baseUri, "schema.xsd"));
66+
assertTrue(Jaxp.isXsd11Schema(baseUri, "schema.xsd"));
6867

6968
// Flip the on-disk content to XSD 1.0 without going through the cache -- if the second
7069
// call is actually served from cache, it must still report the stale (cached) "true",
7170
// not re-read this new content.
7271
Files.writeString(schema, XSD_1_0_SCHEMA, StandardCharsets.UTF_8);
7372
assertTrue("second call should be served from cache, not re-read the changed file",
74-
invokeIsXsd11Schema(baseUri, "schema.xsd"));
73+
Jaxp.isXsd11Schema(baseUri, "schema.xsd"));
7574

7675
// Clearing the cache (what validation:clear-grammar-cache() does) must make the next
7776
// call re-read the file and observe the now-current (XSD 1.0) content.
7877
Jaxp.clearXsd11DetectionCache();
7978
assertFalse("after clearing the cache, the now-current XSD 1.0 content must be observed",
80-
invokeIsXsd11Schema(baseUri, "schema.xsd"));
79+
Jaxp.isXsd11Schema(baseUri, "schema.xsd"));
8180
} finally {
8281
Files.deleteIfExists(tempDir.resolve("instance.xml"));
8382
Files.deleteIfExists(tempDir.resolve("schema.xsd"));
8483
Files.deleteIfExists(tempDir);
8584
}
8685
}
87-
88-
private static boolean invokeIsXsd11Schema(final String baseUri, final String location) throws Exception {
89-
final Method method = Jaxp.class.getDeclaredMethod("isXsd11Schema", String.class, String.class);
90-
method.setAccessible(true);
91-
return (boolean) method.invoke(null, baseUri, location);
92-
}
9386
}

0 commit comments

Comments
 (0)