Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion rb/lib/selenium/webdriver/common/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def as_json(*)
'proxyType' => TYPES[type].downcase,
'ftpProxy' => ftp,
'httpProxy' => http,
'noProxy' => no_proxy.is_a?(String) ? no_proxy.split(', ') : no_proxy,
'noProxy' => no_proxy_list,
'proxyAutoconfigUrl' => pac,
'sslProxy' => ssl,
'autodetect' => auto_detect,
Expand All @@ -161,6 +161,13 @@ def as_json(*)
def to_json(*)
JSON.generate as_json
end

# @api private
def no_proxy_list
return no_proxy unless no_proxy.is_a?(String)

no_proxy.split(',').map(&:strip).reject(&:empty?)
end
end # Proxy
end # WebDriver
end # Selenium
16 changes: 14 additions & 2 deletions rb/lib/selenium/webdriver/remote/http/default.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,10 @@ def proxy
def use_proxy?
return false if proxy.nil?

if proxy.no_proxy
ignored = proxy.no_proxy.split(',').any? do |host|
no_proxy = no_proxy_list

if no_proxy
ignored = no_proxy.any? do |host|
host == '*' ||
host == server_url.host || (
begin
Expand All @@ -166,6 +168,16 @@ def use_proxy?
true
end
end

def no_proxy_list
return proxy.no_proxy_list if proxy.respond_to?(:no_proxy_list)
return unless proxy.respond_to?(:no_proxy)

no_proxy = proxy.no_proxy
return no_proxy unless no_proxy.is_a?(String)

no_proxy.split(',').map(&:strip).reject(&:empty?)
Comment on lines +172 to +179

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

1. no_proxy_list lacks type validation 📘 Rule violation ☼ Reliability

no_proxy_list returns proxy.no_proxy unchanged for any non-String value, but use_proxy?
assumes the result responds to any?, risking a runtime NoMethodError for invalid config values.
This violates the requirement to validate config-derived inputs early and fail with deterministic,
actionable exceptions.
Agent Prompt
## Issue description
`no_proxy_list` can return a non-enumerable value (e.g., Integer/Object) when `proxy.no_proxy` is not a `String`, causing `use_proxy?` to call `any?` and crash with `NoMethodError`. Per compliance, config-derived inputs should be validated early and raise a deterministic, actionable exception.

## Issue Context
`proxy.no_proxy` can be provided via user config/duck-typed proxy objects, so its type is not guaranteed.

## Fix Focus Areas
- rb/lib/selenium/webdriver/remote/http/default.rb[152-156]
- rb/lib/selenium/webdriver/remote/http/default.rb[172-180]

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

end
end # Default
end # Http
end # Remote
Expand Down
2 changes: 2 additions & 0 deletions rb/sig/lib/selenium/webdriver/common/proxy.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ module Selenium
def as_json: (*untyped) -> untyped

def to_json: (*untyped) -> untyped

def no_proxy_list: () -> untyped
end
end
end
7 changes: 4 additions & 3 deletions rb/spec/integration/selenium/server_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ module Selenium
end

it 'downloads specified version' do
@location = described_class.download('4.9.0')
version = described_class.latest
@location = described_class.download(version)

expect(File.exist?(@location)).to be true
expect(@location).to eq('selenium-server-4.9.0.jar')
expect(@location).to eq("selenium-server-#{version}.jar")
end

it 'starts and stops server' do
Expand All @@ -63,7 +64,7 @@ def server_status(url)

# Ruby Selenium is tagged one version ahead of release to support nightly gem
def current_version
selenium_version = Gem::Version.new(Selenium::WebDriver::VERSION).segments
selenium_version = Gem::Version.new(Selenium::WebDriver::VERSION).segments.take(3)
selenium_version[1] = selenium_version[1] - 1
selenium_version.join('.')
end
Expand Down
16 changes: 8 additions & 8 deletions rb/spec/integration/selenium/webdriver/action_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,19 +290,19 @@ module WebDriver
move_by = properties(moves[1])
up = properties(driver.find_element(class: 'pointerup'))

expect(move_to).to include('button' => '-1',
'pageX' => (x_val + 5).to_s,
'pageY' => (y_val + 5).floor.to_s)
expect(move_to).to include('button' => '-1')
expect(move_to['pageX'].to_f).to be_within(1).of(x_val + 5)
expect(move_to['pageY'].to_f).to be_within(1).of(y_val + 5)
expect(down).to include('button' => '0')
expect(move_by).to include('button' => '-1',
'pageX' => (x_val + 5 + 2).to_s,
'pageY' => (y_val + 5 + 2).floor.to_s,
'tiltX' => '-40',
'tiltY' => '-10',
'twist' => '177')
expect(up).to include('button' => '0',
'pageX' => (x_val + 5 + 2).to_s,
'pageY' => (y_val + 5 + 2).floor.to_s)
expect(move_by['pageX'].to_f).to be_within(1).of(x_val + 5 + 2)
expect(move_by['pageY'].to_f).to be_within(1).of(y_val + 5 + 2)
expect(up).to include('button' => '0')
expect(up['pageX'].to_f).to be_within(1).of(x_val + 5 + 2)
expect(up['pageY'].to_f).to be_within(1).of(y_val + 5 + 2)
end
end

Expand Down
4 changes: 3 additions & 1 deletion rb/spec/integration/selenium/webdriver/driver_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ module WebDriver
driver.navigate.to url_for('relative_locators.html')

above = driver.find_elements(relative: {css: 'td', above: {id: 'center'}})
expect(above.map { |e| e.attribute('id') }).to eq(%w[top topLeft topRight])
ids = above.map { |e| e.attribute('id') }
expect(ids.first).to eq('top')
expect(ids.drop(1)).to contain_exactly('topLeft', 'topRight')
end

it 'finds below element' do
Expand Down
2 changes: 2 additions & 0 deletions rb/spec/integration/selenium/webdriver/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ def example_finished(notification)
c.include(WebDriver::SpecSupport::Helpers)

c.before(:suite) do
# Unit specs load WebMock; integration specs need real HTTP for drivers and downloads.
WebMock.allow_net_connect! if defined?(WebMock)
GlobalTestEnv.ensure_grid if GlobalTestEnv.driver == :remote && ENV['WD_REMOTE_URL'].nil?
GlobalTestEnv.print_env
end
Expand Down
4 changes: 2 additions & 2 deletions rb/spec/unit/selenium/webdriver/remote/capabilities_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ module WebDriver
module Remote
describe Capabilities do
it 'converts noProxy from string to array' do
proxy = Proxy.new(no_proxy: 'proxy_url, localhost')
proxy = Proxy.new(no_proxy: ' proxy_url,localhost, 127.0.0.1 ')
caps = described_class.new(proxy: proxy)
expect(caps.as_json['proxy']['noProxy']).to eq(%w[proxy_url localhost])
expect(caps.as_json['proxy']['noProxy']).to eq(%w[proxy_url localhost 127.0.0.1])
end

it 'does not convert noProxy if it is already array' do
Expand Down
36 changes: 36 additions & 0 deletions rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,28 @@ module Http
expect { client.send :http }.to raise_error(Error::WebDriverError)
end

it 'supports duck-typed proxies with http only' do
client.proxy = Struct.new(:http).new('http://proxy.org:8080')

http = client.send :http
expect(http).to be_proxy
expect(http.proxy_address).to eq('proxy.org')
end

it 'supports duck-typed proxies without no_proxy_list' do
client.proxy = Struct.new(:http, :no_proxy).new('http://proxy.org:8080', 'foo.com, example.com')

http = client.send :http
expect(http).not_to be_proxy
end

it 'supports duck-typed proxies with no_proxy arrays' do
client.proxy = Struct.new(:http, :no_proxy).new('http://proxy.org:8080', %w[foo.com example.com])

http = client.send :http
expect(http).not_to be_proxy
end

%w[http_proxy HTTP_PROXY].each do |proxy_var|
it "honors the #{proxy_var} environment variable" do
with_env(proxy_var => 'http://proxy.org:8080') do
Expand Down Expand Up @@ -115,6 +137,20 @@ module Http
end
end

it "trims entries in #{no_proxy_var}" do
with_env('http_proxy' => 'proxy.org:8080', no_proxy_var => 'foo.com, example.com') do
http = client.send :http
expect(http).not_to be_proxy
end
end

it "trims a single entry in #{no_proxy_var}" do
with_env('http_proxy' => 'proxy.org:8080', no_proxy_var => ' example.com ') do
http = client.send :http
expect(http).not_to be_proxy
end
end

it "understands subnetting in #{no_proxy_var}" do
with_env('http_proxy' => 'proxy.org:8080', no_proxy_var => 'localhost,127.0.0.0/8') do
client.server_url = URI.parse('http://127.0.0.1:4444/wd/hub')
Expand Down
Loading