Trim entries in no_proxy list

[skatkov]

🔗 Related Issues

Fixes #17460

💥 What does this PR do?

trims no_proxy list

🔧 Implementation Notes

🤖 AI assistance

• No substantial AI assistance used
• [ x ] AI assisted (complete below)
  • Tool(s): OpenCode, Codex
  • What was generated: test reproducing the problem have been written by AI
  • [ x ] I reviewed all AI output and can explain the change

[qodo-code-review]

Review Summary by Qodo

Trim and normalize no_proxy list entries

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Trim whitespace from no_proxy entries in proxy configuration
• Extract no_proxy parsing logic into dedicated no_proxy_list method
• Update proxy matching to use trimmed no_proxy list consistently
• Add tests for whitespace handling in no_proxy entries

Diagram

flowchart LR
  A["no_proxy string input"] -->|split and trim| B["no_proxy_list method"]
  B -->|returns cleaned array| C["as_json serialization"]
  B -->|returns cleaned array| D["use_proxy? matching"]
  C --> E["Proxy JSON output"]
  D --> F["Proxy bypass decision"]

Loading

File Changes

1. rb/lib/selenium/webdriver/common/proxy.rb ✨ Enhancement +8/-1

Extract and enhance no_proxy parsing logic

• Added new no_proxy_list private method that splits comma-separated strings and trims whitespace
• Refactored as_json to use no_proxy_list instead of inline parsing logic
• Method handles both string and array inputs, rejecting empty entries

rb/lib/selenium/webdriver/common/proxy.rb

---

2. rb/lib/selenium/webdriver/remote/http/default.rb 🐞 Bug fix +1/-1

Use trimmed no_proxy list for proxy matching

• Updated use_proxy? method to use no_proxy_list for consistent trimmed entries
• Ensures proxy bypass matching works with whitespace-padded no_proxy values

rb/lib/selenium/webdriver/remote/http/default.rb

---

3. rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb 🧪 Tests +2/-2

Test no_proxy whitespace trimming behavior

• Updated test case to verify trimming of whitespace in no_proxy entries
• Changed test input from 'proxy_url, localhost' to ' proxy_url,localhost, 127.0.0.1 '
• Updated expected output to include trimmed 127.0.0.1 entry

rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb

---

View more (2)

4. rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb 🧪 Tests +14/-0

Add tests for no_proxy whitespace handling

• Added test for comma-separated no_proxy entries with whitespace
• Added test for single no_proxy entry with leading/trailing whitespace
• Both tests verify that trimmed entries are properly recognized for proxy bypass

rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb

---

5. rb/sig/lib/selenium/webdriver/common/proxy.rbs 📝 Documentation +2/-0

Add type signature for no_proxy_list method

• Added type signature for new no_proxy_list private method
• Method returns untyped value representing the cleaned no_proxy array

rb/sig/lib/selenium/webdriver/common/proxy.rbs

---

[CLAassistant]

All committers have signed the CLA.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

1. Array no_proxy not trimmed 🐞 Bug ≡ Correctness

Description

Proxy#no_proxy_list only strips whitespace when no_proxy is a String; when no_proxy is an
Array (a supported input), entries are returned unchanged. This can cause
Remote::Http::Default#use_proxy? host matching to fail (sending traffic through the proxy when it
should bypass) and will serialize whitespace-padded entries into capabilities.

Code

rb/lib/selenium/webdriver/common/proxy.rb[R166-170]

+      def no_proxy_list
+        return no_proxy unless no_proxy.is_a?(String)
+
+        no_proxy.split(',').map(&:strip).reject(&:empty?)
+      end

Evidence

The repository tests and code show no_proxy Arrays are supported, but no_proxy_list returns
non-String values unchanged. Since Default#use_proxy? compares each entry directly to
server_url.host, leading/trailing whitespace in Array entries will prevent matches and thus
incorrectly use the proxy.

rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb[26-36]
rb/lib/selenium/webdriver/common/proxy.rb[143-156]
rb/lib/selenium/webdriver/common/proxy.rb[165-170]
rb/lib/selenium/webdriver/remote/http/default.rb[149-165]

Agent prompt

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

### Issue description
`Proxy#no_proxy_list` trims and filters entries only for String inputs. When `no_proxy` is provided as an Array, elements are returned as-is, so values like `[' example.com ']` will not match `server_url.host` and will be serialized with whitespace.

### Issue Context
The codebase already treats `no_proxy` Arrays as valid input. With the new helper used in both capability serialization and HTTP proxy bypass matching, array entries should be normalized consistently with string parsing.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/proxy.rb[166-170]
- rb/lib/selenium/webdriver/remote/http/default.rb[149-165]
- rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb[26-36]

### Suggested fix
Update `no_proxy_list` to also normalize Arrays (strip each element and drop empty strings), e.g.:
- If `no_proxy` is a String: keep current behavior.
- If `no_proxy` is an Array: `no_proxy.map { |h| h.to_s.strip }.reject(&:empty?)`.

Add a unit test demonstrating that `Proxy.new(no_proxy: [' example.com ']).no_proxy_list` (and/or `caps.as_json['proxy']['noProxy']`) returns `['example.com']`, and that HTTP bypass matching works with array inputs containing whitespace.

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

---

Previous review results

Review updated until commit aab1803

Results up to commit 6490416

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

1. Array no_proxy not trimmed 🐞 Bug ≡ Correctness

Description

Proxy#no_proxy_list only strips whitespace when no_proxy is a String; when no_proxy is an
Array (a supported input), entries are returned unchanged. This can cause
Remote::Http::Default#use_proxy? host matching to fail (sending traffic through the proxy when it
should bypass) and will serialize whitespace-padded entries into capabilities.

Code

rb/lib/selenium/webdriver/common/proxy.rb[R166-170]

+      def no_proxy_list
+        return no_proxy unless no_proxy.is_a?(String)
+
+        no_proxy.split(',').map(&:strip).reject(&:empty?)
+      end

Evidence

The repository tests and code show no_proxy Arrays are supported, but no_proxy_list returns
non-String values unchanged. Since Default#use_proxy? compares each entry directly to
server_url.host, leading/trailing whitespace in Array entries will prevent matches and thus
incorrectly use the proxy.

rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb[26-36]
rb/lib/selenium/webdriver/common/proxy.rb[143-156]
rb/lib/selenium/webdriver/common/proxy.rb[165-170]
rb/lib/selenium/webdriver/remote/http/default.rb[149-165]

Agent prompt

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

### Issue description
`Proxy#no_proxy_list` trims and filters entries only for String inputs. When `no_proxy` is provided as an Array, elements are returned as-is, so values like `[' example.com ']` will not match `server_url.host` and will be serialized with whitespace.

### Issue Context
The codebase already treats `no_proxy` Arrays as valid input. With the new helper used in both capability serialization and HTTP proxy bypass matching, array entries should be normalized consistently with string parsing.

### Fix Focus Areas
- rb/lib/selenium/webdriver/common/proxy.rb[166-170]
- rb/lib/selenium/webdriver/remote/http/default.rb[149-165]
- rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb[26-36]

### Suggested fix
Update `no_proxy_list` to also normalize Arrays (strip each element and drop empty strings), e.g.:
- If `no_proxy` is a String: keep current behavior.
- If `no_proxy` is an Array: `no_proxy.map { |h| h.to_s.strip }.reject(&:empty?)`.

Add a unit test demonstrating that `Proxy.new(no_proxy: [' example.com ']).no_proxy_list` (and/or `caps.as_json['proxy']['noProxy']`) returns `['example.com']`, and that HTTP bypass matching works with array inputs containing whitespace.

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

---

[qodo-code-review]

Persistent review updated to latest commit 6490416

[qodo-code-review]

Persistent review updated to latest commit aab1803