[java] Fix SpotBugs findings in ChromiumDriver and PortProber

[mk868]

User description

Description

As described in this comment, SpotBugs found some additional bugs in the code.
In this PR I fix part of the found problems.

ChromiumDriver

SpotBugs error:

H C RV: Bad attempt to compute absolute value of signed 32-bit hashcode in org.openqa.selenium.chromium.ChromiumDriver.pin(String)  At ChromiumDriver.java:[line 215]

documentation

Yes there is an actual bug here.
When script.hashCode() == Integer.MIN_VALUE then the generated code is not valid JavaScript code, and will cause the script to crash.

  String.format(
      "window.seleniumPinnedScript%s = function(){%s}",
      Math.abs(Integer.MIN_VALUE), "...");
// output: window.seleniumPinnedScript-2147483648 = function(){...}

Solution: cast hashCode to long

  String.format(
      "window.seleniumPinnedScript%s = function(){%s}",
      Math.abs((long) Integer.MIN_VALUE), "...");
// output: window.seleniumPinnedScript2147483648 = function(){...}

PortProber

SpotBugs error:

M D REC: Exception is caught when Exception is not thrown in org.openqa.selenium.net.PortProber.isFree(String, int)  At PortProber.java:[line 122]

documentation

Actually the code in the try block declares that it can throw exceptions of types:

• IOException
• SocketException
• SecurityException
• IllegalArgumentException

This can be shortened to IOException | RuntimeException union.

Motivation and Context

Fixing the actual problems is necessary before enabling full SpotBugs analysis, in order to not break the build.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Fixed a bug in ChromiumDriver by casting the hash code to long to prevent invalid JavaScript code when the hash code equals Integer.MIN_VALUE.
• Simplified exception handling in PortProber by catching specific exceptions (IOException and RuntimeException) instead of a generic Exception.

---

Changes walkthrough 📝

Relevant files

Bug fix

ChromiumDriver.java Fix hashCode handling in `ChromiumDriver` pin method          java/src/org/openqa/selenium/chromium/ChromiumDriver.java Fixed potential bug by casting hashCode to long to avoid invalid JavaScript code. Ensured the script hash code is correctly handled even for Integer.MIN_VALUE. +2/-1      PortProber.java Simplify exception handling in `PortProber` isFree method java/src/org/openqa/selenium/net/PortProber.java Simplified exception handling by catching IOException and RuntimeException. Improved code clarity and reduced unnecessary catch-all exception handling. +1/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Potential Overflow The fix for the hashCode issue is correct, but it's worth considering if there are any other places in the code where similar issues might occur. A thorough review of hashCode usage throughout the codebase could be beneficial. Exception Handling The change from catching Exception to IOException | RuntimeException is more specific, which is good. However, it's worth considering if there are any specific exceptions that should be handled differently or logged for debugging purposes.

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Enhancement	Log exceptions for better debugging and issue tracking Consider logging the exception for debugging purposes, as silently catching exceptions may hide important issues. java/src/org/openqa/selenium/net/PortProber.java [122-124] } catch (IOException | RuntimeException e) { + logger.debug("Port {} is not free: {}", port, e.getMessage()); return false; } Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding logging for caught exceptions can significantly aid in debugging and tracking issues, making the code more maintainable. This enhancement is practical and improves the code's robustness by providing more visibility into potential problems.	7
Best practice	Use a more consistent method for converting int to long in hash code calculation Consider using Long.hashCode(script.hashCode()) instead of casting to long to ensure consistent behavior across different JVM implementations. java/src/org/openqa/selenium/chromium/ChromiumDriver.java [216] -Math.abs((long) script.hashCode()) +Math.abs(Long.hashCode(script.hashCode())) Apply this suggestion Suggestion importance[1-10]: 5 Why: The suggestion to use Long.hashCode(script.hashCode()) instead of casting to long is a valid approach to ensure consistent behavior across JVM implementations. However, the impact of this change is moderate as the current casting method is not necessarily problematic.	5

💡 Need additional feedback ? start a PR chat

[pujagani]

Thank you! @mk868