This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new d69935a118 Fix BZ 66120
d69935a118 is described below
commit d69935a11875769b765d44652bbedcc1f3c38356
Author: Mark Thomas <[email protected]>
AuthorDate: Mon Aug 22 15:12:46 2022 +0100
Fix BZ 66120
Replicate the session note required by FORM authentication. This enables
session failover / persistence+restoration to work correctly if it
occurs during the FORM authentication process.
---
.../apache/catalina/ha/session/DeltaRequest.java | 24 ++++++++
.../apache/catalina/ha/session/DeltaSession.java | 69 +++++++++++++++++++++-
.../apache/catalina/session/StandardSession.java | 44 +++++++++++---
webapps/docs/changelog.xml | 9 +++
4 files changed, 138 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/ha/session/DeltaRequest.java
b/java/org/apache/catalina/ha/session/DeltaRequest.java
index 4a5c65792f..1d1dd23e44 100644
--- a/java/org/apache/catalina/ha/session/DeltaRequest.java
+++ b/java/org/apache/catalina/ha/session/DeltaRequest.java
@@ -53,6 +53,7 @@ public class DeltaRequest implements Externalizable {
public static final int TYPE_MAXINTERVAL = 3;
public static final int TYPE_AUTHTYPE = 4;
public static final int TYPE_LISTENER = 5;
+ public static final int TYPE_NOTE = 6;
public static final int ACTION_SET = 0;
public static final int ACTION_REMOVE = 1;
@@ -90,6 +91,15 @@ public class DeltaRequest implements Externalizable {
addAction(TYPE_ATTRIBUTE, ACTION_REMOVE, name, null);
}
+ public void setNote(String name, Object value) {
+ int action = (value == null) ? ACTION_REMOVE : ACTION_SET;
+ addAction(TYPE_NOTE, action, name, value);
+ }
+
+ public void removeNote(String name) {
+ addAction(TYPE_NOTE, ACTION_REMOVE, name, null);
+ }
+
public void setMaxInactiveInterval(int interval) {
addAction(TYPE_MAXINTERVAL, ACTION_SET, NAME_MAXINTERVAL,
Integer.valueOf(interval));
}
@@ -216,6 +226,20 @@ public class DeltaRequest implements Externalizable {
} else {
session.removeSessionListener(listener, false);
}
+ break;
+ case TYPE_NOTE:
+ if (info.getAction() == ACTION_SET) {
+ if (log.isTraceEnabled()) {
+ log.trace("Session.setNote('" + info.getName() +
"', '" + info.getValue() + "')");
+ }
+ session.setNote(info.getName(), info.getValue(),
false);
+ } else {
+ if (log.isTraceEnabled()) {
+ log.trace("Session.removeNote('" + info.getName()
+ "')");
+ }
+ session.removeNote(info.getName(), false);
+ }
+
break;
default:
log.warn(sm.getString("deltaRequest.invalidAttributeInfoType", info));
diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java
b/java/org/apache/catalina/ha/session/DeltaSession.java
index 8e4e803f79..968f02272c 100644
--- a/java/org/apache/catalina/ha/session/DeltaSession.java
+++ b/java/org/apache/catalina/ha/session/DeltaSession.java
@@ -808,6 +808,48 @@ public class DeltaSession extends StandardSession
implements Externalizable,Clus
}
+ @Override
+ public void removeNote(String name) {
+ removeNote(name, true);
+ }
+
+ public void removeNote(String name, boolean addDeltaRequest) {
+ lockInternal();
+ try {
+ super.removeNote(name);
+ if (addDeltaRequest) {
+ deltaRequest.removeNote(name);
+ }
+ } finally {
+ unlockInternal();
+ }
+ }
+
+
+ @Override
+ public void setNote(String name, Object value) {
+ setNote(name, value, true);
+ }
+
+ public void setNote(String name, Object value, boolean addDeltaRequest) {
+
+ if (value == null) {
+ removeNote(name, addDeltaRequest);
+ return;
+ }
+
+ lockInternal();
+ try {
+ super.setNote(name, value);
+ if (addDeltaRequest) {
+ deltaRequest.setNote(name, value);
+ }
+ } finally {
+ unlockInternal();
+ }
+ }
+
+
// -------------------------------------------- HttpSession Private Methods
/**
@@ -852,11 +894,30 @@ public class DeltaSession extends StandardSession
implements Externalizable,Clus
log.debug(sm.getString("deltaSession.readSession", id));
}
+ Object nextObject = stream.readObject();
+
+ // Compatibility with versions that do not persist the authentication
+ // notes
+ if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be
+ // 'expected session ID' and 'saved request'
+ if (nextObject != null) {
+
notes.put(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE,
nextObject);
+ }
+ nextObject = stream.readObject();
+ if (nextObject != null) {
+
notes.put(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE,
nextObject);
+ }
+
+ // Next object will be the number of attributes
+ nextObject = stream.readObject();
+ }
+
// Deserialize the attribute count and attribute values
if (attributes == null) {
attributes = new ConcurrentHashMap<>();
}
- int n = ( (Integer) stream.readObject()).intValue();
+ int n = ((Integer) nextObject).intValue();
boolean isValidSave = isValid;
isValid = true;
for (int i = 0; i < n; i++) {
@@ -955,6 +1016,12 @@ public class DeltaSession extends StandardSession
implements Externalizable,Clus
log.debug(sm.getString("deltaSession.writeSession", id));
}
+ // Write the notes associated with authentication. Without these,
+ // authentication can fail without sticky sessions or if there is a
+ // fail-over during authentication.
+
stream.writeObject(notes.get(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE));
+
stream.writeObject(notes.get(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE));
+
// Accumulate the names of serializable and non-serializable attributes
String keys[] = keys();
List<String> saveNames = new ArrayList<>();
diff --git a/java/org/apache/catalina/session/StandardSession.java
b/java/org/apache/catalina/session/StandardSession.java
index 545529f4b4..8525072e3c 100644
--- a/java/org/apache/catalina/session/StandardSession.java
+++ b/java/org/apache/catalina/session/StandardSession.java
@@ -57,6 +57,7 @@ import org.apache.catalina.Session;
import org.apache.catalina.SessionEvent;
import org.apache.catalina.SessionListener;
import org.apache.catalina.TomcatPrincipal;
+import org.apache.catalina.authenticator.SavedRequest;
import org.apache.catalina.security.SecurityUtil;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.res.StringManager;
@@ -1444,10 +1445,23 @@ public class StandardSession implements HttpSession,
Session, Serializable {
("readObject() loading session " + id);
}
- // The next object read could either be the number of attributes
(Integer) or the session's
- // authType followed by a Principal object (not an Integer)
+ if (notes == null) {
+ notes = new Hashtable<>();
+ }
+ /*
+ * The next object read could either be the number of attributes
+ * (Integer) or if authentication information is persisted then:
+ * - authType (String) - always present
+ * - Principal object - always present
+ * - expected session ID - present if BZ 66120 is fixed
+ * - saved request - present if BZ 66120 is fixed
+ *
+ * Note: Some, all or none of the above objects may be null
+ */
Object nextObject = stream.readObject();
if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be authType and
+ // Principal
setAuthType((String) nextObject);
try {
setPrincipal((Principal) stream.readObject());
@@ -1460,8 +1474,22 @@ public class StandardSession implements HttpSession,
Session, Serializable {
}
throw e;
}
- // After that, the next object read should be the number of
attributes (Integer)
+
nextObject = stream.readObject();
+ if (!(nextObject instanceof Integer)) {
+ // Not an Integer so the next two objects will be
+ // 'expected session ID' and 'saved request'
+ if (nextObject != null) {
+
notes.put(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE,
nextObject);
+ }
+ nextObject = stream.readObject();
+ if (nextObject != null) {
+
notes.put(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE,
nextObject);
+ }
+
+ // Next object will be the number of attributes
+ nextObject = stream.readObject();
+ }
}
// Deserialize the attribute count and attribute values
@@ -1508,10 +1536,6 @@ public class StandardSession implements HttpSession,
Session, Serializable {
if (listeners == null) {
listeners = new ArrayList<>();
}
-
- if (notes == null) {
- notes = new Hashtable<>();
- }
}
@@ -1552,6 +1576,8 @@ public class StandardSession implements HttpSession,
Session, Serializable {
// Gather authentication information (if configured)
String sessionAuthType = null;
Principal sessionPrincipal = null;
+ String expectedSessionId = null;
+ SavedRequest savedRequest = null;
if (getPersistAuthentication()) {
sessionAuthType = getAuthType();
sessionPrincipal = getPrincipal();
@@ -1560,6 +1586,8 @@ public class StandardSession implements HttpSession,
Session, Serializable {
manager.getContext().getLogger().warn(
sm.getString("standardSession.principalNotSerializable", id));
}
+ expectedSessionId = (String)
notes.get(org.apache.catalina.authenticator.Constants.SESSION_ID_NOTE);
+ savedRequest = (SavedRequest)
notes.get(org.apache.catalina.authenticator.Constants.FORM_REQUEST_NOTE);
}
// Write authentication information (may be null values)
@@ -1570,6 +1598,8 @@ public class StandardSession implements HttpSession,
Session, Serializable {
manager.getContext().getLogger().warn(
sm.getString("standardSession.principalNotSerializable",
id), e);
}
+ stream.writeObject(expectedSessionId);
+ stream.writeObject(savedRequest);
// Accumulate the names of serializable and non-serializable attributes
String keys[] = keys();
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 35de617224..d669f9be4b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -137,6 +137,11 @@
Improve handling of stack overflow errors when parsing SSI expressions.
(markt)
</fix>
+ <fix>
+ <bug>66120</bug>: Enable FORM authentication to work correctly if
+ session persistence and restoration occurs during the authentication
+ process. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
@@ -195,6 +200,10 @@
passed an unrecognised action type, a warning message will now be
logged. (markt)
</fix>
+ <fix>
+ <bug>66120</bug>: Enable FORM authentication to work correctly if
+ session failover occurs during the authentication process. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Other">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
