[py] fix no_proxy matching so empty entries and substrings do not bypass the proxy

[navin772]

🔗 Related Issues

💥 What does this PR do?

ClientConfig.get_proxy_url() tested whether a no_proxy entry covered the remote server address with a substring check (n_url.path in remote_add.netloc), which had three defects:

1. An empty entry disabled the proxy entirely. "" parses to an empty path and "" in anything is True, so the first empty entry short-circuited to "bypass" for every host, no_proxy=example.com, silently sent all driver traffic direct.
2. Substrings matched. no_proxy=foo.com bypassed the proxy for myfoo.com.other.org.
3. Matching was case-sensitive, so NO_PROXY=EXAMPLE.COM missed example.com.

🔧 Implementation Notes

🤖 AI assistance

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

💡 Additional Considerations

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

PR Summary by Qodo

Fix Python no_proxy matching to ignore empty entries and avoid substring bypass

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Make no_proxy matching case-insensitive and suffix-based (host/subdomain), not substring-based.
• Ignore empty no_proxy entries so trailing/double commas don’t disable proxies.
• Add unit coverage for bypass and non-bypass scenarios across ClientConfig and
 RemoteConnection.

Diagram

graph TD
A["RemoteConnection"] --> B["ClientConfig.get_proxy_url"] --> C{"Bypass proxy?"}
C -->|"yes"| D["Direct (PoolManager)"]
C -->|"no"| E["Proxy URL from env"] --> F["ProxyManager"]
B --> G["_no_proxy_entry_matches()"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Delegate bypass check to urllib.request.proxy_bypass_environment

• ➕ Reuses well-known stdlib semantics and edge-case handling
• ➕ Avoids maintaining custom matching logic over time
• ➖ Less control over which address form is evaluated (hostname vs netloc w/port)
• ➖ May introduce broader matching behavior changes (e.g., special-cases) beyond current Selenium expectations

2. Implement a full no_proxy parser (CIDR, wildcards, )

• ➕ Max compatibility with diverse no_proxy conventions seen in the wild
• ➕ Would align better with existing env examples like CIDR blocks
• ➖ More complex and higher maintenance burden
• ➖ Harder to test exhaustively; higher risk of subtle regressions

Recommendation: The PR’s approach is a good balance: it fixes the concrete correctness/security issues (empty entries, substring matching, case sensitivity) while staying small and well-tested. Consider stdlib delegation only if Selenium wants to fully track Python’s proxy-bypass semantics (including broader pattern handling).

Files changed (3) +153 / -19

Bug fix (1) +30 / -4

client_config.pyAdd robust no_proxy entry matcher and use it in system proxy selection +30/-4

Add robust no_proxy entry matcher and use it in system proxy selection

• Introduces '_no_proxy_entry_matches()' to implement case-insensitive host/subdomain-suffix matching and to ignore empty entries. Updates 'ClientConfig.get_proxy_url()' to use this helper (and normalized hostname/netloc) instead of substring/path checks, preventing unintended proxy bypass.

py/selenium/webdriver/remote/client_config.py

Tests (2) +123 / -15

client_config_tests.pyAdd focused unit tests for no_proxy matching edge cases +95/-0

Add focused unit tests for no_proxy matching edge cases

• Adds fixtures to control proxy-related environment variables and a helper to build a system-proxy ClientConfig. Adds parameterized tests covering empty entries, substring non-matches, expected bypass matches (including dot-prefixed and case-insensitive entries), wildcard behavior, URL-form entries, and behavior when no_proxy is unset.

py/test/unit/selenium/webdriver/remote/client_config_tests.py

remote_connection_tests.pyTighten RemoteConnection proxy/no_proxy assertions and add non-match cases +28/-15

Tighten RemoteConnection proxy/no_proxy assertions and add non-match cases

• Fixes tests to assert 'type(conn) is PoolManager' to distinguish direct connections from proxied ones (since proxy managers subclass PoolManager). Expands no_proxy coverage to include explicit schemes/ports and adds tests ensuring non-matching hosts still route through ProxyManager.

py/test/unit/selenium/webdriver/remote/remote_connection_tests.py

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (0)

1. Docstring Args missing types 📘 Rule violation ✧ Quality ⭐ New

Description

_no_proxy_entry_matches() uses an Args: section but does not include Google-style parameter
types (e.g., netloc (str): ...). This does not meet the required docstring format and reduces
clarity/consistency of API documentation.

Code

py/selenium/webdriver/remote/client_config.py[R47-48]

+        netloc: Lower-cased host of the remote server address, with any port and
+            without the brackets an IPv6 literal is written with.

Evidence

The checklist requires Google-style docstrings where each argument is documented as `name (Type):
.... The docstring documents netloc` but omits a parenthesized type.

Rule 337804: Enforce Google-style docstrings with Args/Returns/Raises sections
py/selenium/webdriver/remote/client_config.py[47-48]

Agent prompt

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

## Issue description
`_no_proxy_entry_matches()` has an `Args:` section but omits the required Google-style type annotations for parameters (e.g., `arg (str): ...`).

## Issue Context
This project requires Google-style docstrings with typed `Args:` entries for modified/new functions.

## Fix Focus Areas
- py/selenium/webdriver/remote/client_config.py[43-52]

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

---

2. system_config lacks type annotations 📘 Rule violation ✧ Quality

Description

Newly added test helper/fixture functions have untyped parameters and no return annotations,
violating the requirement for type-annotated new function signatures. This reduces readability and
makes static analysis/type checking less effective.

Code

py/test/unit/selenium/webdriver/remote/client_config_tests.py[R45-46]

+def system_config(remote_server_addr="http://localhost:4444"):
+    return ClientConfig(remote_server_addr=remote_server_addr, proxy=Proxy(raw={"proxyType": ProxyType.SYSTEM}))

Evidence

PR Compliance ID 337802 requires type annotations on new function and method signatures. In
client_config_tests.py, the newly added system_proxy_env(monkeypatch) and
system_config(remote_server_addr=...) lack parameter and return type annotations.

Rule 337802: Require type annotations on new function and method signatures
py/test/unit/selenium/webdriver/remote/client_config_tests.py[32-46]

Agent prompt

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

## Issue description
Newly added functions in the test module are missing parameter and return type annotations.

## Issue Context
Compliance requires explicit type annotations on new function/method signatures (including return types).

## Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[32-46]

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

---

3. IPv6 URL no_proxy fails ✓ Resolved 🐞 Bug ≡ Correctness

Description

_no_proxy_entry_matches() parses URL-form entries using urlparse(...).netloc, which preserves IPv6
brackets (e.g. "[::1]"). When connecting to an IPv6 remote like "http://[::1]:4444", the matcher
compares against hostname "::1" and netloc "[::1]:4444", so an entry like "http://[::1]" matches
neither and the proxy is incorrectly not bypassed.

Code

py/selenium/webdriver/remote/client_config.py[R54-56]

+    if "://" in entry:
+        entry = parse.urlparse(entry).netloc
+    entry = entry.strip().lstrip(".").lower()

Evidence

The matcher normalizes URL-form entries using urlparse(...).netloc (bracketed for IPv6), while
get_proxy_url() passes in a bracketless hostname and a potentially port-suffixed netloc,
making http://[::1] unable to match http://[::1]:4444. Existing tests demonstrate the intended
behavior for URL-form entries matching host+port remote URLs, but do not cover IPv6, so this
mismatch is currently untested.

py/selenium/webdriver/remote/client_config.py[35-59]
py/selenium/webdriver/remote/client_config.py[157-176]
py/test/unit/selenium/webdriver/remote/client_config_tests.py[126-130]

Agent prompt

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

## Issue description
`_no_proxy_entry_matches()` uses `urlparse(entry).netloc` for URL-form `no_proxy` entries. For IPv6 literals, `.netloc` retains brackets (`[::1]`), while the remote URL’s `hostname` is bracketless (`::1`) and the remote `netloc` may include a port (`[::1]:4444`). This causes URL-form entries like `http://[::1]` to fail to bypass the proxy for common Selenium remote addresses like `http://[::1]:4444`.

## Issue Context
The test suite already asserts that URL-form entries without ports should match host+port remote URLs for regular hostnames (e.g., `no_proxy=http://example.com` should bypass `http://example.com:4444`). The same expectation should hold for IPv6 literals.

## Fix Focus Areas
- py/selenium/webdriver/remote/client_config.py[35-59]
- py/selenium/webdriver/remote/client_config.py[163-172]
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[126-130]

## Implementation notes
- When parsing URL-form entries, prefer `parsed.hostname` (bracketless for IPv6) and handle `parsed.port` explicitly.
- If a port is present in the no_proxy entry, require matching port; if absent, match by hostname only.
- Add a regression test case like:
 - `no_proxy="http://[::1]"` should bypass `remote_server_addr="http://[::1]:4444"`.
 - Optionally also test `no_proxy="http://[::1]:4444"` matches only that port.

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

---

Context used

✅ Compliance rules (platform): 17 rules

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

Previous review results

Review updated until commit 7ac9352

Results up to commit 5acf9ee ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. system_config lacks type annotations 📘 Rule violation ✧ Quality

Description

Newly added test helper/fixture functions have untyped parameters and no return annotations,
violating the requirement for type-annotated new function signatures. This reduces readability and
makes static analysis/type checking less effective.

Code

py/test/unit/selenium/webdriver/remote/client_config_tests.py[R45-46]

+def system_config(remote_server_addr="http://localhost:4444"):
+    return ClientConfig(remote_server_addr=remote_server_addr, proxy=Proxy(raw={"proxyType": ProxyType.SYSTEM}))

Evidence

PR Compliance ID 337802 requires type annotations on new function and method signatures. In
client_config_tests.py, the newly added system_proxy_env(monkeypatch) and
system_config(remote_server_addr=...) lack parameter and return type annotations.

Rule 337802: Require type annotations on new function and method signatures
py/test/unit/selenium/webdriver/remote/client_config_tests.py[32-46]

Agent prompt

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

## Issue description
Newly added functions in the test module are missing parameter and return type annotations.

## Issue Context
Compliance requires explicit type annotations on new function/method signatures (including return types).

## Fix Focus Areas
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[32-46]

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

---

2. IPv6 URL no_proxy fails ✓ Resolved 🐞 Bug ≡ Correctness

Description

_no_proxy_entry_matches() parses URL-form entries using urlparse(...).netloc, which preserves IPv6
brackets (e.g. "[::1]"). When connecting to an IPv6 remote like "http://[::1]:4444", the matcher
compares against hostname "::1" and netloc "[::1]:4444", so an entry like "http://[::1]" matches
neither and the proxy is incorrectly not bypassed.

Code

py/selenium/webdriver/remote/client_config.py[R54-56]

+    if "://" in entry:
+        entry = parse.urlparse(entry).netloc
+    entry = entry.strip().lstrip(".").lower()

Evidence

The matcher normalizes URL-form entries using urlparse(...).netloc (bracketed for IPv6), while
get_proxy_url() passes in a bracketless hostname and a potentially port-suffixed netloc,
making http://[::1] unable to match http://[::1]:4444. Existing tests demonstrate the intended
behavior for URL-form entries matching host+port remote URLs, but do not cover IPv6, so this
mismatch is currently untested.

py/selenium/webdriver/remote/client_config.py[35-59]
py/selenium/webdriver/remote/client_config.py[157-176]
py/test/unit/selenium/webdriver/remote/client_config_tests.py[126-130]

Agent prompt

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

## Issue description
`_no_proxy_entry_matches()` uses `urlparse(entry).netloc` for URL-form `no_proxy` entries. For IPv6 literals, `.netloc` retains brackets (`[::1]`), while the remote URL’s `hostname` is bracketless (`::1`) and the remote `netloc` may include a port (`[::1]:4444`). This causes URL-form entries like `http://[::1]` to fail to bypass the proxy for common Selenium remote addresses like `http://[::1]:4444`.

## Issue Context
The test suite already asserts that URL-form entries without ports should match host+port remote URLs for regular hostnames (e.g., `no_proxy=http://example.com` should bypass `http://example.com:4444`). The same expectation should hold for IPv6 literals.

## Fix Focus Areas
- py/selenium/webdriver/remote/client_config.py[35-59]
- py/selenium/webdriver/remote/client_config.py[163-172]
- py/test/unit/selenium/webdriver/remote/client_config_tests.py[126-130]

## Implementation notes
- When parsing URL-form entries, prefer `parsed.hostname` (bracketless for IPv6) and handle `parsed.port` explicitly.
- If a port is present in the no_proxy entry, require matching port; if absent, match by hostname only.
- Add a regression test case like:
 - `no_proxy="http://[::1]"` should bypass `remote_server_addr="http://[::1]:4444"`.
 - Optionally also test `no_proxy="http://[::1]:4444"` matches only that port.

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

---

[cgoldberg]

It's probably worth fixing the IPv6 edge case that qodo bot mentioned and adding a test for it. Besides that, LGTM .. nice addition of unit tests

[qodo-code-review]

Code review by qodo was updated up to the latest commit 7ac9352