[java] JSpecify annotations for Credential and MBean

[mk868]

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

• org.openqa.selenium.grid.jmx.MBean
• org.openqa.selenium.virtualauthenticator.Credential

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

---

PR Type

Enhancement

---

Description

• Add JSpecify null-safety annotations to MBean class

• Add JSpecify null-safety annotations to Credential class

• Improve null-safety handling in attribute and operation invocation methods

• Add defensive null checks for map lookups in getter/setter/invoke operations

---

Diagram Walkthrough

flowchart LR
  A["MBean.java<br/>Credential.java"] -->|Add @NullMarked| B["Class-level<br/>null-safety"]
  A -->|Add @Nullable| C["Method returns<br/>and parameters"]
  A -->|Add null checks| D["getAttribute/setAttribute<br/>invoke methods"]
  C --> E["Improved type<br/>safety"]
  D --> E

Loading

File Walkthrough

Relevant files

Enhancement

MBean.java Add JSpecify null-safety annotations and null checks          java/src/org/openqa/selenium/grid/jmx/MBean.java Add @NullMarked class-level annotation for null-safety Add @Nullable annotations to method return types and parameters (getter, setter, and various finder methods) Add defensive null checks in getAttribute(), setAttribute(), and invoke() methods to handle null map lookups Improve null-safety in AttributeInfo and OperationInfo handling +31/-15  Credential.java Add JSpecify null-safety annotations to Credential              java/src/org/openqa/selenium/virtualauthenticator/Credential.java Add @Nullable annotation to rpId field and getRpId() method return type Add @Nullable annotation to toMap() method return type for map values Update constructor parameter to reflect nullable rpId +4/-4

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Incomplete error handling Description: The change makes DynamicMBean#getAttribute return null when the attribute is missing or the getter is null, which can silently mask errors and enable information disclosure patterns if callers assume non-null values; consider throwing AttributeNotFoundException per JMX conventions instead of returning null. MBean.java [220-241] Referred Code @Override public @Nullable Object getAttribute(String attribute) { try { AttributeInfo attributeInfo = attributeMap.get(attribute); if (attributeInfo == null || attributeInfo.getter == null) { return null; } Object res = attributeInfo.getter.invoke(bean); if (res instanceof Map<?, ?>) { return ((Map<?, ?>) res) .entrySet().stream() .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().toString())); } else if (res instanceof Number) { return res; } else { return res.toString(); } } catch (IllegalAccessException | InvocationTargetException e) { LOG.severe("Error during execution: " + e.getMessage()); return null; } ... (clipped 1 lines)
	Incomplete error handling Description: The invoke method now returns null when an operation is not found, potentially hiding management misconfigurations and enabling ambiguous behavior; per JMX, it should signal errors (e.g., MBeanException or ReflectionException) rather than returning null. MBean.java [276-287] Referred Code @Override public @Nullable Object invoke(String actionName, Object[] params, String[] signature) { try { OperationInfo operationInfo = operationMap.get(actionName); if (operationInfo == null) { return null; } return operationInfo.method.invoke(bean, params); } catch (IllegalAccessException | InvocationTargetException e) { LOG.severe("Error during execution: " + e.getMessage()); return null; }
Ticket Compliance
🟢	🎫 #14291 🟢 Add JSpecify Nullness annotations across Selenium Java code to declare nullable parameters and return values. Use @nullable on APIs that can return or accept null to improve IDE/static analyzer detection. Use @NullMarked (or equivalent) to enforce non-null by default within scopes where possible. Improve null-safety handling to avoid NPEs at runtime where applicable.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Missing return statements in method Add missing return statements in the findSetter method for branches handling method names starting with get and is to ensure the found setter method is correctly returned. java/src/org/openqa/selenium/grid/jmx/MBean.java [158-175] private @Nullable Method findSetter(Method annotatedMethod) { ManagedAttribute ma = annotatedMethod.getAnnotation(ManagedAttribute.class); if (!"".equals(ma.setter())) { return findMethod(annotatedMethod.getDeclaringClass(), ma.setter()); } else { String name = annotatedMethod.getName(); if (name.startsWith("set")) { return annotatedMethod; } if (name.startsWith("get")) { - findMethod(annotatedMethod.getDeclaringClass(), "s" + name.substring(1)); + return findMethod(annotatedMethod.getDeclaringClass(), "s" + name.substring(1)); } if (name.startsWith("is")) { - findMethod(annotatedMethod.getDeclaringClass(), "set" + name.substring(2)); + return findMethod(annotatedMethod.getDeclaringClass(), "set" + name.substring(2)); } } return null; } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a significant bug in the findSetter method where return statements are missing, causing the logic for get and is prefixed methods to fail and always return null.	High
Learned best practice	Fail on invalid attribute sets Validate and throw precise exceptions when an attribute is unknown or not writable, and include the attribute name to aid debugging. java/src/org/openqa/selenium/grid/jmx/MBean.java [244-254] public void setAttribute(Attribute attribute) { + String name = attribute.getName(); + AttributeInfo attributeInfo = attributeMap.get(name); + if (attributeInfo == null) { + throw new IllegalArgumentException("Unknown attribute: " + name); + } + if (attributeInfo.setter == null) { + throw new IllegalStateException("Attribute not writable: " + name); + } try { - AttributeInfo attributeInfo = attributeMap.get(attribute.getName()); - if (attributeInfo == null || attributeInfo.setter == null) { - return; - } attributeInfo.setter.invoke(bean, attribute.getValue()); } catch (IllegalAccessException | InvocationTargetException e) { LOG.severe("Error during execution: " + e.getMessage()); + throw new IllegalStateException("Failed to set attribute: " + name, e); } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Validate inputs and states early; avoid silent no-ops and null returns in command methods to prevent logic errors and hidden failures.	Low
	Throw explicit errors on invalid attributes Fail fast with precise exceptions when an attribute is missing or not readable instead of returning null; include the attribute name in the message. java/src/org/openqa/selenium/grid/jmx/MBean.java [221-241] -public @Nullable Object getAttribute(String attribute) { +public Object getAttribute(String attribute) { + AttributeInfo attributeInfo = attributeMap.get(attribute); + if (attributeInfo == null) { + throw new IllegalArgumentException("Unknown attribute: " + attribute); + } + if (attributeInfo.getter == null) { + throw new IllegalStateException("Attribute not readable: " + attribute); + } try { - AttributeInfo attributeInfo = attributeMap.get(attribute); - if (attributeInfo == null || attributeInfo.getter == null) { - return null; + Object res = attributeInfo.getter.invoke(bean); + if (res instanceof Map<?, ?>) { + return ((Map<?, ?>) res) + .entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().toString())); + } else if (res instanceof Number) { + return res; + } else { + return res.toString(); } - Object res = attributeInfo.getter.invoke(bean); - ... } catch (IllegalAccessException | InvocationTargetException e) { LOG.severe("Error during execution: " + e.getMessage()); - return null; + throw new IllegalStateException("Failed to get attribute: " + attribute, e); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Validate inputs and states early to avoid NPEs and logic errors; avoid returning null where JMX contract expects informative errors.	Low
More