[asolntsev]

🔗 Related Issues

Part of #14291

💥 What does this PR do?

Marks the whole package org.openqa.selenium.bidi as non-nullable by default.

🔄 Types of changes

• Cleanup (formatting, renaming)

[qodo-code-review]

PR Type

(Describe updated until commit 371911b)

Enhancement, Tests

---

Description

• Add nullability annotations to all BiDi packages using JSpecify

• Replace deprecated Collections methods with modern Java alternatives

• Convert ExtensionData from abstract class to interface

• Improve test reliability with deterministic cookie naming

---

File Walkthrough

	Relevant files
Enhancement	21 files package-info.java Add package-level nullability annotation                                  +21/-0    BiDi.java Replace Collections methods with modern alternatives          +8/-20    BiDiException.java Add nullability annotation to cause parameter                        +2/-1      BiDiSessionStatus.java Add nullability checks and improve JSON parsing                    +6/-14    Command.java Replace Collections with Map.copyOf and readNonNull            +3/-5      Connection.java Add nullability annotations to send method                              +4/-3      ConverterFunctions.java Add nullability annotation to map function                              +2/-1      Input.java Replace Collections.singletonList with List.of                      +2/-3      package-info.java Add package-level nullability annotation                                  +21/-0    BrowsingContextPartitionDescriptor.java Simplify toMap method using Map.of                                              +1/-6      DeleteCookiesParameters.java Add nullability documentation and simplify toMap                  +12/-8    GetCookiesParameters.java Add nullability documentation and simplify toMap                  +7/-6      GetCookiesResult.java Add nullability checks and documentation                                  +7/-2      PartitionKey.java Clean up formatting and nullability annotations                    +0/-1      SetCookieParameters.java Add nullability documentation and simplify toMap                  +7/-7      StorageKeyPartitionDescriptor.java Add private modifier to map field                                                +1/-1      package-info.java Add package-level nullability annotation                                  +21/-0    ExtensionData.java Convert abstract class to interface                                            +2/-2      ExtensionArchivePath.java Implement interface and add documentation                                +7/-3      ExtensionBase64Encoded.java Implement interface and simplify toMap                                      +2/-3      ExtensionPath.java Implement interface and simplify toMap                                      +2/-3
Documentation	1 files WebExtension.java Add BiDi spec documentation links                                                +8/-0
Tests	1 files StorageCommandsTest.java Replace random with deterministic sequence counter              +20/-25
Dependencies	2 files BUILD.bazel Add JSpecify dependency                                                                    +1/-0      BUILD.bazel Add JSpecify dependency                                                                    +1/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟢	🎫 #14291 🟢 Add JSpecify nullness annotations across Selenium Java code to declare which parameters/return values can be null, improving IDE/static analysis and Kotlin interoperability.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Null edge case: addCookieOnServerSide appends cookie.getPath() without a null-check, risking a NullPointerException when the cookie path is unset. Referred Code private void addCookieOnServerSide(Cookie cookie) { StringBuilder url = new StringBuilder(cookiePage); url.append("?action=add"); url.append("&name=").append(cookie.getName()); url.append("&value=").append(cookie.getValue()); if (cookie.getDomain() != null) { url.append("&domain=").append(cookie.getDomain()); } url.append("&path=").append(cookie.getPath()); if (cookie.getExpiry() != null) { Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Parameter serialization: toMap() now always serializes filter and partition as Optional values (including Optional.empty()), which may violate the BiDi wire format expectations compared to the previous conditional inclusion behavior. Referred Code public Map<String, Object> toMap() { return Map.of( "filter", cookieFilter, "partition", partitionDescriptor); } Learn more about managing compliance generic rules or creating your own custom rules
Update

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	Unwrap and filter optionals In toMap(), unwrap the cookieFilter and partitionDescriptor Optionals. Only include entries in the returned map if the Optionals are present, and call their respective toMap() methods. java/src/org/openqa/selenium/bidi/storage/DeleteCookiesParameters.java [47-51] public Map<String, Object> toMap() { - return Map.of( - "filter", cookieFilter, - "partition", partitionDescriptor); + Map<String, Object> map = new HashMap<>(); + cookieFilter.ifPresent(filter -> map.put("filter", filter.toMap())); + partitionDescriptor.ifPresent(descriptor -> map.put("partition", descriptor.toMap())); + return Map.copyOf(map); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The PR change incorrectly passes Optional objects to Map.of. This suggestion fixes a functional bug by ensuring that only present values are added to the map and that their toMap() methods are called, which is necessary for correct JSON serialization according to the BiDi protocol.	Medium
	Conditionally include real values In toMap(), unwrap the cookieFilter and partitionDescriptor Optionals. Only include entries in the returned map if the Optionals are present, and call their respective toMap() methods. java/src/org/openqa/selenium/bidi/storage/GetCookiesParameters.java [43-47] public Map<String, Object> toMap() { - return Map.of( - "filter", cookieFilter, - "partition", partitionDescriptor); + Map<String, Object> map = new HashMap<>(); + cookieFilter.ifPresent(filter -> map.put("filter", filter.toMap())); + partitionDescriptor.ifPresent(descriptor -> map.put("partition", descriptor.toMap())); + return Map.copyOf(map); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The PR change incorrectly passes Optional objects to Map.of. This suggestion fixes a functional bug by ensuring that only present values are added to the map and that their toMap() methods are called, which is necessary for correct JSON serialization according to the BiDi protocol.	Medium
Learned best practice	Avoid appending null query params Guard the path query parameter so it’s only appended when cookie.getPath() is non-null (or supply a safe default), preventing &path=null from being sent. java/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java [411] -url.append("&path=").append(cookie.getPath()); +if (cookie.getPath() != null) { + url.append("&path=").append(cookie.getPath()); +} Apply / Chat Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Preserve null-safety when constructing URLs/strings; do not append query parameters with null values.	Low
Update

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡	🎫 #14291 🟢 Add JSpecify nullness annotations to Selenium Java code so parameters/returns that can be null are explicitly annotated. ⚪ Make nullness information transparent to IDEs and static analyzers (beyond JavaDoc). Improve interoperability with Kotlin via explicit nullability metadata.
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing null check: The new test code appends cookie.getPath() to a URL without checking for null, which can cause a NullPointerException and make the test brittle. Referred Code private void addCookieOnServerSide(Cookie cookie) { StringBuilder url = new StringBuilder(cookiePage); url.append("?action=add"); url.append("&name=").append(cookie.getName()); url.append("&value=").append(cookie.getValue()); if (cookie.getDomain() != null) { url.append("&domain=").append(cookie.getDomain()); } url.append("&path=").append(cookie.getPath()); if (cookie.getExpiry() != null) { Learn more about managing compliance generic rules or creating your own custom rules

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	Correctly build map from Optional values Modify toMap() to conditionally build the map, adding entries only if the Optional values for cookieFilter and partitionDescriptor are present to ensure correct JSON serialization. java/src/org/openqa/selenium/bidi/storage/DeleteCookiesParameters.java [47-51] public Map<String, Object> toMap() { - return Map.of( - "filter", cookieFilter, - "partition", partitionDescriptor); + Map<String, Object> map = new HashMap<>(); + cookieFilter.ifPresent(filter -> map.put("filter", filter)); + partitionDescriptor.ifPresent(descriptor -> map.put("partition", descriptor.toMap())); + + return Map.copyOf(map); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the PR's change to use Map.of with Optional values will lead to incorrect serialization, as the Optional objects themselves would be added to the map. This is a functional bug, and the proposed fix correctly builds the map by only including present values.	Medium
General	Only include present entries in map Modify toMap() to conditionally build the map, adding entries only if the Optional values for cookieFilter and partitionDescriptor are present to ensure correct serialization. java/src/org/openqa/selenium/bidi/storage/GetCookiesParameters.java [43-47] public Map<String, Object> toMap() { - return Map.of( - "filter", cookieFilter, - "partition", partitionDescriptor); + Map<String, Object> map = new HashMap<>(); + cookieFilter.ifPresent(filter -> map.put("filter", filter.toMap())); + partitionDescriptor.ifPresent(descriptor -> map.put("partition", descriptor.toMap())); + return Map.copyOf(map); } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that using Map.of with Optional fields will result in incorrect serialization, as the Optional objects themselves are placed in the map. The proposed fix correctly constructs the map by only including entries for present values, which fixes a functional bug introduced by the PR.	Medium
More