This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new b9431c831d Use RequestDispatcher target rather than source for
security-role-refs
b9431c831d is described below
commit b9431c831de28a1aff51ab6878c235f24d3ce871
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Oct 6 14:49:02 2026 +0100
Use RequestDispatcher target rather than source for security-role-refs
---
java/org/apache/catalina/connector/Request.java | 27 +++++++-
.../apache/catalina/connector/RequestFacade.java | 13 ++++
.../catalina/core/ApplicationDispatcher.java | 2 +-
.../catalina/core/ApplicationHttpRequest.java | 81 +++++++++++++++++++++-
java/org/apache/catalina/mapper/MappingData.java | 6 ++
.../catalina/core/TestApplicationHttpRequest.java | 2 +-
.../TesterApplicationHttpRequestPerformance.java | 4 +-
webapps/docs/changelog.xml | 4 ++
8 files changed, 129 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/catalina/connector/Request.java
b/java/org/apache/catalina/connector/Request.java
index c848763e7e..f2fced5964 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -2391,8 +2391,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) {
+ context = (Context) container;
+ } 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;
}
@@ -2409,13 +2430,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 e35813e102..5a30faad31 100644
--- a/java/org/apache/catalina/connector/RequestFacade.java
+++ b/java/org/apache/catalina/connector/RequestFacade.java
@@ -43,6 +43,7 @@ import javax.servlet.http.Part;
import javax.servlet.http.PushBuilder;
import org.apache.catalina.Globals;
+import org.apache.catalina.Wrapper;
import org.apache.catalina.security.SecurityUtil;
import org.apache.tomcat.util.res.StringManager;
@@ -853,6 +854,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 8fbef38d90..87180d3e91 100644
--- a/java/org/apache/catalina/core/ApplicationDispatcher.java
+++ b/java/org/apache/catalina/core/ApplicationDispatcher.java
@@ -810,7 +810,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 c74db80178..9c8ac968c2 100644
--- a/java/org/apache/catalina/core/ApplicationHttpRequest.java
+++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java
@@ -44,6 +44,7 @@ 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;
@@ -100,6 +101,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 +202,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 +240,58 @@ 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) {
+ RequestFacade facade = (RequestFacade) request;
+ /*
+ * 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.
*
@@ -628,8 +704,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) {
@@ -801,7 +876,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 09f7b3febc..8be1eb4660 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.
*/
@@ -97,6 +102,7 @@ public class MappingData {
contextSlashCount = 0;
contexts = null;
wrapper = null;
+ requestDispatcherWrapper = null;
jspWildCard = false;
contextPath.recycle();
requestPath.recycle();
diff --git a/test/org/apache/catalina/core/TestApplicationHttpRequest.java
b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
index 88e6d92796..433cd225c9 100644
--- a/test/org/apache/catalina/core/TestApplicationHttpRequest.java
+++ b/test/org/apache/catalina/core/TestApplicationHttpRequest.java
@@ -220,7 +220,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 9412686e55..44e93c4bff 100644
--- a/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
+++ b/test/org/apache/catalina/core/TesterApplicationHttpRequestPerformance.java
@@ -33,7 +33,7 @@ public class TesterApplicationHttpRequestPerformance {
org.apache.coyote.Request coyoteRequest = new
org.apache.coyote.Request();
Request request = new Request(null);
request.setCoyoteRequest(coyoteRequest);
- ApplicationHttpRequest applicationHttpRequest = new
ApplicationHttpRequest(request, null, false);
+ ApplicationHttpRequest applicationHttpRequest = new
ApplicationHttpRequest(request, null, false, null);
// Warm-up
doTestGetAttribute(applicationHttpRequest);
@@ -59,7 +59,7 @@ public class TesterApplicationHttpRequestPerformance {
request.setCoyoteRequest(coyoteRequest);
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 93a6546f18..0beae0aac4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -309,6 +309,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]
