Stop SLF4JMDCServletFilter from always creating a HTTP session

Description

Snippet from net.shibboleth.idp.log.SLF4JMDCServletFilter:

if (request instanceof HttpServletRequest) { final HttpSession session = ((HttpServletRequest) request).getSession(); if (session != null) { MDC.put(JSESSIONID_MDC_ATTRIBUTE, session.getId()); } }

The above call to getSession() will always create a HTTP session even if it doesn't exist (and this is according the spec in the HttpServletRequest interface).  The spec also provides a getSession(bool) function that indicates whether a session should be created if it doesn't already exist.

Could the above code be modified to use getSession(false) so that a HTTP session isn't always created for every request (since that filter applies to /*)?  This way things like monitoring requests don't needlessly create sessions (and it even looks like returning users with a valid IdP session but no HTTP session will not need a HTTP session created either).

Environment

None

Activity

Scott Cantor 
February 18, 2021 at 7:18 PM

It definitely compromises the function of the filter to not create the session, but as a cleaner option for people not using it "fully", I added a createSession init parameter option that defaults true.

Scott Cantor 
February 18, 2021 at 6:12 PM

I never got back to it, but I may want to do a bit of testing with that suggested approach of passing false to the API. If that doesn't "break" the log output under ordinary conditions I can try, then I probably wouldn't object to doing that.

Rod Widdowson 
February 18, 2021 at 5:06 PM

Almost a year old. Close/Won't do, or Do?

Scott Cantor 
February 21, 2020 at 2:04 PM

I'll leave it for a bit since it doesn't need any action for V4, so I can think about it a bit more, but noted, thank you.

Jamie Arthur 
February 21, 2020 at 4:01 AM

I agree with the "leave it as is and let people update the mapping if required" approach.  For our deployment, we don't need to log the web session id, so we replace the default SLF4JMDCServletFilter with ch.qos.logback.classic.helpers.MDCInsertingServletFilter since it includes fields that we do want to log.

I'm happy for this issue to be closed.

Done

Details

Assignee

Reporter

Fix versions

Created October 3, 2019 at 4:27 AM
Updated March 24, 2021 at 3:41 PM
Resolved February 18, 2021 at 7:18 PM