Skip to content

[java] skip browser-restricted ports when picking a free port#17561

Merged
titusfortner merged 4 commits into
trunkfrom
port_prober_java
May 24, 2026
Merged

[java] skip browser-restricted ports when picking a free port#17561
titusfortner merged 4 commits into
trunkfrom
port_prober_java

Conversation

@titusfortner
Copy link
Copy Markdown
Member

Was getting flaky tests from https://github.com/SeleniumHQ/selenium/actions/runs/26340150176/job/77540628162?pr=17559

Tracked down this as the root cause: Firefox has a hard-coded banned-port list and refuses to navigate
Not sure how common this is or if my PR just got unlucky.

💥 What does this PR do?

  • Prevents PortProber from handing out a bad port

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

Probably can do this for other bindings as well.

🔄 Types of changes

  • Bug fix (backwards compatible)

@titusfortner titusfortner requested a review from Copilot May 23, 2026 22:26
@selenium-ci selenium-ci added the C-java Java Bindings label May 23, 2026
@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Skip browser-restricted ports in PortProber

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Prevents PortProber from assigning browser-restricted ports
• Adds hardcoded list of unsafe ports from Firefox/Chromium
• Increases retry attempts from 5 to 10 iterations
• Skips port selection if restricted port is randomly chosen
Diagram
flowchart LR
  A["findFreePort()"] --> B["createAcceptablePort()"]
  B --> C{"isBrowserRestrictedPort?"}
  C -->|Yes| D["continue to next iteration"]
  C -->|No| E["checkPortIsFree()"]
  E --> F["return port or retry"]
  D --> B

Loading

File Changes

1. java/src/org/openqa/selenium/net/PortProber.java 🐞 Bug fix +41/-1

Add browser-restricted port filtering to PortProber

• Added Set import for browser-restricted ports collection
• Defined BROWSER_RESTRICTED_PORTS constant with 18 unsafe ports sourced from Mozilla and Chromium
• Added isBrowserRestrictedPort() static method to check if port is restricted
• Increased retry loop in findFreePort() from 5 to 10 iterations
• Added port restriction check before attempting to use a port

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


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 23, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1)

Grey Divider


Remediation recommended

1. Restricted ports waste retries 🐞 Bug ☼ Reliability
Description
In PortProber.findFreePort, selecting a browser-restricted seed port triggers continue but still
consumes one of the 10 loop iterations, reducing the number of actual free-port checks performed.
Under port contention, this increases the likelihood of hitting the terminal `Unable to find a free
port` exception despite other usable ports existing.
Code

java/src/org/openqa/selenium/net/PortProber.java[R90-96]

Evidence
The loop is bounded by i < 10, and when a restricted port is generated the code immediately
continues, which advances the loop without performing checkPortIsFree, effectively reducing the
number of real checks below 10.

