Skip to content

Conversation

mk868
Copy link
Contributor

@mk868 mk868 commented Oct 18, 2025

User description

🔗 Related Issues

Related #14291

💥 What does this PR do?

JSpecify annotations added to the:

  • org.openqa.selenium.net.LinuxEphemeralPortRangeDetector
  • org.openqa.selenium.net.NetworkUtils
  • org.openqa.selenium.net.PortProber
  • org.openqa.selenium.net.UrlChecker
  • org.openqa.selenium.net.Urls

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add JSpecify @NullMarked annotations to five net package classes

  • Mark nullable fields and return types with @nullable annotation

  • Add null-safety checks using Require.nonNull() for guaranteed non-null returns

  • Improve type safety and null-handling in NetworkUtils class


Diagram Walkthrough

flowchart LR
  A["Five net package classes"] -->|Add @NullMarked| B["Class-level null contracts"]
  A -->|Mark nullable fields| C["@Nullable annotations"]
  A -->|Add null checks| D["Require.nonNull() calls"]
  B --> E["Improved type safety"]
  C --> E
  D --> E
Loading

File Walkthrough

Relevant files
Enhancement
LinuxEphemeralPortRangeDetector.java
Add JSpecify @NullMarked annotation                                           

java/src/org/openqa/selenium/net/LinuxEphemeralPortRangeDetector.java

  • Add @NullMarked class-level annotation
  • Import org.jspecify.annotations.NullMarked
+2/-0     
NetworkUtils.java
Add comprehensive null-safety annotations and checks         

java/src/org/openqa/selenium/net/NetworkUtils.java

  • Add @NullMarked class-level annotation and @nullable imports
  • Mark four fields as @nullable
    (cachedIp4NonLoopbackAddressOfThisMachine,
    cachedIp4NonLoopbackAddressHostName, hostname, address)
  • Add Require.nonNull() checks in getHostname() and getHostAddress()
    methods
  • Mark three method return types as @nullable
    (getNonLoopbackAddressOfThisMachine, obtainLoopbackIp4Address,
    getIpOfLoopBackIp4)
  • Add null-safety check for loopback InetAddress in
    obtainLoopbackIp4Address()
  • Mark getLoopBackAndIp4Only() return type as @nullable
+18/-11 
PortProber.java
Add JSpecify @NullMarked annotation                                           

java/src/org/openqa/selenium/net/PortProber.java

  • Add @NullMarked class-level annotation
  • Import org.jspecify.annotations.NullMarked
+2/-0     
UrlChecker.java
Add JSpecify @NullMarked annotation                                           

java/src/org/openqa/selenium/net/UrlChecker.java

  • Add @NullMarked class-level annotation
  • Import org.jspecify.annotations.NullMarked
+2/-0     
Urls.java
Add JSpecify @NullMarked annotation                                           

java/src/org/openqa/selenium/net/Urls.java

  • Add @NullMarked class-level annotation
  • Import org.jspecify.annotations.NullMarked
+2/-0     

@selenium-ci selenium-ci added the C-java Java Bindings label Oct 18, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Nullability contract change

Description: The new nullable return in obtainLoopbackIp4Address() may return null where callers
previously expected a non-null String, potentially causing NPEs or logic errors in
unadjusted call sites outside this PR.
NetworkUtils.java [120-131]

