Copilot commented on code in PR #2807:
URL: https://github.com/apache/groovy/pull/2807#discussion_r3809232720
##########
subprojects/groovy-jmx/src/main/groovy/groovy/jmx/builder/JmxServerConnectorFactory.groovy:
##########
@@ -149,17 +157,34 @@ class JmxServerConnectorFactory extends AbstractFactory {
if (!props) return null
HashMap<String, Object> env = new HashMap<String, Object>()
- // secure connection
+ // Authentication. The com.sun.management.jmxremote.* names belong to
the JDK's
+ // out-of-the-box management agent, which translates them into the
jmx.remote.x.*
+ // names a connector server actually consumes (see
sun.management.jmxremote.
+ // ConnectorBootstrap). Nothing performs that translation here, so do
it: putting the
+ // agent's names into a connector environment leaves the connector
with no
+ // authenticator at all, and it accepts credential-less clients.
def auth = props.remove("com.sun.management.jmxremote.authenticate")
?: props.remove("authenticate")
- env.put("com.sun.management.jmxremote.authenticate", auth)
def pFile = props.remove("com.sun.management.jmxremote.password.file")
?: props.remove("passwordFile")
- env.put("com.sun.management.jmxremote.password.file", pFile)
def aFile = props.remove("com.sun.management.jmxremote.access.file")
?: props.remove("accessFile")
- env.put("com.sun.management.jmxremote.access.file", aFile)
+ def loginConfig =
props.remove("com.sun.management.jmxremote.login.config") ?:
props.remove("loginConfig")
+
+ if (Boolean.valueOf(auth?.toString())) {
+ // A caller may instead pass a JMXAuthenticator straight through,
which is the
+ // standard JSR-160 route for custom authentication and is a
credential source too.
+ boolean suppliedAuthenticator =
props.containsKey(JMXConnectorServer.AUTHENTICATOR)
+ if (!pFile && !loginConfig && !suppliedAuthenticator) {
Review Comment:
`props.containsKey(JMXConnectorServer.AUTHENTICATOR)` treats a
present-but-null (or wrong-typed) value as a valid credential source. That can
still result in an unauthenticated connector (null authenticator) or a later
runtime failure. Since the contract here is specifically a `JMXAuthenticator`,
validate the value rather than just the key’s presence.
--
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]
