CodeSOD: It's Not What You Didn't Think it Wasn't

Mike fired up a local copy of his company's Java application and found out that, at least running locally, the login didn't work. Since the available documentation didn't make it clear how to set this up correctly, he plowed through the code to try and understand.

Along his way to finding out how to properly configure the system, he stumbled across its logic for ensuring that every page except the login page required a valid session.

/** * session shouldn't be checked for some pages. For example: for timeout * page.. Since we're redirecting to timeout page from this filter, if we * don't disable session control for it, filter will again redirect to it * and this will be result with an infinite loop... * @param httpServletRequest the http servlet request * @return true, if is session control required for this resource */private boolean isSessionControlRequiredForThisResource(HttpServletRequest httpServletRequest) {String requestPath = httpServletRequest.getRequestURI();boolean controlRequiredLogin = !StringUtils.contains(requestPath, "login");return !controlRequiredLogin ? false : true;}

The core of the logic here is definitely a big whiff of bad decisions. Checking if the URL contains the word "login" seems like an incredibly fragile way to disable sessions. And, as it relies on "login" never showing up in the URI in any other capacity, I suspect this could end up being a delayed action foot-gun.

But that's all hypothetical. Because TRWTF here is the stack of negations.

The logic is: If it isn't the case that the string doesn't contain "login" we return false, otherwise we return true.

As Mike writes:

It took me a good 10 minutes to assure myself that logic was correct. ... This could be rewritten in a saner manner with

String requestPath = httpServletRequest.getRequestURI();return !StringUtils.contains(requestPath, "login");

Anyway, It made me laugh when I saw it. My login issue at the end of the day had nothing to do with this but coming across this was a treat.

"Treat" might be the wrong word.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!