Referred Code
public @Nullable String obtainLoopbackIp4Address() {
  final NetworkInterface networkInterface = getLoopBackAndIp4Only();
  if (networkInterface != null) {
    InetAddress loopback = networkInterface.getIp4LoopbackOnly();
    if (loopback != null) {
      return loopback.getHostName();
    }
  }

  final String ipOfIp4LoopBack = getIpOfLoopBackIp4();
  if (ipOfIp4LoopBack != null) {
    return ipOfIp4LoopBack;
Unexpected runtime throw

Description: getHostname() and getHostAddress() now throw at runtime via Require.nonNull if
hostname/address remain null, which could be triggered by edge network states and may
introduce unexpected exceptions.
NetworkUtils.java [60-70]

Referred Code
public String getHostname() {
  determineHostnameAndAddress();

  return Require.nonNull("Hostname", hostname);
}

public String getHostAddress() {
  determineHostnameAndAddress();

  return Require.nonNull("Address", address);
}
Ticket Compliance
🟡
🎫 #14291
🟢 Add JSpecify nullness annotations (@NullMarked, @nullable) across Selenium Java code to
explicitly document null contracts for parameters and return values.
Ensure IDEs and static analyzers can infer nullability to help prevent
NullPointerExceptions.
Apply annotations starting with relevant interfaces/classes, e.g., network-related
utilities, without breaking existing behavior.
Where nullable values are possible, add appropriate null-safety handling to avoid NPEs.
Verify project-wide consistency of nullness contracts beyond the five modified classes to
meet broader ticket intent across the codebase.
Confirm build integration includes jspecify dependency and that IDE/static analyzer
tooling picks up annotations in CI.
Validate no behavioral regressions at runtime in environments with diverse network
configurations.
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.

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

Copy link
Contributor

qodo-merge-pro bot commented Oct 18, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Re-evaluate the nullability contract change

The PR introduces a breaking change by making getHostname() and getHostAddress()
throw exceptions instead of returning null. It is suggested to instead annotate
the methods' return types with @Nullable to align with their "best-effort"
nature and avoid breaking the public API.

Examples:

java/src/org/openqa/selenium/net/NetworkUtils.java [60-70]
  public String getHostname() {
    determineHostnameAndAddress();

    return Require.nonNull("Hostname", hostname);
  }

  public String getHostAddress() {
    determineHostnameAndAddress();

    return Require.nonNull("Address", address);

 ... (clipped 1 lines)

Solution Walkthrough:

Before:

// in NetworkUtils.java
@NullMarked
public class NetworkUtils {
  private volatile @Nullable String hostname;
  private volatile @Nullable String address;

  public String getHostname() {
    determineHostnameAndAddress();
    return Require.nonNull("Hostname", hostname); // Throws if hostname is null
  }

  public String getHostAddress() {
    determineHostnameAndAddress();
    return Require.nonNull("Address", address); // Throws if address is null
  }
}

After:

// in NetworkUtils.java
@NullMarked
public class NetworkUtils {
  private volatile @Nullable String hostname;
  private volatile @Nullable String address;

  public @Nullable String getHostname() {
    determineHostnameAndAddress();
    return hostname; // Can return null
  }

  public @Nullable String getHostAddress() {
    determineHostnameAndAddress();
    return address; // Can return null
  }
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a breaking change in the public API contract of getHostname() and getHostAddress(), proposing a more appropriate, non-breaking solution that aligns with the PR's goal of improving null-safety via annotations.

High
Possible issue
Prevent potential NullPointerException on address

Add a null check for ip4NonLoopbackAddressOfThisMachine before calling
getHostAddress() to prevent a potential NullPointerException.

java/src/org/openqa/selenium/net/NetworkUtils.java [88-96]

 public @Nullable String getNonLoopbackAddressOfThisMachine() {
   InetAddress ip4NonLoopbackAddressOfThisMachine = getIp4NonLoopbackAddressOfThisMachine();
   if (!Objects.equals(
       cachedIp4NonLoopbackAddressOfThisMachine, ip4NonLoopbackAddressOfThisMachine)) {
     cachedIp4NonLoopbackAddressOfThisMachine = ip4NonLoopbackAddressOfThisMachine;
-    cachedIp4NonLoopbackAddressHostName = ip4NonLoopbackAddressOfThisMachine.getHostAddress();
+    cachedIp4NonLoopbackAddressHostName =
+        ip4NonLoopbackAddressOfThisMachine == null
+            ? null
+            : ip4NonLoopbackAddressOfThisMachine.getHostAddress();
   }
   return cachedIp4NonLoopbackAddressHostName;
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a potential NullPointerException that could occur if getIp4NonLoopbackAddressOfThisMachine() returns null, which is a valid scenario. The proposed fix prevents this critical runtime error.

High
  • Update

@mk868 mk868 force-pushed the jspecify-net branch 2 times, most recently from d1db2b9 to da5d929 Compare October 18, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants