You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@juneau.apache.org by ja...@apache.org on 2026/08/16 19:33:49 UTC
(juneau) branch master updated (bd08d89b25 -> 0219941939)
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
from bd08d89b25 TODO-397: Add injectable LoginStateStore SPI for multi-node OIDC login state
new 4c63c6acb9 READY-393: Reject CR/LF and control chars in multipart part name/filename/content-type
new 18e68d819d READY-387: Enforce declared document-length bounds in the BSON parser
new 1b22e38502 READY-379: Bind SAML assertions to the request ACS recipient (bearer subject-confirmation)
new e3adb833a7 READY-363: Fix RestContext annotationWork init-ordering NPE in framework-bean memoizers
new 694bed1ddc READY-388/391: Bound Protobuf LEN fields by maxLength and reject overlong varints
new ad28481527 READY-395: Don't mount unauthenticated /info and /loggers actuator endpoints by default
new 69fca6f618 READY-382: Enforce HTTPS-or-loopback on the OidcDiscoveryClient issuer
new 0219941939 READY-384: Remove the MCP DCR requireSecure(false) plaintext escape hatch
The 8 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../juneau/marshall/bson/BsonInputStream.java | 82 +++++++++-
.../juneau/marshall/protobuf/ProtobufReader.java | 37 ++++-
.../juneau/marshall/bson/BsonInputStream_Test.java | 68 ++++++++
.../marshall/bson/BsonParserSession_Test.java | 5 +-
.../marshall/protobuf/ProtobufReader_Test.java | 42 +++++
.../rest/server/auth/saml/SamlAuthFilter_Test.java | 3 +-
.../management/BasicActuatorGroup.java | 26 ++-
.../management/BasicActuatorGroup_Test.java | 86 +++++++---
.../rest/auth/oauth/oidc/OidcDiscoveryClient.java | 4 +
.../oidc}/OidcDiscoveryClient_Builder_Test.java | 40 +++--
.../client/mcp/auth/McpDynamicClientRegistrar.java | 14 +-
.../mcp/auth/McpDynamicClientRegistrar_Test.java | 7 +
.../apache/juneau/http/entity/MultipartBody.java | 67 +++++++-
.../apache/juneau/http/entity/HttpBodies_Test.java | 53 ++++++
.../server/auth/saml/SamlAssertionValidator.java | 69 ++++++--
.../rest/server/auth/saml/SamlAuthFilter.java | 71 +++++++-
...ssertionValidator_SubjectConfirmation_Test.java | 44 +++++
.../auth/saml/SamlAuthFilter_MaxInflate_Test.java | 3 +-
.../saml/SamlAuthFilter_RecipientBinding_Test.java | 182 +++++++++++++++++++++
.../rest/server/auth/saml/SamlAuthFilter_Test.java | 4 +-
.../org/apache/juneau/rest/server/RestContext.java | 24 ++-
.../rest/server/RestContext_Construction_Test.java | 18 ++
22 files changed, 841 insertions(+), 108 deletions(-)
copy juneau-rest/{juneau-rest-server-auth-oauth/src/test/java/org/apache/juneau/rest/server/auth/oauth => juneau-rest-auth-oauth-flow/src/test/java/org/apache/juneau/rest/auth/oauth/oidc}/OidcDiscoveryClient_Builder_Test.java (55%)
create mode 100644 juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_RecipientBinding_Test.java
(juneau) 08/08: READY-384: Remove the MCP DCR requireSecure(false) plaintext escape hatch
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 021994193951eaefff639b364717853ce900ed4f
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:33:34 2026 -0400
READY-384: Remove the MCP DCR requireSecure(false) plaintext escape hatch
---
.../rest/client/mcp/auth/McpDynamicClientRegistrar.java | 14 +-------------
.../client/mcp/auth/McpDynamicClientRegistrar_Test.java | 7 +++++++
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/juneau-rest/juneau-rest-client-mcp-auth/src/main/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar.java b/juneau-rest/juneau-rest-client-mcp-auth/src/main/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar.java
index 84eb649e65..7c892c40fb 100644
--- a/juneau-rest/juneau-rest-client-mcp-auth/src/main/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar.java
+++ b/juneau-rest/juneau-rest-client-mcp-auth/src/main/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar.java
@@ -102,7 +102,6 @@ public class McpDynamicClientRegistrar {
boolean confidential;
String clientName;
Supplier<String> initialAccessTokenSupplier;
- boolean requireSecure = true;
Duration httpTimeout = DEFAULT_HTTP_TIMEOUT;
Consumer<HTTPRequest> httpRequestConfigurator;
@@ -254,17 +253,6 @@ public class McpDynamicClientRegistrar {
return this;
}
- /**
- * Whether to require the registration endpoint to use {@code https} (loopback exempt). Default <jk>true</jk>.
- *
- * @param value <jk>false</jk> to allow a plaintext registration endpoint (testing only).
- * @return This object.
- */
- public Builder requireSecure(boolean value) {
- requireSecure = value;
- return this;
- }
-
/**
* Sets the connect/read timeout applied to the registration request. Default 10 seconds.
*
@@ -303,7 +291,7 @@ public class McpDynamicClientRegistrar {
throw isex("McpDynamicClientRegistrar requires applicationType(...) (SEP-837)");
if (redirectUris.isEmpty())
throw isex("McpDynamicClientRegistrar requires at least one redirectUri(...)");
- if (requireSecure && ! isSecureOrLoopback(registrationEndpoint))
+ if (! isSecureOrLoopback(registrationEndpoint))
throw isex("McpDynamicClientRegistrar registration endpoint must use https (loopback exempt): " + registrationEndpoint);
return new McpDynamicClientRegistrar(this);
}
diff --git a/juneau-rest/juneau-rest-client-mcp-auth/src/test/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar_Test.java b/juneau-rest/juneau-rest-client-mcp-auth/src/test/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar_Test.java
index ee08186ef5..2656af6cc7 100644
--- a/juneau-rest/juneau-rest-client-mcp-auth/src/test/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar_Test.java
+++ b/juneau-rest/juneau-rest-client-mcp-auth/src/test/java/org/apache/juneau/rest/client/mcp/auth/McpDynamicClientRegistrar_Test.java
@@ -20,6 +20,7 @@ import static org.apache.juneau.BasicTestUtils.assertThrowsWithMessage;
import static org.junit.jupiter.api.Assertions.*;
import java.net.*;
+import java.util.*;
import org.apache.juneau.*;
import org.junit.jupiter.api.*;
@@ -81,6 +82,12 @@ class McpDynamicClientRegistrar_Test extends TestBase {
.addRedirectUri(URI.create("http://127.0.0.1/callback")).build());
}
+ @Test void a06_noRequireSecureEscapeHatch() {
+ var hasRequireSecure = Arrays.stream(McpDynamicClientRegistrar.Builder.class.getMethods())
+ .anyMatch(m -> m.getName().equals("requireSecure"));
+ assertFalse(hasRequireSecure, "Builder must not expose a public requireSecure(...) escape hatch");
+ }
+
// Request construction
@Test void b01_emitsApplicationTypeNative() {
(juneau) 06/08: READY-395: Don't mount unauthenticated /info and /loggers actuator endpoints by default
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit ad28481527110acb0989f1772a31212e074a5560
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:33:14 2026 -0400
READY-395: Don't mount unauthenticated /info and /loggers actuator endpoints by default
---
.../management/BasicActuatorGroup.java | 26 +++++--
.../management/BasicActuatorGroup_Test.java | 86 ++++++++++++++++------
2 files changed, 82 insertions(+), 30 deletions(-)
diff --git a/juneau-microservice/juneau-microservice/src/main/java/org/apache/juneau/microservice/management/BasicActuatorGroup.java b/juneau-microservice/juneau-microservice/src/main/java/org/apache/juneau/microservice/management/BasicActuatorGroup.java
index 28784b2af8..7f6a6b5e03 100644
--- a/juneau-microservice/juneau-microservice/src/main/java/org/apache/juneau/microservice/management/BasicActuatorGroup.java
+++ b/juneau-microservice/juneau-microservice/src/main/java/org/apache/juneau/microservice/management/BasicActuatorGroup.java
@@ -28,13 +28,26 @@ import org.apache.juneau.rest.server.servlet.*;
* Mounts the management endpoints by composing their {@code *Mixin} flavors via
* {@link Rest#mixins() @Rest(mixins=...)} on top of {@link BasicRestServletGroup}:
* <ul>
- * <li>{@code /info} — {@link InfoMixin} (manifest/build/version/git metadata)
- * <li>{@code /loggers}, {@code /loggers/{name}} — {@link LoggersMixin} (runtime JUL level get/set)
* <li>{@code /healthz}, {@code /readyz}, {@code /livez} — {@link HealthMixin} (read-through health view)
* <li>{@code /threaddump}, {@code /heapdump} — {@link DumpsMixin} (deny-by-default diagnostics)
* </ul>
*
* <p>
+ * <b>{@code /info} and {@code /loggers} are off by default</b> — unlike health, they are not mounted
+ * on the bare group, since {@link InfoMixin} discloses the full manifest and {@link LoggersMixin}'s read side
+ * discloses logger topology, neither behind any auth. Turn either back on (either approach suffices):
+ * <ul>
+ * <li><b>A-la-carte (recommended):</b> mount {@link InfoMixin} / {@link LoggersMixin} on a resource you
+ * control, e.g. a {@code BasicActuatorGroup} subclass with
+ * {@code @Rest(mixins={InfoMixin.class, LoggersMixin.class})} — mixins declared on a subclass are
+ * additive to the mixins inherited from this class, so {@code HealthMixin}/{@code DumpsMixin} stay mounted
+ * too.
+ * <li><b>Zero-code:</b> not currently supported — {@code @Rest(mixins=...)} is a compile-time class
+ * list, and the {@code RestContext.Builder} injection point that would have let a system property add a
+ * mixin at init time was removed prior to 10.0.0 (see {@link RestInit}). Use the a-la-carte subclass above.
+ * </ul>
+ *
+ * <p>
* <b>Path prefix:</b> defaults to {@code /actuator} and is configurable via the {@code juneau.actuator.path}
* system property (resolved through the standard {@code $S{...}} SVL var). Mount the group at a custom prefix
* either by setting that property or by subclassing with your own {@link Rest#path() @Rest(path=...)}.
@@ -46,10 +59,9 @@ import org.apache.juneau.rest.server.servlet.*;
* (which this module does not depend on), so add {@code MetricsMixin} a-la-carte when that module is present.
*
* <p>
- * <b>Exposure policy:</b> the read endpoints ({@code /info}, {@code /loggers} read, health) are on; the
- * mutating/sensitive ones ({@code /loggers} write, {@code /threaddump}, {@code /heapdump}) are deny-by-default
- * — the dumps via {@link DumpsSettings}, and logger-writes should be guarded by the consumer. No auth
- * provider is auto-wired (explicit-over-magic).
+ * <b>Exposure policy:</b> of the mounted endpoints, health is on; the mutating/sensitive ones
+ * ({@code /threaddump}, {@code /heapdump}) are deny-by-default via {@link DumpsSettings}. No auth provider is
+ * auto-wired (explicit-over-magic).
*
* <h5 class='section'>See Also:</h5><ul>
* <li class='link'><a class="doclink" href="https://juneau.apache.org/docs/topics/ManagementSurface">Management Surface</a>
@@ -63,8 +75,6 @@ import org.apache.juneau.rest.server.servlet.*;
title="Management",
description="Actuator-style management surface.",
mixins={
- InfoMixin.class,
- LoggersMixin.class,
HealthMixin.class,
DumpsMixin.class
}
diff --git a/juneau-microservice/juneau-microservice/src/test/java/org/apache/juneau/microservice/management/BasicActuatorGroup_Test.java b/juneau-microservice/juneau-microservice/src/test/java/org/apache/juneau/microservice/management/BasicActuatorGroup_Test.java
index 223f36149c..e24a494771 100644
--- a/juneau-microservice/juneau-microservice/src/test/java/org/apache/juneau/microservice/management/BasicActuatorGroup_Test.java
+++ b/juneau-microservice/juneau-microservice/src/test/java/org/apache/juneau/microservice/management/BasicActuatorGroup_Test.java
@@ -31,9 +31,9 @@ import org.apache.juneau.rest.server.servlet.*;
import org.junit.jupiter.api.*;
/**
- * Tests for {@link BasicActuatorGroup} — verifies the management endpoints are reachable under the
- * configured prefix, the deny-by-default gating on the diagnostics is honored, and the standalone flavors
- * remain independently mountable.
+ * Tests for {@link BasicActuatorGroup} — verifies that {@code /info} and {@code /loggers} are off by
+ * default (not disclosed to an unauthenticated client), that health and the deny-by-default diagnostics behave
+ * as before, and that a-la-carte subclassing restores {@code /info} and {@code /loggers}.
*/
@SuppressWarnings({
"resource" // Closeable MockRestClient fixtures; lifecycle managed by the test/framework, not a real leak.
@@ -58,9 +58,6 @@ class BasicActuatorGroup_Test extends TestBase {
@Bean public DumpsSettings dumpsSettings() {
return DumpsSettings.create().enableThreadDump().enableHeapDump().build();
}
- @Bean public LoggersSettings loggersSettings() {
- return LoggersSettings.create().enableWrite().build();
- }
}
@Rest(children={ActuatorChild.class})
@@ -73,22 +70,21 @@ class BasicActuatorGroup_Test extends TestBase {
Logger.getLogger(LName).setLevel(null);
}
- @Test void a01_infoReachable() throws Exception {
+ @Test void a01_infoOffByDefault() throws Exception {
var c = MockRestClient.buildLax(A.class);
- c.get("/actuator/info").accept("application/json").run().assertStatus(200)
- .assertContent().asString().isContains("Implementation-Version", "10.0.0");
+ c.get("/actuator/info").accept("application/json").run().assertStatus(404);
}
- @Test void a02_loggersReadReachable() throws Exception {
+ @Test void a02_loggersReadOffByDefault() throws Exception {
var c = MockRestClient.buildLax(A.class);
- c.get("/actuator/loggers").accept("application/json").run().assertStatus(200)
- .assertContent().asString().isContains("ROOT");
+ c.get("/actuator/loggers").accept("application/json").run().assertStatus(404);
}
- @Test void a03_loggersWriteRoundTrip() throws Exception {
+ @Test void a03_loggersWriteOffByDefault() throws Exception {
var c = MockRestClient.buildLax(A.class);
- c.put("/actuator/loggers/" + LName, "FINE").accept("application/json").run().assertStatus(200);
- assertEquals(Level.FINE, Logger.getLogger(LName).getLevel());
+ // The entire LoggersMixin (read and write) is unmounted by default, so the write side is also
+ // unreachable regardless of the child's LoggersSettings bean.
+ c.put("/actuator/loggers/" + LName, "FINE").accept("application/json").run().assertStatus(404);
}
@Test void a04_healthReachable() throws Exception {
@@ -124,16 +120,62 @@ class BasicActuatorGroup_Test extends TestBase {
c.get("/actuator/heapdump").run().assertStatus(403);
}
- @Test void b03_loggersWriteDeniedByDefaultInGroup() throws Exception {
- var c = MockRestClient.buildLax(B.class);
- // Reads still work; the mutating set-level is denied without an opt-in LoggersSettings bean.
- c.get("/actuator/loggers").accept("application/json").run().assertStatus(200);
+ /**
+ * A-la-carte re-enablement: a subclass that adds {@link InfoMixin} and {@link LoggersMixin} restores both
+ * endpoints, on top of the {@link org.apache.juneau.rest.server.health.HealthMixin}/{@link DumpsMixin}
+ * inherited from {@link BasicActuatorGroup} (mixins declared on a subclass are additive, not a replacement).
+ */
+ @Rest(path="/actuator", mixins={InfoMixin.class, LoggersMixin.class})
+ public static class EnabledChild extends BasicActuatorGroup {
+ private static final long serialVersionUID = 1L;
+ @Bean public ManifestFile manifest() throws IOException { return BasicActuatorGroup_Test.manifest(); }
+ }
+
+ @Rest(children={EnabledChild.class})
+ public static class C extends BasicRestServlet {
+ private static final long serialVersionUID = 1L;
+ }
+
+ @Test void c01_infoReachableWhenMixinAddedALaCarte() throws Exception {
+ var c = MockRestClient.buildLax(C.class);
+ c.get("/actuator/info").accept("application/json").run().assertStatus(200)
+ .assertContent().asString().isContains("Implementation-Version", "10.0.0");
+ }
+
+ @Test void c02_loggersReadReachableWhenMixinAddedALaCarte() throws Exception {
+ var c = MockRestClient.buildLax(C.class);
+ c.get("/actuator/loggers").accept("application/json").run().assertStatus(200)
+ .assertContent().asString().isContains("ROOT");
+ }
+
+ @Test void c03_loggersWriteStillDeniedByDefaultWhenMixinAddedALaCarte() throws Exception {
+ var c = MockRestClient.buildLax(C.class);
+ // The read side is reachable once the mixin is mounted, but the mutating set-level endpoint keeps its
+ // own independent deny-by-default policy (no LoggersSettings bean registered on EnabledChild).
c.put("/actuator/loggers/" + LName, "FINE").run().assertStatus(403);
}
- @Test void b02_infoStillReachableWithoutManifest() throws Exception {
- // No manifest bean -> /info degrades to an empty map but stays reachable (200).
- var c = MockRestClient.buildLax(B.class);
+ @Test void c04_healthAndDumpsStillMountedWhenMixinAddedALaCarte() throws Exception {
+ // Confirms subclass mixins are additive: adding Info/Loggers didn't drop the inherited Health/Dumps.
+ var c = MockRestClient.buildLax(C.class);
+ c.get("/actuator/healthz").accept("application/json").run().assertStatus(200);
+ c.get("/actuator/threaddump").run().assertStatus(403);
+ }
+
+ /** A-la-carte {@link InfoMixin} with no manifest bean registered. */
+ @Rest(path="/actuator", mixins={InfoMixin.class})
+ public static class D01_BareEnabledChild extends BasicActuatorGroup {
+ private static final long serialVersionUID = 1L;
+ }
+
+ @Rest(children={D01_BareEnabledChild.class})
+ public static class D01_D extends BasicRestServlet {
+ private static final long serialVersionUID = 1L;
+ }
+
+ @Test void d01_infoStillReachableWithoutManifest() throws Exception {
+ // No manifest bean -> /info degrades to an empty map but stays reachable (200), once mounted a-la-carte.
+ var c = MockRestClient.buildLax(D01_D.class);
c.get("/actuator/info").accept("application/json").run().assertStatus(200).assertContent().asString().is("{}");
}
}
(juneau) 07/08: READY-382: Enforce HTTPS-or-loopback on the OidcDiscoveryClient issuer
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 69fca6f6181aaeb12970a5f9345d5d09a7f1dcb2
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:33:25 2026 -0400
READY-382: Enforce HTTPS-or-loopback on the OidcDiscoveryClient issuer
---
.../rest/auth/oauth/oidc/OidcDiscoveryClient.java | 4 ++
.../oidc/OidcDiscoveryClient_Builder_Test.java | 61 ++++++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git a/juneau-rest/juneau-rest-auth-oauth-flow/src/main/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient.java b/juneau-rest/juneau-rest-auth-oauth-flow/src/main/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient.java
index 7e68906f05..49c9a264df 100644
--- a/juneau-rest/juneau-rest-auth-oauth-flow/src/main/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient.java
+++ b/juneau-rest/juneau-rest-auth-oauth-flow/src/main/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient.java
@@ -24,6 +24,8 @@ import java.time.*;
import java.util.*;
import java.util.function.*;
+import org.apache.juneau.commons.utils.*;
+
import com.nimbusds.oauth2.sdk.*;
import com.nimbusds.oauth2.sdk.as.*;
import com.nimbusds.oauth2.sdk.http.*;
@@ -119,10 +121,12 @@ public class OidcDiscoveryClient {
* Builds the client.
*
* @return A new {@link OidcDiscoveryClient}.
+ * @throws IllegalArgumentException If the issuer is not <js>"https"</js> and does not target a loopback host.
*/
public OidcDiscoveryClient build() {
if (issuer == null)
throw new IllegalStateException("OidcDiscoveryClient requires issuer(...)");
+ UriUtils.assertSecureOrLoopback(issuer);
return new OidcDiscoveryClient(this);
}
}
diff --git a/juneau-rest/juneau-rest-auth-oauth-flow/src/test/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient_Builder_Test.java b/juneau-rest/juneau-rest-auth-oauth-flow/src/test/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient_Builder_Test.java
new file mode 100644
index 0000000000..cbd6497385
--- /dev/null
+++ b/juneau-rest/juneau-rest-auth-oauth-flow/src/test/java/org/apache/juneau/rest/auth/oauth/oidc/OidcDiscoveryClient_Builder_Test.java
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.rest.auth.oauth.oidc;
+
+import static org.apache.juneau.BasicTestUtils.*;
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.net.*;
+
+import org.apache.juneau.*;
+import org.junit.jupiter.api.*;
+
+/**
+ * Builder tests for {@link OidcDiscoveryClient}, focused on the issuer's HTTPS-or-loopback requirement.
+ *
+ * @since 10.0.0
+ */
+class OidcDiscoveryClient_Builder_Test extends TestBase {
+
+ @Test void a01_requireIssuer() {
+ assertThrows(IllegalStateException.class, () -> OidcDiscoveryClient.create().build());
+ }
+
+ @Test void a02_build_httpsAccepted() {
+ var c = OidcDiscoveryClient.create()
+ .issuer(URI.create("https://idp.example.com"))
+ .build();
+ assertEquals(URI.create("https://idp.example.com"), c.getIssuer());
+ }
+
+ @Test void a03_build_loopbackHttpAccepted() {
+ var c1 = OidcDiscoveryClient.create().issuer(URI.create("http://localhost:8080")).build();
+ assertEquals(URI.create("http://localhost:8080"), c1.getIssuer());
+
+ var c2 = OidcDiscoveryClient.create().issuer(URI.create("http://127.0.0.1:8080")).build();
+ assertEquals(URI.create("http://127.0.0.1:8080"), c2.getIssuer());
+
+ var c3 = OidcDiscoveryClient.create().issuer(URI.create("http://[::1]:8080")).build();
+ assertEquals(URI.create("http://[::1]:8080"), c3.getIssuer());
+ }
+
+ @Test void a04_build_remotePlaintextRejected() {
+ assertThrowsWithMessage(IllegalArgumentException.class,
+ "URI must use https or target a loopback host: http://idp.example.com",
+ () -> OidcDiscoveryClient.create().issuer(URI.create("http://idp.example.com")).build());
+ }
+}
(juneau) 05/08: READY-388/391: Bound Protobuf LEN fields by maxLength and reject overlong varints
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 694bed1ddc02f26d5dbe9137910fbec157c7acee
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:33:06 2026 -0400
READY-388/391: Bound Protobuf LEN fields by maxLength and reject overlong varints
---
.../juneau/marshall/protobuf/ProtobufReader.java | 37 ++++++++++++++++---
.../marshall/protobuf/ProtobufReader_Test.java | 42 ++++++++++++++++++++++
2 files changed, 75 insertions(+), 4 deletions(-)
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java
index 4d52c8662e..e7599c174a 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java
@@ -57,6 +57,9 @@ public class ProtobufReader {
/** Default cap (16 MiB) for wire-declared length-delimited block sizes. */
static final int DEFAULT_MAX_LENGTH = 16 * 1024 * 1024;
+ /** Maximum encoded length, in bytes, of a 64-bit protobuf varint (ceil(64 / 7) LEB128 groups). */
+ private static final int MAX_VARINT_BYTES = 10;
+
private final InputStream is;
private int maxLength = DEFAULT_MAX_LENGTH;
@@ -112,19 +115,28 @@ public class ProtobufReader {
/**
* Reads a base-128 varint (LEB128) as a 64-bit value.
*
+ * <p>
+ * Caps the encoding at {@value #MAX_VARINT_BYTES} bytes (the maximum a 64-bit varint can occupy), so a
+ * malformed stream of continuation bytes cannot burn unbounded CPU/bytes before EOF.
+ *
* @return The decoded value.
- * @throws IOException If the stream ends mid-varint or the underlying stream fails.
+ * @throws IOException If the stream ends mid-varint, the varint exceeds {@value #MAX_VARINT_BYTES} bytes,
+ * or the underlying stream fails.
*/
public long readVarint() throws IOException {
var result = 0L;
var shift = 0;
+ var count = 0;
int b;
do {
b = read();
if (b == -1)
throw ioex("Unexpected end of protobuf input while reading varint");
+ count++;
result |= ((long)(b & 0x7F)) << shift;
shift += 7;
+ if (count == MAX_VARINT_BYTES && (b & 0x80) != 0)
+ throw ioex("Protobuf varint exceeds maximum length of %s bytes", MAX_VARINT_BYTES);
} while ((b & 0x80) != 0);
return result;
}
@@ -132,8 +144,12 @@ public class ProtobufReader {
/**
* Reads a field tag, or returns {@link #EOF} if the stream is exhausted at a field boundary.
*
+ * <p>
+ * Caps the encoding at {@value #MAX_VARINT_BYTES} bytes, the same bound applied by {@link #readVarint()}.
+ *
* @return The decoded tag value (<c>(fieldNumber << 3) | wireType</c>), or {@link #EOF} at end of stream.
- * @throws IOException If the stream ends mid-tag or the underlying stream fails.
+ * @throws IOException If the stream ends mid-tag, the tag exceeds {@value #MAX_VARINT_BYTES} bytes, or the
+ * underlying stream fails.
*/
public long readTag() throws IOException {
var b = read();
@@ -141,12 +157,16 @@ public class ProtobufReader {
return EOF;
var result = (long)(b & 0x7F);
var shift = 7;
+ var count = 1;
while ((b & 0x80) != 0) {
b = read();
if (b == -1)
throw ioex("Unexpected end of protobuf input while reading tag");
+ count++;
result |= ((long)(b & 0x7F)) << shift;
shift += 7;
+ if (count == MAX_VARINT_BYTES && (b & 0x80) != 0)
+ throw ioex("Protobuf varint exceeds maximum length of %s bytes", MAX_VARINT_BYTES);
}
return result;
}
@@ -277,15 +297,24 @@ public class ProtobufReader {
/**
* Skips a field's value, consuming exactly the right number of bytes for the specified wire type.
*
+ * <p>
+ * For {@link WireType#LEN}, the wire-declared length is validated against the configured maximum (the
+ * same check applied by {@link #readLenDelimited()}) before skipping, so an unknown forward-compatible
+ * field cannot bypass the length cap via an unbounded or wrapped-negative skip.
+ *
* @param wireType The wire type of the field to skip.
- * @throws IOException If the stream ends early or the underlying stream fails.
+ * @throws IOException If the stream ends early, the declared length is out of bounds, or the underlying
+ * stream fails.
*/
public void skipField(WireType wireType) throws IOException {
switch (wireType) {
case VARINT -> readVarint();
case I64 -> skip(8);
case I32 -> skip(4);
- case LEN -> skip((int)readVarint());
+ case LEN -> {
+ var len = ParserInputStream.checkLength(readVarint(), maxLength, "protobuf field");
+ skip(len);
+ }
default -> throw ioex("Cannot skip unsupported protobuf wire type: %s", wireType);
}
}
diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java
index 13f364fa9c..cc215deb12 100644
--- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java
+++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java
@@ -16,8 +16,11 @@
*/
package org.apache.juneau.marshall.protobuf;
+import static org.apache.juneau.BasicTestUtils.*;
import static org.junit.jupiter.api.Assertions.*;
+import java.io.*;
+
import org.apache.juneau.*;
import org.junit.jupiter.api.*;
@@ -111,4 +114,43 @@ class ProtobufReader_Test extends TestBase {
r4.skipField(WireType.LEN);
assertEquals(2, ProtobufReader.fieldNumber(r4.readTag()));
}
+
+ @Test
+ void a09_varintRejectsOverlongContinuation() throws Exception {
+ // 10 continuation bytes (0x80) with no terminating byte: the 10th byte still has the continuation
+ // bit set, so the varint never terminates within the 10-byte cap.
+ var r = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80);
+ assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r::readVarint);
+
+ // Same shape but with an 11th byte present ("10x0x80 + more") -- still rejected at byte 10.
+ var r2 = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x01);
+ assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r2::readVarint);
+ }
+
+ @Test
+ void a10_varintCanonicalTenByteNegativeOneStillDecodes() throws Exception {
+ // Canonical 10-byte encoding of -1L must remain valid (terminating byte on the 10th).
+ assertEquals(-1L, reader(0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x01).readVarint());
+ }
+
+ @Test
+ void a11_tagRejectsOverlongContinuation() throws Exception {
+ var r = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80);
+ assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r::readTag);
+ }
+
+ @Test
+ void b01_skipFieldLenRejectsLengthAboveMaxLength() throws Exception {
+ // Declared length (10) exceeds the configured maximum (5); must reject before skipping any data.
+ var r = reader(0x0A, 1,2,3,4,5,6,7,8,9,10, 0x10);
+ r.setMaxLength(5);
+ assertThrowsWithMessage(IOException.class, "exceeds maximum allowed", () -> r.skipField(WireType.LEN));
+ }
+
+ @Test
+ void b02_skipFieldLenRejectsWrappedNegativeVarint() throws Exception {
+ // Varint decodes to -1L (wrapped-negative as an int cast); must reject rather than skip a negative count.
+ var r = reader(0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x01);
+ assertThrowsWithMessage(IOException.class, "negative", () -> r.skipField(WireType.LEN));
+ }
}
(juneau) 03/08: READY-379: Bind SAML assertions to the request ACS recipient (bearer subject-confirmation)
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 1b22e38502f51a6181a906b59e995d783e16701b
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:32:47 2026 -0400
READY-379: Bind SAML assertions to the request ACS recipient (bearer subject-confirmation)
Makes SamlAuthFilter.deriveRecipient() null-safe: when getRequestURL()
returns null, the ACS recipient is reconstructed from scheme/server
name/server port/request URI (omitting the port when it's the
scheme's default), and the request is rejected via AuthenticationException
when even that fails, instead of throwing an NPE. Also syncs the
juneau-integration-tests duplicate of SamlAuthFilter_Test with the
module copy's request mock so it exercises the same getRequestURL()
path.
---
.../rest/server/auth/saml/SamlAuthFilter_Test.java | 3 +-
.../server/auth/saml/SamlAssertionValidator.java | 69 ++++++--
.../rest/server/auth/saml/SamlAuthFilter.java | 71 +++++++-
...ssertionValidator_SubjectConfirmation_Test.java | 44 +++++
.../auth/saml/SamlAuthFilter_MaxInflate_Test.java | 3 +-
.../saml/SamlAuthFilter_RecipientBinding_Test.java | 182 +++++++++++++++++++++
.../rest/server/auth/saml/SamlAuthFilter_Test.java | 4 +-
7 files changed, 355 insertions(+), 21 deletions(-)
diff --git a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
index d63d8f92cb..fadaf32376 100644
--- a/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
+++ b/juneau-integration-tests/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
@@ -51,7 +51,7 @@ class SamlAuthFilter_Test extends TestBase {
.spEntityId("https://sp.example.com")
.expectedIssuer("https://idp.example.com")
.signingCredential(new BasicCredential(pair.getPublic(), pair.getPrivate()))) {
- @Override public Principal validate(String xml) {
+ @Override public Principal validate(String xml, String recipient) {
return impl.apply(xml);
}
};
@@ -62,6 +62,7 @@ class SamlAuthFilter_Test extends TestBase {
when(r.getPathInfo()).thenReturn(path);
when(r.getServletPath()).thenReturn(path);
when(r.getParameter("SAMLResponse")).thenReturn(samlResponse);
+ when(r.getRequestURL()).thenReturn(new StringBuffer("https://sp.example.com" + path));
return r;
}
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
index 62044f1d9c..a410dc86ef 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator.java
@@ -72,11 +72,17 @@ import net.shibboleth.shared.resolver.*;
* <li><b>Clock skew</b> — defaults to 60 seconds tolerance on {@code NotBefore} /
* {@code NotOnOrAfter}; capped at 300 seconds.
* <li><b>Audience restriction</b> — {@code <AudienceRestriction>} must list the configured SP entity ID.
- * <li><b>Bearer subject confirmation</b> — when a {@link Builder#recipient(String) recipient} (the SP
- * ACS URL) is configured, the assertion must carry a {@code bearer} {@code <SubjectConfirmation>} whose
- * {@code <SubjectConfirmationData>} matches the expected {@code Recipient}, is within its
- * {@code NotBefore}/{@code NotOnOrAfter} window, and satisfies the configured
- * {@code InResponseTo}/{@code Address} expectations.
+ * <li><b>Bearer subject confirmation</b> — enforced whenever a {@code recipient} (the SP ACS URL) is
+ * known, either because {@link Builder#recipient(String)} was configured on this validator or because
+ * the caller invoked {@link #validate(String, String)} with one. {@link SamlAuthFilter} always calls
+ * {@link #validate(String, String)} with the ACS URL derived from the current request, so a validator
+ * reached only through the filter can never skip this check. When enforced, the assertion must carry a
+ * {@code bearer} {@code <SubjectConfirmation>} whose {@code <SubjectConfirmationData>} matches the
+ * expected {@code Recipient}, is within its {@code NotBefore}/{@code NotOnOrAfter} window, and satisfies
+ * the configured {@code InResponseTo}/{@code Address} expectations. A validator used standalone (not
+ * through {@link SamlAuthFilter}) skips this check unless {@link Builder#recipient(String)} is
+ * configured — that mode is <b>not</b> safe for browser-SSO ACS endpoints, since nothing then binds
+ * the assertion to a specific recipient.
* <li><b>One-time use</b> — each assertion ID is recorded in a {@link ReplayCache} and a second
* presentation of the same ID is rejected. The check is fail-closed: if the cache cannot answer, the
* assertion is rejected. An assertion whose validity window is unbounded — carrying neither a
@@ -100,6 +106,14 @@ import net.shibboleth.shared.resolver.*;
*
* <h5 class='topic'>Builder usage</h5>
*
+ * <p>
+ * This recipe builds a <b>standalone</b> validator with no {@link Builder#recipient(String)} configured, so
+ * bearer subject-confirmation is not enforced. It is safe for assertion-only / non-browser callers that
+ * validate an assertion outside of an ACS redirect. Deploying it behind {@link SamlAuthFilter} for
+ * browser-SSO is always safe regardless: the filter derives the ACS recipient from the request and calls
+ * {@link #validate(String, String)}, which enforces bearer confirmation for that call no matter how this
+ * validator was built.
+ *
* <p class='bjava'>
* <jk>var</jk> validator = SamlAssertionValidator.<jsm>create</jsm>()
* .metadataResolver(SamlMetadataResolvers.<jsm>url</jsm>(<js>"https://idp.example.com/metadata"</js>))
@@ -303,8 +317,10 @@ public class SamlAssertionValidator {
* {@code <SubjectConfirmationData>} names this exact {@code Recipient}, is within its
* {@code NotBefore}/{@code NotOnOrAfter} window, and satisfies the {@link #expectedInResponseTo(String)}
* / {@link #subjectAddress(String)} expectations. When left unset (the default), subject-confirmation
- * validation is skipped — operators consuming bearer assertions from a browser SSO flow should
- * configure this.
+ * validation is skipped for calls to {@link SamlAssertionValidator#validate(String)} — safe only
+ * for assertion-only / non-browser callers. A validator reached through {@link SamlAuthFilter} enforces
+ * bearer confirmation regardless of this setting, since the filter derives the recipient from the
+ * request and calls {@link SamlAssertionValidator#validate(String, String)}.
*
* @param value The expected ACS URL. Must not be <jk>null</jk> or blank.
* @return This object.
@@ -495,12 +511,41 @@ public class SamlAssertionValidator {
*/
public Principal validate(String xml) throws AuthenticationException {
assertArgNotNullOrBlank("xml", xml);
+ return validateInternal(xml, recipient);
+ }
+
+ /**
+ * Validates the supplied SAML 2.0 {@code <samlp:Response>} XML, enforcing bearer
+ * {@code <SubjectConfirmation>} against the given {@code recipient} for this call.
+ *
+ * <p>
+ * Unlike {@link #validate(String)}, bearer subject-confirmation is always enforced here —
+ * {@code recipient} overrides whatever {@link Builder#recipient(String)} this validator was built with
+ * (including unset). {@link SamlAuthFilter} calls this overload with the ACS URL derived from the
+ * current request so that a validator built without {@link Builder#recipient(String)} configured still
+ * rejects an assertion bearer-confirmed to a different endpoint on the filter path.
+ *
+ * @param xml The full XML payload (already base64-decoded; already URL-decoded and inflated for the
+ * REDIRECT binding). Must be a {@code <samlp:Response>} envelope.
+ * @param recipient The expected {@code Recipient} (this SP's ACS URL) for bearer subject-confirmation.
+ * Must not be <jk>null</jk> or blank.
+ * @return A {@link ClaimsPrincipal} carrying the IdP-supplied claims plus the {@code issuerType=SAML}
+ * marker.
+ * @throws AuthenticationException If the response cannot be parsed or validation fails.
+ */
+ public Principal validate(String xml, String recipient) throws AuthenticationException {
+ assertArgNotNullOrBlank("xml", xml);
+ assertArgNotNullOrBlank("recipient", recipient);
+ return validateInternal(xml, recipient);
+ }
+
+ private Principal validateInternal(String xml, String recipient) throws AuthenticationException {
var response = parseResponse(xml);
validateStatus(response);
var assertion = extractAndDecryptAssertion(response);
verifySignature(assertion);
var claims = buildClaims(response, assertion);
- validateSubjectConfirmation(assertion);
+ validateSubjectConfirmation(assertion, recipient);
recordSingleUse(assertion);
var subject = nameId(assertion);
return new ClaimsPrincipal(subject, claims);
@@ -758,8 +803,8 @@ public class SamlAssertionValidator {
}
}
- private void validateSubjectConfirmation(Assertion assertion) throws AuthenticationException {
- if (recipient == null) // Bearer subject-confirmation validation is enabled by configuring recipient(...).
+ private void validateSubjectConfirmation(Assertion assertion, String recipient) throws AuthenticationException {
+ if (recipient == null) // Bearer subject-confirmation validation is enabled by configuring recipient(...) or calling validate(xml, recipient).
return;
var subject = assertion.getSubject();
if (subject == null)
@@ -770,7 +815,7 @@ public class SamlAssertionValidator {
if (! SubjectConfirmation.METHOD_BEARER.equals(sc.getMethod()))
continue;
sawBearer = true;
- var reason = confirmationFailureReason(sc);
+ var reason = confirmationFailureReason(sc, recipient);
if (reason == null)
return; // A valid bearer confirmation was found.
failure = reason;
@@ -780,7 +825,7 @@ public class SamlAssertionValidator {
throw rejectAssertion(failure);
}
- private String confirmationFailureReason(SubjectConfirmation sc) {
+ private String confirmationFailureReason(SubjectConfirmation sc, String recipient) {
var data = sc.getSubjectConfirmationData();
if (data == null)
return "bearer <SubjectConfirmationData> is missing";
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter.java b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter.java
index e2f57cdb76..2d2aa65349 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/main/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter.java
@@ -42,9 +42,13 @@ import jakarta.servlet.http.*;
* <li>For the {@link SamlBinding#POST} binding: reads the {@code SAMLResponse} form parameter and
* base64-decodes it. For {@link SamlBinding#REDIRECT}: reads the {@code SAMLResponse} query parameter,
* base64-decodes it, and then DEFLATE-inflates (RFC 1951) per OASIS SAML 2.0 Redirect binding rules.
- * <li>Delegates to {@link SamlAssertionValidator#validate(String)}. On success, builds an {@link AuthResult}
- * carrying the resolved {@link ClaimsPrincipal}. On failure, re-throws as an
- * {@link AuthenticationException} with a {@code WWW-Authenticate: SAML ...} challenge.
+ * <li>Derives the ACS recipient URL (scheme, host, port, and path) from the current request and delegates
+ * to {@link SamlAssertionValidator#validate(String, String)} with it, so the assertion's bearer
+ * {@code <SubjectConfirmation>} is always bound to the actual endpoint this request was delivered to
+ * — regardless of whether the {@link SamlAssertionValidator} was itself built with
+ * {@link SamlAssertionValidator.Builder#recipient(String) recipient(...)} configured. On success,
+ * builds an {@link AuthResult} carrying the resolved {@link ClaimsPrincipal}. On failure, re-throws as
+ * an {@link AuthenticationException} with a {@code WWW-Authenticate: SAML ...} challenge.
* </ol>
*
* <h5 class='topic'>Roles</h5>
@@ -55,6 +59,12 @@ import jakarta.servlet.http.*;
*
* <h5 class='topic'>Usage</h5>
*
+ * <p>
+ * Note that {@code validator} below is <i>not</i> built with
+ * {@link SamlAssertionValidator.Builder#recipient(String) recipient(...)}. That is safe here because this
+ * filter always derives the ACS recipient from the request and binds it for that call, so bearer
+ * subject-confirmation is enforced regardless of the validator's own configuration.
+ *
* <p class='bjava'>
* <jk>var</jk> validator = SamlAssertionValidator.<jsm>create</jsm>()
* .metadataResolver(SamlMetadataResolvers.<jsm>url</jsm>(<js>"https://idp.example.com/metadata"</js>))
@@ -259,7 +269,7 @@ public class SamlAuthFilter extends AuthFilter {
if (isEmpty(raw))
return oe();
var xml = decodeSamlResponse(raw);
- var principal = runValidator(xml);
+ var principal = runValidator(xml, deriveRecipient(req));
return o(AuthResult.of(principal, extractRoles(principal)));
}
@@ -269,6 +279,55 @@ public class SamlAuthFilter extends AuthFilter {
return s != null && s.equals(consumerPath);
}
+ /**
+ * Derives the ACS recipient URL (scheme, host, port, and path) that this request was actually delivered
+ * to, so it can be bound to the assertion's bearer {@code <SubjectConfirmation>}.
+ *
+ * <p>
+ * Normally this is just {@link HttpServletRequest#getRequestURL()}. Some servlet-container and test
+ * request implementations return <jk>null</jk> from that method, so as a fallback the URL is
+ * reconstructed from {@link HttpServletRequest#getScheme()}, {@link HttpServletRequest#getServerName()},
+ * {@link HttpServletRequest#getServerPort()} (omitted when it's the scheme's default port), and
+ * {@link HttpServletRequest#getRequestURI()}. If the recipient still cannot be determined, the request
+ * is rejected via {@link AuthenticationException} — never with an {@link NullPointerException}.
+ *
+ * @param req The current request; {@link #matchesPath} has already confirmed its path matches
+ * {@code consumerPath}.
+ * @return The recipient URL, e.g. {@code https://sp.example.com/saml/acs}.
+ * @throws AuthenticationException If the recipient URL cannot be determined from the request.
+ */
+ private String deriveRecipient(HttpServletRequest req) throws AuthenticationException {
+ var url = req.getRequestURL();
+ if (url != null)
+ return url.toString();
+ return reconstructRecipient(req);
+ }
+
+ /**
+ * Fallback for {@link #deriveRecipient(HttpServletRequest)} when {@link HttpServletRequest#getRequestURL()}
+ * returns <jk>null</jk>.
+ *
+ * @param req The current request.
+ * @return The reconstructed recipient URL.
+ * @throws AuthenticationException If {@code scheme}, {@code serverName}, or {@code requestURI} is
+ * <jk>null</jk> or blank, so no recipient can be determined.
+ */
+ private String reconstructRecipient(HttpServletRequest req) throws AuthenticationException {
+ var scheme = req.getScheme();
+ var host = req.getServerName();
+ var uri = req.getRequestURI();
+ if (isBlank(scheme) || isBlank(host) || isBlank(uri))
+ throw new AuthenticationException("Unable to derive SAML ACS recipient from request").wwwAuthenticate(challenge);
+ var port = req.getServerPort();
+ var isDefaultPort = port <= 0
+ || (port == 80 && "http".equalsIgnoreCase(scheme))
+ || (port == 443 && "https".equalsIgnoreCase(scheme));
+ var sb = new StringBuilder(scheme).append("://").append(host);
+ if (!isDefaultPort)
+ sb.append(':').append(port);
+ return sb.append(uri).toString();
+ }
+
private String decodeSamlResponse(String raw) throws AuthenticationException {
try {
byte[] decoded = Base64.getMimeDecoder().decode(raw);
@@ -288,9 +347,9 @@ public class SamlAuthFilter extends AuthFilter {
}
}
- private Principal runValidator(String xml) throws AuthenticationException {
+ private Principal runValidator(String xml, String recipient) throws AuthenticationException {
try {
- var p = validator.validate(xml);
+ var p = validator.validate(xml, recipient);
if (p == null)
throw new AuthenticationException("SAML validator returned null").wwwAuthenticate(challenge);
return p;
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_SubjectConfirmation_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_SubjectConfirmation_Test.java
index 2b0e6adcb5..3cb691e181 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_SubjectConfirmation_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAssertionValidator_SubjectConfirmation_Test.java
@@ -199,4 +199,48 @@ class SamlAssertionValidator_SubjectConfirmation_Test extends TestBase {
var xml = signedWithSubject(cred, SamlTestSupport.subjectWithConfirmations("alice", bad, good));
assertEquals("alice", base(cred).recipient(ACS).build().validate(xml).getName());
}
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // validate(xml, recipient) overload — used by SamlAuthFilter to bind the actual per-request ACS URL
+ // -----------------------------------------------------------------------------------------------------------------
+
+ @Test void f01_explicitRecipientOverload_matchingRecipient_accepted() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var sc = SamlTestSupport.bearerConfirmation(ACS, null, NOA, null, null);
+ var xml = signedWithSubject(cred, SamlTestSupport.subjectWithConfirmations("alice", sc));
+ // No recipient(...) configured on the builder — the two-arg overload must still enforce it.
+ var v = base(cred).build();
+ assertEquals("alice", v.validate(xml, ACS).getName());
+ }
+
+ @Test void f02_explicitRecipientOverload_mismatchedRecipient_rejected() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var sc = SamlTestSupport.bearerConfirmation("https://evil.example.com/acs", null, NOA, null, null);
+ var xml = signedWithSubject(cred, SamlTestSupport.subjectWithConfirmations("alice", sc));
+ var v = base(cred).build();
+ assertThrows(AuthenticationException.class, () -> v.validate(xml, ACS));
+ }
+
+ @Test void f03_explicitRecipientOverload_overridesConfiguredBuilderRecipient() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var sc = SamlTestSupport.bearerConfirmation(ACS, null, NOA, null, null);
+ var xml = signedWithSubject(cred, SamlTestSupport.subjectWithConfirmations("alice", sc));
+ // Builder was configured with a DIFFERENT recipient; the per-call argument wins.
+ var v = base(cred).recipient("https://other-sp.example.com/saml/acs").build();
+ assertEquals("alice", v.validate(xml, ACS).getName());
+ }
+
+ @Test void f04_explicitRecipientOverload_nullRecipient_throws() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = SamlTestSupport.buildSignedResponse(cred, ISSUER, AUDIENCE, "alice", NBF, NOA, Map.of());
+ var v = base(cred).build();
+ assertThrows(IllegalArgumentException.class, () -> v.validate(xml, null));
+ }
+
+ @Test void f05_explicitRecipientOverload_blankRecipient_throws() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = SamlTestSupport.buildSignedResponse(cred, ISSUER, AUDIENCE, "alice", NBF, NOA, Map.of());
+ var v = base(cred).build();
+ assertThrows(IllegalArgumentException.class, () -> v.validate(xml, " "));
+ }
}
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_MaxInflate_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_MaxInflate_Test.java
index 0f1735069d..fa1cf346f6 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_MaxInflate_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_MaxInflate_Test.java
@@ -51,7 +51,7 @@ class SamlAuthFilter_MaxInflate_Test extends TestBase {
.spEntityId("https://sp.example.com")
.expectedIssuer("https://idp.example.com")
.signingCredential(new BasicCredential(pair.getPublic(), pair.getPrivate()))) {
- @Override public Principal validate(String xml) {
+ @Override public Principal validate(String xml, String recipient) {
return impl.apply(xml);
}
};
@@ -62,6 +62,7 @@ class SamlAuthFilter_MaxInflate_Test extends TestBase {
when(r.getPathInfo()).thenReturn("/saml/acs");
when(r.getServletPath()).thenReturn("/saml/acs");
when(r.getParameter("SAMLResponse")).thenReturn(samlResponse);
+ when(r.getRequestURL()).thenReturn(new StringBuffer("https://sp.example.com/saml/acs"));
return r;
}
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_RecipientBinding_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_RecipientBinding_Test.java
new file mode 100644
index 0000000000..708cfea661
--- /dev/null
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_RecipientBinding_Test.java
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.juneau.rest.server.auth.saml;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import java.nio.charset.*;
+import java.time.*;
+import java.util.*;
+
+import org.apache.juneau.*;
+import org.apache.juneau.rest.server.auth.*;
+import org.junit.jupiter.api.*;
+import org.opensaml.security.credential.*;
+
+import jakarta.servlet.http.*;
+
+/**
+ * Filter-level tests proving {@link SamlAuthFilter} binds every assertion to the actual ACS URL of the
+ * current request — regardless of whether the {@link SamlAssertionValidator} it wraps was itself built
+ * with {@link SamlAssertionValidator.Builder#recipient(String) recipient(...)} configured.
+ *
+ * @since 10.0.0
+ */
+class SamlAuthFilter_RecipientBinding_Test extends TestBase {
+
+ private static final String ISSUER = "https://idp.example.com";
+ private static final String AUDIENCE = "https://sp.example.com";
+ private static final String ACS = "https://sp.example.com/saml/acs";
+ private static final Instant NOW = Instant.parse("2026-01-01T00:00:00Z");
+ private static final Clock CLOCK = Clock.fixed(NOW, ZoneOffset.UTC);
+ private static final Instant NBF = NOW.minusSeconds(60);
+ private static final Instant NOA = NOW.plusSeconds(300);
+
+ private static SamlAssertionValidator.Builder base(Credential cred) {
+ return SamlAssertionValidator.create()
+ .spEntityId(AUDIENCE)
+ .expectedIssuer(ISSUER)
+ .signingCredential(cred)
+ .clock(CLOCK);
+ }
+
+ /** Builds a signed response whose bearer confirmation names {@code confirmedRecipient} as its Recipient. */
+ private static String signedResponseWithRecipient(Credential cred, String confirmedRecipient) throws Exception {
+ var sc = SamlTestSupport.bearerConfirmation(confirmedRecipient, null, NOA, null, null);
+ var sub = SamlTestSupport.subjectWithConfirmations("alice", sc);
+ var assertion = SamlTestSupport.buildMinimalAssertionWithSubject(ISSUER, AUDIENCE, sub, NBF, NOA);
+ return SamlTestSupport.signAndBuildResponse((BasicCredential) cred, ISSUER, assertion);
+ }
+
+ /** Mocks a request that was actually delivered to {@code deliveredToAcs}, carrying {@code responseXml}. */
+ private static HttpServletRequest req(String deliveredToAcs, String responseXml) {
+ var r = mock(HttpServletRequest.class);
+ when(r.getPathInfo()).thenReturn("/saml/acs");
+ when(r.getServletPath()).thenReturn("/saml/acs");
+ when(r.getParameter("SAMLResponse")).thenReturn(
+ Base64.getEncoder().encodeToString(responseXml.getBytes(StandardCharsets.UTF_8)));
+ when(r.getRequestURL()).thenReturn(new StringBuffer(deliveredToAcs));
+ return r;
+ }
+
+ @Test void a01_matchingAcsRecipient_accepted() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, ACS);
+ // No recipient(...) configured on the validator — the filter must derive and bind it anyway.
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var result = f.authenticate(req(ACS, xml));
+ assertTrue(result.isPresent());
+ assertEquals("alice", result.get().getPrincipal().getName());
+ }
+
+ @Test void a02_differentAcsRecipient_rejected() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ // The assertion's bearer confirmation names a different ACS than the one this request actually hit.
+ var xml = signedResponseWithRecipient(cred, "https://other-sp.example.com/saml/acs");
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var ex = assertThrows(AuthenticationException.class, () -> f.authenticate(req(ACS, xml)));
+ assertTrue(ex.getHeaders().stream().anyMatch(h -> "WWW-Authenticate".equalsIgnoreCase(h.getName())));
+ }
+
+ @Test void a03_publishedExampleRecipe_stillBindsRecipientOnFilterPath() throws Exception {
+ // Mirrors SamlAuthFilter's javadoc "Usage" example exactly: validator built with no recipient(...).
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, "https://other-sp.example.com/saml/acs");
+ var validator = SamlAssertionValidator.create()
+ .spEntityId(AUDIENCE)
+ .expectedIssuer(ISSUER)
+ .signingCredential(cred)
+ .clock(CLOCK)
+ .build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ assertThrows(AuthenticationException.class, () -> f.authenticate(req(ACS, xml)));
+ }
+
+ @Test void a04_standaloneValidator_recipientOptedInDirectly_unaffectedByFilterOverride() throws Exception {
+ // Sanity check: calling the validator directly (not through the filter) with recipient(...) configured
+ // still enforces the builder-level recipient, independent of the filter's per-call override.
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, ACS);
+ var validator = base(cred).recipient(ACS).build();
+ assertEquals("alice", validator.validate(xml).getName());
+ }
+
+ // -----------------------------------------------------------------------------------------------------------------
+ // B: getRequestURL() returns null — the recipient is reconstructed from scheme/host/port/path, or the
+ // request is rejected via AuthenticationException (never an NPE) when reconstruction is impossible.
+ // -----------------------------------------------------------------------------------------------------------------
+
+ /** Mocks a request with a null getRequestURL(), delivered to the given scheme/host/port/uri. */
+ private static HttpServletRequest reqNullRequestUrl(String scheme, String host, int port, String uri, String responseXml) {
+ var r = mock(HttpServletRequest.class);
+ when(r.getPathInfo()).thenReturn("/saml/acs");
+ when(r.getServletPath()).thenReturn("/saml/acs");
+ when(r.getParameter("SAMLResponse")).thenReturn(
+ Base64.getEncoder().encodeToString(responseXml.getBytes(StandardCharsets.UTF_8)));
+ when(r.getRequestURL()).thenReturn(null);
+ when(r.getScheme()).thenReturn(scheme);
+ when(r.getServerName()).thenReturn(host);
+ when(r.getServerPort()).thenReturn(port);
+ when(r.getRequestURI()).thenReturn(uri);
+ return r;
+ }
+
+ @Test void b01_nullRequestUrl_reconstructedAtDefaultPort_acceptsMatchingRecipient() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, ACS);
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var result = f.authenticate(reqNullRequestUrl("https", "sp.example.com", 443, "/saml/acs", xml));
+ assertTrue(result.isPresent());
+ assertEquals("alice", result.get().getPrincipal().getName());
+ }
+
+ @Test void b02_nullRequestUrl_reconstructedAtNonDefaultPort_includesPortInRecipient() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, "https://sp.example.com:8443/saml/acs");
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var result = f.authenticate(reqNullRequestUrl("https", "sp.example.com", 8443, "/saml/acs", xml));
+ assertTrue(result.isPresent());
+ }
+
+ @Test void b03_nullRequestUrl_reconstructedRecipientMismatch_rejected() throws Exception {
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ // The assertion's bearer confirmation names a different ACS than the one this request actually hit.
+ var xml = signedResponseWithRecipient(cred, "https://other-sp.example.com/saml/acs");
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var ex = assertThrows(AuthenticationException.class,
+ () -> f.authenticate(reqNullRequestUrl("https", "sp.example.com", 443, "/saml/acs", xml)));
+ assertTrue(ex.getHeaders().stream().anyMatch(h -> "WWW-Authenticate".equalsIgnoreCase(h.getName())));
+ }
+
+ @Test void b04_nullRequestUrl_andNullScheme_failsClosedNotNpe() throws Exception {
+ // Neither getRequestURL() nor the scheme/host/uri fallback getters are usable — must fail closed with
+ // an AuthenticationException, never propagate an NPE.
+ var cred = SamlTestSupport.credential(SamlTestSupport.generateRsaKeyPair());
+ var xml = signedResponseWithRecipient(cred, ACS);
+ var validator = base(cred).build();
+ var f = SamlAuthFilter.create().validator(validator).build();
+ var ex = assertThrows(AuthenticationException.class,
+ () -> f.authenticate(reqNullRequestUrl(null, null, 0, null, xml)));
+ assertTrue(ex.getHeaders().stream().anyMatch(h -> "WWW-Authenticate".equalsIgnoreCase(h.getName())));
+ }
+}
diff --git a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
index 2d5fd09a99..904efb5a1d 100644
--- a/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
+++ b/juneau-rest/juneau-rest-server-auth-saml/src/test/java/org/apache/juneau/rest/server/auth/saml/SamlAuthFilter_Test.java
@@ -51,7 +51,7 @@ class SamlAuthFilter_Test extends TestBase {
.spEntityId("https://sp.example.com")
.expectedIssuer("https://idp.example.com")
.signingCredential(new BasicCredential(pair.getPublic(), pair.getPrivate()))) {
- @Override public Principal validate(String xml) {
+ @Override public Principal validate(String xml, String recipient) {
return impl.apply(xml);
}
};
@@ -62,6 +62,7 @@ class SamlAuthFilter_Test extends TestBase {
when(r.getPathInfo()).thenReturn(path);
when(r.getServletPath()).thenReturn(path);
when(r.getParameter("SAMLResponse")).thenReturn(samlResponse);
+ when(r.getRequestURL()).thenReturn(new StringBuffer("https://sp.example.com" + path));
return r;
}
@@ -144,6 +145,7 @@ class SamlAuthFilter_Test extends TestBase {
when(r.getServletPath()).thenReturn("/saml/acs");
var b64 = Base64.getEncoder().encodeToString("<x/>".getBytes(StandardCharsets.UTF_8));
when(r.getParameter("SAMLResponse")).thenReturn(b64);
+ when(r.getRequestURL()).thenReturn(new StringBuffer("https://sp.example.com/saml/acs"));
var cp = new ClaimsPrincipal("bob", Map.of());
var f = SamlAuthFilter.create().validator(validator(x -> cp)).build();
assertTrue(f.authenticate(r).isPresent());
(juneau) 04/08: READY-363: Fix RestContext annotationWork init-ordering NPE in framework-bean memoizers
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit e3adb833a70ce3ba85f24a7eb59e842611b8421a
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:32:57 2026 -0400
READY-363: Fix RestContext annotationWork init-ordering NPE in framework-bean memoizers
---
.../org/apache/juneau/rest/server/RestContext.java | 24 ++++++++++++++++------
.../rest/server/RestContext_Construction_Test.java | 18 ++++++++++++++++
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/RestContext.java b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/RestContext.java
index 96fae5558c..9c253f5843 100644
--- a/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/RestContext.java
+++ b/juneau-rest/juneau-rest-server/src/main/java/org/apache/juneau/rest/server/RestContext.java
@@ -2537,6 +2537,20 @@ public class RestContext extends Context {
// dependencies through the bean store.
registerFrameworkDefaults(beanStore);
+ var rci2 = ClassInfo.of(resourceClass);
+
+ // Build the annotation work list as early as possible in construction — immediately after
+ // registerFrameworkDefaults() registers the framework-bean default suppliers above, and
+ // before any step below (the @Bean field/method scan, @RestInit hooks, or the @Bean field
+ // back-fill) that can trigger a beanStore lookup and force-evaluate a creator/builder memoizer
+ // that reads annotationWork directly (partSerializerCreator, partParserCreator,
+ // jsonSchemaGeneratorBuilder, beanContextBuilder). Building it this late used to leave those
+ // memoizers exposed to an NPE whenever something forced them early — e.g. a @Bean static
+ // witness field of type HttpPartSerializer, or an @RestInit method parameter of one of the
+ // affected framework types — since annotationWork was still null at that point.
+ var vrs = getBootstrapVarResolver().createSession();
+ annotationWork = AnnotationWorkList.of(vrs, rstream(AnnotationProvider.INSTANCE.find(rci2)).filter(CONTEXT_APPLY_FILTER));
+
// For mixin sub-contexts, the bean store is parent-linked to the host's full beanStore so that
// host-declared @Bean factory results (e.g. @Bean(name="db") HealthIndicator dbIndicator()) are
// visible through the mixin's lookup chain. But the parent walk also picks up the host's
@@ -2560,8 +2574,6 @@ public class RestContext extends Context {
// tier-4 default / parent-wins semantics, so a Spring/parent-supplied bean still wins for them.
beanStore.addBean(RestContextProperties.class, getRestContextProperties());
- var rci2 = ClassInfo.of(resourceClass);
-
// Register @Bean fields that already have a value.
// @formatter:off
rci2.getAllFields().stream()
@@ -2682,10 +2694,10 @@ public class RestContext extends Context {
// Config injection visible to SVL without firing the full runtime VarResolver memoizer.
paths = resolveMountPaths(builder, resource.get(), getBootstrapVarResolver(), beanStore, getRestAnnotations());
- // Build annotation work list, then trigger beanContextBuilder (which applies it).
- var vrs = getBootstrapVarResolver().createSession();
- annotationWork = AnnotationWorkList.of(vrs, rstream(AnnotationProvider.INSTANCE.find(rci2)).filter(CONTEXT_APPLY_FILTER));
- beanContextBuilder.get(); // force init with annotationWork now set
+ // annotationWork was already built above (right after registerFrameworkDefaults(), well before
+ // any step that could force-evaluate a memoizer reading it) — force-init beanContextBuilder
+ // here so annotation work is applied before the builder is used by any other memoizer.
+ beanContextBuilder.get();
// @formatter:off
beanStore
diff --git a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/RestContext_Construction_Test.java b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/RestContext_Construction_Test.java
index 7874ca984a..dd7631449c 100644
--- a/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/RestContext_Construction_Test.java
+++ b/juneau-rest/juneau-rest-server/src/test/java/org/apache/juneau/rest/server/RestContext_Construction_Test.java
@@ -24,6 +24,7 @@ import java.util.logging.*;
import org.apache.juneau.commons.inject.*;
import org.apache.juneau.commons.svl.*;
+import org.apache.juneau.marshall.httppart.*;
import org.apache.juneau.marshall.oapi.*;
import org.apache.juneau.rest.server.metrics.*;
import org.apache.juneau.rest.server.openapi.*;
@@ -311,4 +312,21 @@ class RestContext_Construction_Test extends org.apache.juneau.TestBase {
assertEquals(Fix_Bare.class.getName(), ctx.getLogger().getName());
assertNotNull(Level.INFO);
}
+
+ //-----------------------------------------------------------------------------------------------------------
+ // m - a @Bean static witness field forces a framework-bean creator memoizer (partSerializerCreator) during
+ // the @Bean field back-fill step, well before annotationWork used to be assigned in the constructor
+ //-----------------------------------------------------------------------------------------------------------
+
+ @Rest
+ static class Fix_PartSerializerWitness {
+ @Bean static HttpPartSerializer partSerializerCapture;
+ }
+
+ @Test void m01_beanStaticWitnessField_forcesPartSerializerCreator_duringBackfill_doesNotNpe() throws Exception {
+ Fix_PartSerializerWitness.partSerializerCapture = null;
+ var ctx = new RestContext(argsOf(Fix_PartSerializerWitness.class, Fix_PartSerializerWitness::new));
+ assertNotNull(Fix_PartSerializerWitness.partSerializerCapture);
+ assertSame(ctx.getPartSerializer(), Fix_PartSerializerWitness.partSerializerCapture);
+ }
}
(juneau) 02/08: READY-387: Enforce declared document-length bounds in the BSON parser
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 18e68d819d699d3cd950784cbf25297cd470113d
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:32:36 2026 -0400
READY-387: Enforce declared document-length bounds in the BSON parser
---
.../juneau/marshall/bson/BsonInputStream.java | 82 ++++++++++++++++++++--
.../juneau/marshall/bson/BsonInputStream_Test.java | 68 ++++++++++++++++++
.../marshall/bson/BsonParserSession_Test.java | 5 +-
3 files changed, 146 insertions(+), 9 deletions(-)
diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/bson/BsonInputStream.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/bson/BsonInputStream.java
index ea8e698132..e6e5aac726 100644
--- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/bson/BsonInputStream.java
+++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/bson/BsonInputStream.java
@@ -21,6 +21,7 @@ import static org.apache.juneau.commons.utils.Shorts.*;
import java.io.*;
import java.math.*;
import java.nio.charset.*;
+import java.util.*;
import org.apache.juneau.marshall.parser.*;
@@ -49,6 +50,12 @@ public class BsonInputStream extends ParserInputStream {
private int pushback = -1;
+ // Stack of remaining-byte counts, one per currently-open document/array (innermost on top). Every
+ // physical byte pulled off the wire decrements *all* active levels, since bytes consumed by a nested
+ // document/array also count against its enclosing document's declared length. This is what stops a
+ // forged-short document length plus an unterminated cstring from growing past the declared bound.
+ private final Deque<int[]> bounds = new ArrayDeque<>();
+
/**
* Constructor.
*
@@ -66,7 +73,11 @@ public class BsonInputStream extends ParserInputStream {
pushback = -1;
return b;
}
- return super.read();
+ checkBoundAvailable();
+ var b = super.read();
+ if (b >= 0)
+ decrementBounds();
+ return b;
}
/**
@@ -76,6 +87,48 @@ public class BsonInputStream extends ParserInputStream {
pushback = b & 0xFF;
}
+ /**
+ * Verifies every currently-open document/array bound still has at least one byte of budget left,
+ * before a further physical byte is pulled off the wire.
+ */
+ private void checkBoundAvailable() throws IOException {
+ for (var bound : bounds)
+ if (bound[0] <= 0)
+ throw ioex("BSON document declared length exceeded");
+ }
+
+ /**
+ * Charges one byte against every currently-open document/array bound.
+ */
+ private void decrementBounds() {
+ for (var bound : bounds)
+ bound[0]--;
+ }
+
+ /**
+ * Opens a length-bounded scope for a document/array body being entered, so cstrings and nested values
+ * cannot be read past its declared length regardless of what the enclosing bound(s) still allow.
+ *
+ * @param remaining The number of bytes remaining in the body (declared size minus the 4-byte length
+ * header already consumed).
+ */
+ private void pushDocumentBound(int remaining) {
+ bounds.push(new int[]{remaining});
+ }
+
+ /**
+ * Closes the innermost document/array bound after its terminator has been consumed.
+ *
+ * @throws IOException If the declared length was not fully consumed (unconsumed trailing bytes).
+ */
+ private void popDocumentBound() throws IOException {
+ if (bounds.isEmpty())
+ return;
+ var remaining = bounds.pop()[0];
+ if (remaining != 0)
+ throw ioex("BSON document declared length not fully consumed (%s byte(s) remaining)", remaining);
+ }
+
/**
* Reads a 4-byte little-endian integer.
*
@@ -105,13 +158,17 @@ public class BsonInputStream extends ParserInputStream {
}
/**
- * Reads the document size (int32) from the stream.
+ * Reads the document size (int32) from the stream and opens a length-bounded scope for the document
+ * body that follows, so a forged-short declared length cannot be bypassed by traversal that otherwise
+ * relies solely on finding a {@code 0x00} terminator.
*
* @return The document size in bytes.
* @throws IOException If the stream ends prematurely.
*/
public int readDocumentSize() throws IOException {
- return checkLength(readLE4(), "document");
+ var size = checkLength(readLE4(), "document");
+ pushDocumentBound(size - 4);
+ return size;
}
/**
@@ -140,14 +197,21 @@ public class BsonInputStream extends ParserInputStream {
/**
* Reads a cstring (UTF-8 bytes until 0x00).
*
+ * <p>
+ * Growth is capped by whichever is tighter of the innermost open document/array bound (enforced by
+ * {@link #read()}) or the configured {@link #setMaxLength(int) maxLength}, so an unterminated cstring
+ * cannot grow the backing buffer without limit.
+ *
* @return The string value.
- * @throws IOException If the stream ends prematurely.
+ * @throws IOException If the stream ends prematurely or the cstring exceeds the configured maximum length.
*/
public String readCString() throws IOException {
var baos = new ByteArrayOutputStream();
int b;
- while ((b = read()) >= 0 && b != 0)
+ while ((b = read()) >= 0 && b != 0) {
+ checkLength(baos.size() + 1, "cstring");
baos.write(b);
+ }
if (b < 0)
throw ioex(UNEXPECTED_END_OF_BSON_STREAM);
return new String(baos.toByteArray(), UTF8);
@@ -167,14 +231,18 @@ public class BsonInputStream extends ParserInputStream {
}
/**
- * Consumes the document terminator byte (0x00).
+ * Consumes the document terminator byte (0x00) and closes the length-bounded scope opened by the
+ * matching {@link #readDocumentSize()}, rejecting trailing bytes left unconsumed within the declared
+ * length.
*
- * @throws IOException If the stream ends prematurely or byte is not 0x00.
+ * @throws IOException If the stream ends prematurely, the byte is not 0x00, or the declared document
+ * length was not fully consumed.
*/
public void readDocumentTerminator() throws IOException {
var b = read();
if (b != 0x00)
throw ioex("Expected document terminator, got %s", b);
+ popDocumentBound();
}
/**
diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonInputStream_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonInputStream_Test.java
index d757cdd62b..2609eb398e 100644
--- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonInputStream_Test.java
+++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonInputStream_Test.java
@@ -366,6 +366,74 @@ class BsonInputStream_Test extends TestBase {
}
}
+ // ====================================================================
+ // Document/array declared-length bound enforcement (forged-header protection; see b05_readDocumentSize)
+ // ====================================================================
+
+ @Test
+ void b31_readDocumentSize_forgedShortBoundRejectsUnterminatedCString() throws Exception {
+ // Forged-short document length (5 = 4-byte header + 1 remaining body byte) followed by a long run
+ // of non-null bytes simulating an attacker-controlled unterminated cstring. Before the fix, traversal
+ // relied solely on finding 0x00 and would grow the buffer across the entire stream.
+ var unterminated = new byte[10_000];
+ Arrays.fill(unterminated, (byte) 'a');
+ try (var is = openIs(cat(le4(5), unterminated))) {
+ is.readDocumentSize();
+ var ex = assertThrows(IOException.class, is::readCString);
+ assertTrue(ex.getMessage().contains("declared length exceeded"));
+ }
+ }
+
+ @Test
+ void b32_readDocumentSize_wellFormedDocumentBoundedReadSucceeds() throws Exception {
+ // Declared size exactly matches the payload: bound enforcement must not reject the well-formed case.
+ var body = cat(cstring("ok"), new byte[]{0x00});
+ var doc = cat(le4(body.length + 4), body);
+ try (var is = openIs(doc)) {
+ is.readDocumentSize();
+ assertEquals("ok", is.readCString());
+ is.readDocumentTerminator();
+ }
+ }
+
+ @Test
+ void b33_readDocumentSize_nestedDocumentHonorsOwnBound() throws Exception {
+ // Outer document has ample remaining budget; a nested document's own forged-short declared length
+ // must still bound its cstring reads independently of (and tighter than) the outer bound.
+ var unterminated = new byte[1_000];
+ Arrays.fill(unterminated, (byte) 'x');
+ var outerBody = cat(le4(5), unterminated); // nested doc header (size=5) + long unterminated payload
+ var outer = cat(le4(outerBody.length + 4), outerBody);
+ try (var is = openIs(outer)) {
+ is.readDocumentSize(); // outer bound: ~1000 bytes remaining
+ is.readDocumentSize(); // nested bound: 1 byte remaining
+ var ex = assertThrows(IOException.class, is::readCString);
+ assertTrue(ex.getMessage().contains("declared length exceeded"));
+ }
+ }
+
+ @Test
+ void b34_readDocumentTerminator_trailingUnconsumedBytesRejected() throws Exception {
+ // Declared size promises more body bytes than are actually consumed before the terminator arrives.
+ var bytes = cat(le4(20), new byte[]{0x00}); // declares 16 body bytes, but terminator arrives immediately
+ try (var is = openIs(bytes)) {
+ is.readDocumentSize();
+ assertTrue(is.isDocumentEnd());
+ var ex = assertThrows(IOException.class, is::readDocumentTerminator);
+ assertTrue(ex.getMessage().contains("not fully consumed"));
+ }
+ }
+
+ @Test
+ void b35_readDocumentSize_eofBeforeDeclaredSizeConsumedRejected() throws Exception {
+ // Stream ends before the declared document length is fully consumed.
+ var bytes = le4(100); // declares 96 body bytes, but the stream provides none
+ try (var is = openIs(bytes)) {
+ is.readDocumentSize();
+ assertThrows(IOException.class, is::readElementType);
+ }
+ }
+
// ====================================================================
// Pushback mechanism (read after pushback)
// ====================================================================
diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonParserSession_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonParserSession_Test.java
index 3765d87480..8003be7511 100644
--- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonParserSession_Test.java
+++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/bson/BsonParserSession_Test.java
@@ -712,8 +712,9 @@ class BsonParserSession_Test extends TestBase {
var nameValue = cat(le4("n".getBytes(StandardCharsets.UTF_8).length + 1), cstring("n"));
var body = cat(
new byte[]{0x0A}, cstring("_type"),
- new byte[]{0x02}, cstring("name"), nameValue);
- var bytes = cat(le4(body.length + 4), body, new byte[]{0x00});
+ new byte[]{0x02}, cstring("name"), nameValue,
+ new byte[]{0x00});
+ var bytes = cat(le4(body.length + 4), body);
var result = BsonParser.DEFAULT.read(bytes, G02_Bean.class);
assertNotNull(result);
assertEquals("n", result.name);
(juneau) 01/08: READY-393: Reject CR/LF and control chars in multipart part name/filename/content-type
Posted by ja...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 4c63c6acb980b8ffc256df349bfcf01e4b99ea86
Author: James Bognar <ja...@apache.org>
AuthorDate: Sun Aug 16 15:32:26 2026 -0400
READY-393: Reject CR/LF and control chars in multipart part name/filename/content-type
---
.../apache/juneau/http/entity/MultipartBody.java | 67 ++++++++++++++++++++--
.../apache/juneau/http/entity/HttpBodies_Test.java | 53 +++++++++++++++++
2 files changed, 116 insertions(+), 4 deletions(-)
diff --git a/juneau-rest/juneau-rest-common/src/main/java/org/apache/juneau/http/entity/MultipartBody.java b/juneau-rest/juneau-rest-common/src/main/java/org/apache/juneau/http/entity/MultipartBody.java
index d5694fa3dd..b0d089048a 100644
--- a/juneau-rest/juneau-rest-common/src/main/java/org/apache/juneau/http/entity/MultipartBody.java
+++ b/juneau-rest/juneau-rest-common/src/main/java/org/apache/juneau/http/entity/MultipartBody.java
@@ -67,6 +67,54 @@ public final class MultipartBody implements HttpBody {
private final String boundary;
private final List<MultipartPart> parts;
+ /**
+ * Rejects a part-metadata value (name, filename, or content-type) that could inject an extra MIME header line
+ * or otherwise corrupt the multipart framing at the receiving service.
+ *
+ * @param fieldName The name of the field being validated, for the exception message.
+ * @param value The value to validate. Can be <jk>null</jk> (no-op).
+ * @throws IllegalArgumentException If {@code value} contains CR, LF, another C0 control, NEL, or a bidi control.
+ */
+ private static void assertNoInjectionChars(String fieldName, String value) {
+ if (value == null)
+ return;
+ for (var i = 0; i < value.length(); i++)
+ if (isInjectionChar(value.charAt(i)))
+ throw iaex("MultipartPart '%s' must not contain CR, LF, or other control characters.", fieldName);
+ }
+
+ private static boolean isInjectionChar(char c) {
+ if (c == '\t')
+ return false;
+ if (c <= '\u001F') // C0 controls (CR/LF included).
+ return true;
+ if (c >= '\u007F' && c <= '\u009F') // DEL + C1 controls (includes NEL \u0085).
+ return true;
+ if (c == '\u2028' || c == '\u2029') // Line / paragraph separators.
+ return true;
+ if (c >= '\u202A' && c <= '\u202E') // Bidi embedding/override controls.
+ return true;
+ return c >= '\u2066' && c <= '\u2069'; // Bidi isolate controls.
+ }
+
+ /**
+ * Escapes {@code \} and {@code "} in a {@code Content-Disposition} quoted-string parameter value so it cannot
+ * break out of the surrounding quotes.
+ *
+ * @param value The raw value to escape. Must not be <jk>null</jk>.
+ * @return The escaped value, unquoted (caller adds the surrounding {@code "..."}).
+ */
+ private static String escapeQuoted(String value) {
+ var sb = new StringBuilder(value.length() + 4);
+ for (var i = 0; i < value.length(); i++) {
+ var c = value.charAt(i);
+ if (c == '\\' || c == '"')
+ sb.append('\\');
+ sb.append(c);
+ }
+ return sb.toString();
+ }
+
private MultipartBody(String boundary, List<MultipartPart> parts) {
this.boundary = boundary;
this.parts = List.copyOf(parts);
@@ -94,13 +142,18 @@ public final class MultipartBody implements HttpBody {
public void writeTo(OutputStream out) throws IOException {
var bnd = boundary.getBytes(StandardCharsets.US_ASCII);
for (var part : parts) {
+ // Belt-and-suspenders: MultipartPart's compact constructor already rejects these, but re-validate at
+ // write time in case a part was ever reachable without going through that constructor.
+ assertNoInjectionChars("name", part.name());
+ assertNoInjectionChars("filename", part.filename());
+ assertNoInjectionChars("contentType", part.contentType());
out.write(DASHDASH);
out.write(bnd);
out.write(CRLF);
// Content-Disposition header
- var cd = new StringBuilder("Content-Disposition: form-data; name=\"").append(part.name()).append('"');
+ var cd = new StringBuilder("Content-Disposition: form-data; name=\"").append(escapeQuoted(part.name())).append('"');
if (part.filename() != null)
- cd.append("; filename=\"").append(part.filename()).append('"');
+ cd.append("; filename=\"").append(escapeQuoted(part.filename())).append('"');
out.write(cd.toString().getBytes(StandardCharsets.UTF_8));
out.write(CRLF);
// Content-Type header (if present)
@@ -157,11 +210,16 @@ public final class MultipartBody implements HttpBody {
public record MultipartPart(String name, String filename, String contentType, HttpBody body) {
/**
- * Compact constructor enforcing the documented non-null contract on {@code name} and {@code body}.
+ * Compact constructor enforcing the documented non-null contract on {@code name} and {@code body}, and
+ * rejecting CR/LF and other control characters in {@code name}, {@code filename}, and {@code contentType}
+ * that could otherwise inject an extra MIME header line into the multipart wire format.
*/
public MultipartPart {
assertArgNotNull("name", name);
assertArgNotNull("body", body);
+ assertNoInjectionChars("name", name);
+ assertNoInjectionChars("filename", filename);
+ assertNoInjectionChars("contentType", contentType);
}
/**
@@ -179,7 +237,8 @@ public final class MultipartBody implements HttpBody {
* Creates a file upload part.
*
* @param name The field name. Must not be <jk>null</jk>.
- * @param file The file to upload. Must not be <jk>null</jk>.
+ * @param file The file to upload. Must not be <jk>null</jk>. Its {@link File#getName()} is validated like any
+ * other {@code filename} — a hostile name containing CR/LF or other control characters is rejected.
* @param contentType The MIME content type for the file (e.g. {@code "application/pdf"}). May be <jk>null</jk>.
* @return A new instance. Never <jk>null</jk>.
*/
diff --git a/juneau-rest/juneau-rest-common/src/test/java/org/apache/juneau/http/entity/HttpBodies_Test.java b/juneau-rest/juneau-rest-common/src/test/java/org/apache/juneau/http/entity/HttpBodies_Test.java
index 554ce1dd8b..09842600fd 100644
--- a/juneau-rest/juneau-rest-common/src/test/java/org/apache/juneau/http/entity/HttpBodies_Test.java
+++ b/juneau-rest/juneau-rest-common/src/test/java/org/apache/juneau/http/entity/HttpBodies_Test.java
@@ -16,6 +16,7 @@
*/
package org.apache.juneau.http.entity;
+import static org.apache.juneau.BasicTestUtils.*;
import static org.junit.jupiter.api.Assertions.*;
import java.io.*;
@@ -208,4 +209,56 @@ class HttpBodies_Test extends TestBase {
assertEquals("missing.txt", p2.filename());
assertEquals("text/plain", p2.contentType());
}
+
+ @Test void f04_multipartPart_nameCrLfThrows() {
+ assertThrowsWithMessage(IllegalArgumentException.class, "MultipartPart 'name' must not contain CR, LF, or other control characters.",
+ () -> MultipartBody.MultipartPart.of("a\r\nX-Injected: true", null, null, StringBody.of("x")));
+ }
+
+ @Test void f05_multipartPart_filenameCrLfThrows() {
+ assertThrowsWithMessage(IllegalArgumentException.class, "MultipartPart 'filename' must not contain CR, LF, or other control characters.",
+ () -> MultipartBody.MultipartPart.of("a", "evil.txt\r\nX-Injected: true", null, StringBody.of("x")));
+ }
+
+ @Test void f06_multipartPart_contentTypeCrLfThrows() {
+ assertThrowsWithMessage(IllegalArgumentException.class, "MultipartPart 'contentType' must not contain CR, LF, or other control characters.",
+ () -> MultipartBody.MultipartPart.of("a", null, "text/plain\r\nX-Injected: true", StringBody.of("x")));
+ }
+
+ @Test void f07_multipartPart_otherC0ControlThrows() {
+ assertThrowsWithMessage(IllegalArgumentException.class, "MultipartPart 'name' must not contain CR, LF, or other control characters.",
+ () -> MultipartBody.MultipartPart.of("a\u0000b", null, null, StringBody.of("x")));
+ }
+
+ @Test void f08_multipartPart_tabInNameAllowed() {
+ var p = MultipartBody.MultipartPart.of("a\tb", null, null, StringBody.of("x"));
+ assertEquals("a\tb", p.name());
+ }
+
+ // Hostile File whose getName() returns attacker-controlled text with embedded CR/LF.
+ static class F09_HostileFile extends File {
+ F09_HostileFile() {
+ super("innocuous.txt");
+ }
+
+ @Override public String getName() {
+ return "evil.txt\r\nX-Injected: true";
+ }
+ }
+
+ @Test void f09_multipartPart_hostileFileNameThrows() {
+ assertThrowsWithMessage(IllegalArgumentException.class, "MultipartPart 'filename' must not contain CR, LF, or other control characters.",
+ () -> MultipartBody.MultipartPart.file("upload", new F09_HostileFile(), "text/plain"));
+ }
+
+ @Test void f10_multipartBody_writeTo_escapesQuotesAndBackslashesInFilename() throws Exception {
+ var body = MultipartBody.builder()
+ .boundary("b")
+ .part(MultipartBody.MultipartPart.of("upload", "a\"b\\c.txt", "text/plain", StringBody.of("x")))
+ .build();
+ var out = new ByteArrayOutputStream();
+ body.writeTo(out);
+ var wire = out.toString();
+ assertTrue(wire.contains("filename=\"a\\\"b\\\\c.txt\""), "filename must be escaped, not interpolated raw: " + wire);
+ }
}