java/src/org/openqa/selenium/net/PortProber.java[90-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`findFreePort()` uses a bounded `for` loop and `continue`s when a restricted port is chosen, but that still increments the loop counter. This means you may perform fewer than 10 actual `checkPortIsFree` attempts.

## Issue Context
This behavior can slightly increase the chance of failing to find a free port in congested environments.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/PortProber.java[90-101]

## Suggested change
Refactor to only increment the attempt counter when a port-free check is performed (e.g., use a `while (checks < 10)` loop, or decrement `i` when a restricted port is generated).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. findFreePort() change lacks tests 📘 Rule violation ☼ Reliability
Description
findFreePort() now skips browser-restricted ports and changes retry behavior, but there are no
unit tests asserting the new contract. This increases the risk of regressions and CI flakiness if
the restricted-port list or selection logic changes.
Code

java/src/org/openqa/selenium/net/PortProber.java[R90-96]

Evidence
PR Compliance ID 6 requires that bug fixes/behavior changes be accompanied by tests when feasible.
The PR changes findFreePort() to skip restricted ports, but the existing PortProberTest only
covers checkPortIsFree and includes no assertions for the new restricted-port logic.

AGENTS.md: Prefer Adding/Updating Tests for Implemented Changes (Favor Small Unit Tests; Avoid Mocks)
java/src/org/openqa/selenium/net/PortProber.java[90-96]
java/test/org/openqa/selenium/net/PortProberTest.java[28-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PortProber.findFreePort()` now avoids browser-restricted ports, but the behavior is not covered by unit tests.

## Issue Context
The PR introduces new logic (`isBrowserRestrictedPort` and the skip/continue in `findFreePort`) intended to prevent flaky failures caused by browsers rejecting certain ports. Without tests, future edits to the restricted port set or selection algorithm can reintroduce flakiness.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/PortProber.java[90-96]
- java/test/org/openqa/selenium/net/PortProberTest.java[28-55]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 38e9a8e

Results up to commit f188bc7


🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)


Remediation recommended
1. findFreePort() change lacks tests 📘 Rule violation ☼ Reliability
Description
findFreePort() now skips browser-restricted ports and changes retry behavior, but there are no
unit tests asserting the new contract. This increases the risk of regressions and CI flakiness if
the restricted-port list or selection logic changes.
Code

java/src/org/openqa/selenium/net/PortProber.java[R90-96]

Evidence
PR Compliance ID 6 requires that bug fixes/behavior changes be accompanied by tests when feasible.
The PR changes findFreePort() to skip restricted ports, but the existing PortProberTest only
covers checkPortIsFree and includes no assertions for the new restricted-port logic.

AGENTS.md: Prefer Adding/Updating Tests for Implemented Changes (Favor Small Unit Tests; Avoid Mocks)
java/src/org/openqa/selenium/net/PortProber.java[90-96]
java/test/org/openqa/selenium/net/PortProberTest.java[28-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`PortProber.findFreePort()` now avoids browser-restricted ports, but the behavior is not covered by unit tests.

## Issue Context
The PR introduces new logic (`isBrowserRestrictedPort` and the skip/continue in `findFreePort`) intended to prevent flaky failures caused by browsers rejecting certain ports. Without tests, future edits to the restricted port set or selection algorithm can reintroduce flakiness.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/PortProber.java[90-96]
- java/test/org/openqa/selenium/net/PortProberTest.java[28-55]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 13522c1


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)


Remediation recommended
1. Restricted ports waste retries 🐞 Bug ☼ Reliability
Description
In PortProber.findFreePort, selecting a browser-restricted seed port triggers continue but still
consumes one of the 10 loop iterations, reducing the number of actual free-port checks performed.
Under port contention, this increases the likelihood of hitting the terminal `Unable to find a free
port` exception despite other usable ports existing.
Code

java/src/org/openqa/selenium/net/PortProber.java[R90-96]

Evidence
The loop is bounded by i < 10, and when a restricted port is generated the code immediately
continues, which advances the loop without performing checkPortIsFree, effectively reducing the
number of real checks below 10.

java/src/org/openqa/selenium/net/PortProber.java[90-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`findFreePort()` uses a bounded `for` loop and `continue`s when a restricted port is chosen, but that still increments the loop counter. This means you may perform fewer than 10 actual `checkPortIsFree` attempts.

## Issue Context
This behavior can slightly increase the chance of failing to find a free port in congested environments.

## Fix Focus Areas
- java/src/org/openqa/selenium/net/PortProber.java[90-101]

## Suggested change
Refactor to only increment the attempt counter when a port-free check is performed (e.g., use a `while (checks < 10)` loop, or decrement `i` when a restricted port is generated).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents Java’s PortProber from selecting ports that modern browsers refuse to connect to (avoiding intermittent “unsafe/denied port” navigation failures in tests and local servers).

Changes:

  • Add a browser-restricted port allowlist check (BROWSER_RESTRICTED_PORTS + isBrowserRestrictedPort).
  • Skip restricted ports when selecting a candidate free port; increase retry attempts from 5 to 10.
  • Run ./go format before pushing to avoid formatter-related CI failures.

Comment thread java/src/org/openqa/selenium/net/PortProber.java
@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 23, 2026

Persistent review updated to latest commit 6666e54

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 13522c1

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented May 24, 2026

Persistent review updated to latest commit 38e9a8e

@titusfortner titusfortner merged commit 04497d5 into trunk May 24, 2026
39 of 41 checks passed
@titusfortner titusfortner deleted the port_prober_java branch May 24, 2026 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants