This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 4d829c560f Use RequestDispatcher target rather than source for
security-role-refs
4d829c560f is described below
commit 4d829c560fa0945c93c8b63da83849879a332962
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Oct 6 14:49:02 2026 +0100
Use RequestDispatcher target rather than source for security-role-refs
# Conflicts:
# webapps/docs/changelog.xml
# Conflicts:
# webapps/docs/changelog.xml
# Conflicts:
# webapps/docs/changelog.xml
# Conflicts:
# webapps/docs/changelog.xml
# Conflicts:
# webapps/docs/changelog.xml
---
java/org/apache/catalina/connector/Request.java | 27 ++++++-
.../apache/catalina/connector/RequestFacade.java | 13 ++++
.../catalina/core/ApplicationDispatcher.java | 2 +-
.../catalina/core/ApplicationHttpRequest.java | 83 +++++++++++++++++++++-
java/org/apache/catalina/mapper/MappingData.java | 6 ++
.../catalina/core/TestApplicationHttpRequest.java | 2 +-
.../TesterApplicationHttpRequestPerformance.java | 4 +-
webapps/docs/changelog.xml | 4 ++
8 files changed, 131 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index 12568b2d8d..422bc6157a 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -2254,8 +2254,29 @@ public class Request implements HttpServletRequest {
return false;
}
- // Identify the Realm we will use for checking role assignments
- Context context = getContext();
+ /*
+ * There might be a RequestDispatcher in use. That changes the wrapper
that needs to be used to look up any
+ * security-role-ref. If the dispatch is a cross-context dispatch, the
context changes as well.
+ */
+ Wrapper wrapper;
+ Context context;
+ if (mappingData.requestDispatcherWrapper != null) {
+ wrapper = mappingData.requestDispatcherWrapper;
+ Container container = wrapper.getParent();
+ if (container instanceof Context c) {
+ context = c;
+ } else {
+ context = null;
+ }
+ } else {
+ wrapper = getWrapper();
+ context = getContext();
+ }
+
+ /*
+ * This shouldn't happen. No context means no realm. No realm means no
user can be authenticated. That in turn
+ * means userPrincipal should have been null.
+ */
if (context == null) {
return false;
}
@@ -2272,13 +2293,13 @@ public class Request implements HttpServletRequest {
return true;
}
+ // Identify the Realm we will use for checking role assignments
Realm realm = context.getRealm();
if (realm == null) {
return false;
}
// Check for a role alias defined in a <security-role-ref> element
- Wrapper wrapper = getWrapper();
if (wrapper != null) {
String realRole = wrapper.findSecurityReference(role);
if (realRole != null) {
diff --git a/java/org/apache/catalina/connector/RequestFacade.java
b/java/org/apache/catalina/connector/RequestFacade.java
index 5354490250..aff5775d77 100644
--- a/java/org/apache/catalina/connector/RequestFacade.java
+++ b/java/org/apache/catalina/connector/RequestFacade.java
@@ -41,6 +41,7 @@ import jakarta.servlet.http.HttpSession;
import jakarta.servlet.http.HttpUpgradeHandler;
import jakarta.servlet.http.Part;
+import org.apache.catalina.Wrapper;
import org.apache.tomcat.util.res.StringManager;
/**
@@ -609,6 +610,18 @@ public class RequestFacade implements HttpServletRequest {
}
+ public Wrapper getRequestDispatcherWrapper() {
+ checkFacade();
+ return request.getMappingData().requestDispatcherWrapper;
+ }
+
+
+ public void setRequestDispatcherWrapper(Wrapper wrapper) {
+ checkFacade();
+ request.getMappingData().requestDispatcherWrapper = wrapper;
+ }
+
+
private void checkFacade() {
if (request == null) {
throw new
IllegalStateException(sm.getString("requestFacade.nullRequest"));
diff --git a/java/org/apache/catalina/core/ApplicationDispatcher.java
b/java/org/apache/catalina/core/ApplicationDispatcher.java
index 8cb78131dc..07335e6586 100644
--- a/java/org/apache/catalina/core/ApplicationDispatcher.java
+++ b/java/org/apache/catalina/core/ApplicationDispatcher.java
@@ -702,7 +702,7 @@ final class ApplicationDispatcher implements
AsyncDispatcher, RequestDispatcher
}
crossContext = !context.getEncodedPath().equals(contextPath);
}
- wrapper = new ApplicationHttpRequest(hcurrent, context,
crossContext);
+ wrapper = new ApplicationHttpRequest(hcurrent, context,
crossContext, this.wrapper);
} else {
wrapper = new ApplicationRequest(current);
}
diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java
b/java/org/apache/catalina/core/ApplicationHttpRequest.java
index 1523450b2f..220ab49fc7 100644
--- a/java/org/apache/catalina/core/ApplicationHttpRequest.java
+++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java
@@ -32,6 +32,8 @@ import java.util.NoSuchElementException;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletRequestWrapper;
import jakarta.servlet.http.HttpServletMapping;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;
@@ -41,6 +43,8 @@ import org.apache.catalina.Context;
import org.apache.catalina.Globals;
import org.apache.catalina.Manager;
import org.apache.catalina.Session;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.connector.RequestFacade;
import org.apache.catalina.util.ParameterMap;
import org.apache.catalina.util.RequestUtil;
import org.apache.catalina.util.URLEncoder;
@@ -96,6 +100,11 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
*/
protected final Context context;
+ /**
+ * The target servlet for this request.
+ */
+ private final Wrapper wrapper;
+
/**
* The context path for this request.
@@ -196,11 +205,29 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
* @param request The servlet request being wrapped
* @param context The target context for the wrapped request
* @param crossContext {@code true} if the wrapped request will be a
cross-context request, otherwise {@code false}
+ *
+ * @deprecated Use {@link #ApplicationHttpRequest(HttpServletRequest,
Context, boolean, Wrapper)}. Will be removed
+ * in Tomcat 12 onwards.
*/
+ @Deprecated
ApplicationHttpRequest(HttpServletRequest request, Context context,
boolean crossContext) {
+ this(request, context, crossContext, null);
+ }
+
+
+ /**
+ * Construct a new wrapped request around the specified servlet request.
+ *
+ * @param request The servlet request being wrapped
+ * @param context The target context for the wrapped request
+ * @param crossContext {@code true} if the wrapped request will be a
cross-context request, otherwise {@code false}
+ * @param wrapper The target Servlet for the wrapped request
+ */
+ ApplicationHttpRequest(HttpServletRequest request, Context context,
boolean crossContext, Wrapper wrapper) {
super(request);
this.context = context;
this.crossContext = crossContext;
+ this.wrapper = wrapper;
setRequest(request);
}
@@ -216,6 +243,57 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
}
+ @Override
+ public boolean isUserInRole(String role) {
+ /*
+ * isUserInRole() has been called during an include/forward.
isUserInRole() needs to use the security-role-ref
+ * mappings for the target Servlet of the RequestDispatcher.
+ *
+ * There may be multiple layers of wrapping on the Request and the
Response. One or more of those layers might
+ * have overridden isUserInRole().
+ *
+ * If isUserInRole() has been overridden, it is the responsibility of
whatever has overridden it to make sure
+ * that isUserInRole() returns the correct value.
+ *
+ * If the original Tomcat request object is not available then it is
the responsibility of whatever replaced it
+ * to make sure isUserInRole() returns the correct value.
+ */
+
+ // If there is no wrapper, there is no need for special handling.
+ if (wrapper == null) {
+ return super.isUserInRole(role);
+ }
+
+ // Try and find the inner Tomcat RequestFacade
+ ServletRequest request = getRequest();
+ while (request instanceof ServletRequestWrapper) {
+ request = ((ServletRequestWrapper) request).getRequest();
+ }
+
+ // If the facade isn't available, just call the overridden method.
+ if (request instanceof RequestFacade facade) {
+ /*
+ * Only set this in the outer ApplicationHttpRequest so that for a
long stream of include/forwards it is the
+ * current one that determines which wrapper is used.
+ */
+ boolean requestDispatcherWrapperSet = false;
+ if (facade.getRequestDispatcherWrapper() == null) {
+ facade.setRequestDispatcherWrapper(wrapper);
+ requestDispatcherWrapperSet = true;
+ }
+ try {
+ return super.isUserInRole(role);
+ } finally {
+ if (requestDispatcherWrapperSet) {
+ facade.setRequestDispatcherWrapper(null);
+ }
+ }
+ } else {
+ return super.isUserInRole(role);
+ }
+ }
+
+
/**
* Override the <code>getAttribute()</code> method of the wrapped request.
*
@@ -620,8 +698,7 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
// -------------------------------------------------------- Package Methods
/**
- * Recycle this request. Since there is no object reuse, this only ends
- * the session access.
+ * Recycle this request. Since there is no object reuse, this only ends
the session access.
*/
public void recycle() {
if (session != null) {
@@ -804,7 +881,7 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
*/
protected boolean setSpecial(String name, Object value) {
// Performance - see BZ 68089
- if (name == null || name.length() < shortestSpecialNameLength) {
+ if (name == null || name.length() < shortestSpecialNameLength) {
return false;
}
Integer index = specialsMap.get(name);
diff --git a/java/org/apache/catalina/mapper/MappingData.java
b/java/org/apache/catalina/mapper/MappingData.java
index 370a5fecb3..1fc76cde9e 100644
--- a/java/org/apache/catalina/mapper/MappingData.java
+++ b/java/org/apache/catalina/mapper/MappingData.java
@@ -54,6 +54,11 @@ public class MappingData {
* The mapped wrapper.
*/
public Wrapper wrapper = null;
+ /**
+ * The wrapper for the current request dispatcher target. Only set when
required so may be null even during the
+ * processing of a dispatched request.
+ */
+ public Wrapper requestDispatcherWrapper = null;
/**
* Whether this is a JSP wildcard mapping.
*/
@@ -92,6 +97,7 @@ public class MappingData {
contextSlashCount = 0;
contexts = null;
wrapper = null;
+ requestDispatcherWrapper = null;
jspWildCard = false;
requestPath.recycle();
wrapperPath.recycle();
diff --git a/test/org/apache/catalina/core/TestApplicationHttpRequest.java
b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
index 085ceb222c..66a033f5fd 100644
--- a/test/org/apache/catalina/core/TestApplicationHttpRequest.java
+++ b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
@@ -219,7 +219,7 @@ public class TestApplicationHttpRequest extends
TomcatBaseTest {
request.setAttribute("key", "value");
return request;
}
- return new ApplicationHttpRequest(getNestedRequest(depth - 1), null,
false);
+ return new ApplicationHttpRequest(getNestedRequest(depth - 1), null,
false, null);
}
diff --git
a/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
b/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
index 4a3cacb644..145ed8bca0 100644
--- a/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
+++ b/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
@@ -34,7 +34,7 @@ public class TesterApplicationHttpRequestPerformance {
org.apache.coyote.Request coyoteRequest = new
org.apache.coyote.Request();
Request request = new Request(null, coyoteRequest);
request.getMappingData().context = new TesterContext();
- ApplicationHttpRequest applicationHttpRequest = new
ApplicationHttpRequest(request, null, false);
+ ApplicationHttpRequest applicationHttpRequest = new
ApplicationHttpRequest(request, null, false, null);
// Warm-up
doTestGetAttribute(applicationHttpRequest);
@@ -60,7 +60,7 @@ public class TesterApplicationHttpRequestPerformance {
request.getMappingData().context = new TesterContext();
return request;
}
- return new ApplicationHttpRequest(getRequest(depth - 1), null, false);
+ return new ApplicationHttpRequest(getRequest(depth - 1), null, false,
null);
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 05eb82eadf..3bcc5e4a6c 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -324,6 +324,10 @@
the pool generated a name that repeated across pools, which caused
pools to share the same name. (remm)
</fix>
+ <fix>
+ Use RequestDispatcher target rather than source when looking up
+ <code>security-role-ref</code>s for the current Servlet. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
