SamBarker commented on code in PR #12179:
URL: https://github.com/apache/kafka/pull/12179#discussion_r882292722
##########
clients/src/main/java/org/apache/kafka/common/security/authenticator/SaslServerAuthenticator.java:
##########
@@ -681,8 +681,8 @@ else if (connectionsMaxReauthMs == null)
else
retvalSessionLifetimeMs = zeroIfNegative(
Math.min(credentialExpirationMs -
authenticationEndMs, connectionsMaxReauthMs));
- if (retvalSessionLifetimeMs > 0L)
- sessionExpirationTimeNanos = authenticationEndNanos + 1000
* 1000 * retvalSessionLifetimeMs;
+
+ sessionExpirationTimeNanos = authenticationEndNanos + 1000 *
1000 * retvalSessionLifetimeMs;
Review Comment:
Adding `|| credentialExpirationMs != null || connectionsMaxReauthMs != null`
to the if condition adds no value as we already know one of them has to be true
to reach the if guard in general.
I think the correct fix is to delay the initialisation of
`retvalSessionLifetimeMs` so that we can use a `>=` test in the expiry case and
`0` in the non expiry cases while still allowing `sessionExpirationTimeNanos`
being `null` to signal that re-auth is disabled.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
