Bug 62988

Summary: LoadBalancerDrainingValve only works with uncommon webapp configuration
Product: Tomcat 9 Reporter: Andreas Kurth <apache>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: 9.0.13   
Target Milestone: -----   
Hardware: PC   
OS: Linux   
Attachments: Patch for LoadBalancerDrainingValve (see bug description)

Description Andreas Kurth 2018-12-06 15:18:23 UTC
Created attachment 36298 [details]
Patch for LoadBalancerDrainingValve (see bug description)

After configuring LoadBalancerDrainingValve in order to force redirects when mod_jk would route to a DISABLED node we encountered the following bugs:

1)
NullPointerException when the

<session-config>
  <cookie-config>
    <name>JSESSIONID</name>
  </cookie-config>
</session-config>

<name> element would not be present in web.xml. It cannot be expected and is in fact highly unlikely that web applications override the default value "JSESSIONID" in this element.

2)
The removal of a session cookie before redirection does only work when the Context attribute sessionCookiePathUsesTrailingSlash has been set to true, which is also a rather uncommon configuration setting. Failing to remove the session cookie leads to redirection loop. The bug results from the fact that the respective code block is wrongly parenthesized, and can easily be fixed by moving the code block out of the surrounding if statement.

3)
Determination of the session cookie path is incomplete.

I added a patch (trunk) with fixes for these bugs, which also addresses the following minor issues:

- Remove obsolete get methods for configuration parameters
- Remove Javadoc which states that a request parameter will be added to redirect URIs, which is not the case
- Add missing parentheses
Comment 1 Mark Thomas 2018-12-06 19:20:51 UTC
Thanks for the report and the patch.

Working my way through the patch I agree with nearly all of these changes with a couple of minor observations
- The getters are not obsolete. They are used by JMX so they need to stay
- I did the necessary refactoring to remove the code duplication.

Fixed in:
- trunk for 9.0.14 onwards
- 8.5.x for 8.5.36 onwards