sainadh777 commented on code in PR #1086:
URL: https://github.com/apache/tomcat/pull/1086#discussion_r4321398480
##########
java/org/apache/catalina/manager/HTMLManagerServlet.java:
##########
@@ -865,6 +865,24 @@ protected void doSessions(ContextName cn,
HttpServletRequest req, HttpServletRes
req.setAttribute(APPLICATION_MESSAGE, outMessage);
displaySessionDetailPage(req, resp, cn, sessionId, smClient);
return;
+ } else if ("setMaxInactiveInterval".equals(action)) {
+ String sessionId = req.getParameter("sessionId");
+ String intervalParam = req.getParameter("maxInactiveInterval");
+ try {
+ int interval = Integer.parseInt(intervalParam);
+ if (setSessionMaxInactiveInterval(cn, sessionId, interval,
smClient)) {
+ req.setAttribute(APPLICATION_MESSAGE,
+
smClient.getString("htmlManagerServlet.sessionTimeoutUpdated", sessionId,
intervalParam));
+ } else {
+ req.setAttribute(APPLICATION_ERROR,
+
smClient.getString("htmlManagerServlet.sessionNotFound", sessionId));
+ }
Review Comment:
Addressed in ea3c34e779. The failure message now says that the maximum
inactive interval could not be updated, rather than assuming the session was
not found. Added separate debug logs for a missing session and a null HTTP
session; the existing invalidated-session log covers IllegalStateException.
These follow the debug logging convention of the adjacent session actions.
##########
test/org/apache/catalina/manager/TestManagerWebapp.java:
##########
@@ -760,15 +764,7 @@ public boolean isResponseBodyOK() {
Assert.assertTrue("CSRF nonce not found in manager HTML response",
m.find());
String nonce = m.group(1);
- String sessionCookie = null;
- for (String header : client.getResponseHeaders()) {
- if (header.startsWith("Set-Cookie:")) {
- String cookieValue =
header.substring("Set-Cookie:".length()).trim();
- sessionCookie = cookieValue.split(";")[0];
- break;
- }
- }
- Assert.assertNotNull("Session cookie not found", sessionCookie);
+ Assert.assertNotNull("Session cookie not found",
client.getSessionId());
Review Comment:
Addressed in ea3c34e779 by restoring the existing Set-Cookie extraction and
assertion. Replacing them was unnecessary for the existing GET request. The new
POST uses the latest session ID because authentication can rotate it; that
requirement is now documented beside the POST.
##########
test/org/apache/catalina/manager/TestManagerWebapp.java:
##########
@@ -778,7 +774,7 @@ public boolean isResponseBodyOK() {
" HTTP/1.1" + CRLF +
"Host: localhost" + CRLF +
"Authorization: " + basicHeader + CRLF +
- "Cookie: " + sessionCookie + CRLF +
+ "Cookie: JSESSIONID=" + client.getSessionId() + CRLF +
Review Comment:
Addressed in ea3c34e779 by restoring the original sessionCookie header for
this GET. Only the new POST uses client.getSessionId(), because authentication
can change the session ID during the preceding request. Reusing the original
cookie for the POST reproduced a 403; using the latest cookie passes. All five
TestManagerWebapp tests and validate/validate-eoln pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
