From dcd2e8968ed1057c3dab993c6d16b71d824c0db7 Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Mon, 15 Jun 2026 17:19:12 -0500 Subject: [PATCH 1/2] [rb] add ClientConfig for HTTP client customization --- rb/.rubocop.yml | 3 + rb/lib/selenium/webdriver/chrome/driver.rb | 6 +- rb/lib/selenium/webdriver/common.rb | 1 + .../webdriver/common/client_config.rb | 88 +++++++++++++++++++ rb/lib/selenium/webdriver/common/driver.rb | 4 +- .../selenium/webdriver/common/local_driver.rb | 17 +++- rb/lib/selenium/webdriver/edge/driver.rb | 6 +- rb/lib/selenium/webdriver/firefox/driver.rb | 6 +- rb/lib/selenium/webdriver/ie/driver.rb | 6 +- rb/lib/selenium/webdriver/remote/bridge.rb | 13 +-- rb/lib/selenium/webdriver/remote/driver.rb | 9 +- .../selenium/webdriver/remote/http/common.rb | 37 +++++--- .../selenium/webdriver/remote/http/default.rb | 69 +++++++++------ rb/lib/selenium/webdriver/safari/driver.rb | 6 +- .../lib/selenium/webdriver/chrome/driver.rbs | 2 +- .../webdriver/common/client_config.rbs | 36 ++++++++ .../lib/selenium/webdriver/common/driver.rbs | 2 +- .../webdriver/common/local_driver.rbs | 4 +- rb/sig/lib/selenium/webdriver/edge/driver.rbs | 2 +- .../lib/selenium/webdriver/firefox/driver.rbs | 2 +- rb/sig/lib/selenium/webdriver/ie/driver.rbs | 2 +- .../lib/selenium/webdriver/remote/bridge.rbs | 2 +- .../lib/selenium/webdriver/remote/driver.rbs | 2 +- .../selenium/webdriver/remote/http/common.rbs | 18 ++-- .../webdriver/remote/http/default.rbs | 20 ++--- .../lib/selenium/webdriver/safari/driver.rbs | 2 +- .../selenium/webdriver/chrome/driver_spec.rb | 13 +++ .../selenium/webdriver/chrome/service_spec.rb | 2 +- .../selenium/webdriver/edge/service_spec.rb | 2 +- .../webdriver/firefox/service_spec.rb | 2 +- .../selenium/webdriver/ie/service_spec.rb | 2 +- .../selenium/webdriver/remote/bridge_spec.rb | 22 ++--- .../selenium/webdriver/remote/driver_spec.rb | 25 ++++++ .../webdriver/remote/features_spec.rb | 6 +- .../webdriver/remote/http/common_spec.rb | 36 +++++--- .../webdriver/remote/http/default_spec.rb | 67 +++++++++++++- .../selenium/webdriver/safari/service_spec.rb | 2 +- 37 files changed, 416 insertions(+), 128 deletions(-) create mode 100644 rb/lib/selenium/webdriver/common/client_config.rb create mode 100644 rb/sig/lib/selenium/webdriver/common/client_config.rbs diff --git a/rb/.rubocop.yml b/rb/.rubocop.yml index 358226b915ff7..44009c1966ae6 100644 --- a/rb/.rubocop.yml +++ b/rb/.rubocop.yml @@ -71,6 +71,9 @@ Metrics/PerceivedComplexity: Exclude: - '../rake_tasks/**/*' +Metrics/ParameterLists: + CountKeywordArgs: false + Naming/BlockForwarding: EnforcedStyle: explicit diff --git a/rb/lib/selenium/webdriver/chrome/driver.rb b/rb/lib/selenium/webdriver/chrome/driver.rb index 5731d1deaba1b..d9cab81ed88c4 100644 --- a/rb/lib/selenium/webdriver/chrome/driver.rb +++ b/rb/lib/selenium/webdriver/chrome/driver.rb @@ -30,9 +30,9 @@ module Chrome class Driver < Chromium::Driver include LocalDriver - def initialize(options: nil, service: nil, url: nil, **) - initialize_local_driver(options, service, url) do |caps, driver_url| - super(caps: caps, url: driver_url, **) + def initialize(options: nil, service: nil, url: nil, http_client: nil, client_config: nil, **) + initialize_local_driver(options, service, url, http_client, client_config) do |caps, client| + super(caps: caps, http_client: client, **) end end diff --git a/rb/lib/selenium/webdriver/common.rb b/rb/lib/selenium/webdriver/common.rb index 7a04e8a837a72..cc5df02d92be4 100644 --- a/rb/lib/selenium/webdriver/common.rb +++ b/rb/lib/selenium/webdriver/common.rb @@ -18,6 +18,7 @@ # under the License. require 'selenium/webdriver/common/error' +require 'selenium/webdriver/common/client_config' require 'selenium/webdriver/common/local_driver' require 'selenium/webdriver/common/driver_finder' require 'selenium/webdriver/common/platform' diff --git a/rb/lib/selenium/webdriver/common/client_config.rb b/rb/lib/selenium/webdriver/common/client_config.rb new file mode 100644 index 0000000000000..635aafedc91f1 --- /dev/null +++ b/rb/lib/selenium/webdriver/common/client_config.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +# Licensed to the Software Freedom Conservancy (SFC) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The SFC licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +require 'uri' + +module Selenium + module WebDriver + # + # Configuration for HTTP clients. + # + + class ClientConfig + class << self + attr_accessor :default_extra_headers + attr_writer :default_user_agent + + def default_user_agent + @default_user_agent ||= "selenium/#{WebDriver::VERSION} (ruby #{Platform.os})" + end + end + + attr_accessor :open_timeout, :read_timeout, :max_redirects, :proxy + attr_writer :extra_headers, :user_agent + attr_reader :server_url + + def initialize(open_timeout: 60, + read_timeout: 120, + max_redirects: 20, + proxy: nil, + extra_headers: nil, + user_agent: nil, + server_url: nil) + @open_timeout = open_timeout + @read_timeout = read_timeout + @max_redirects = max_redirects + @proxy = proxy || proxy_from_environment + @extra_headers = extra_headers + @user_agent = user_agent + self.server_url = server_url + end + + def user_agent + @user_agent || self.class.default_user_agent + end + + def extra_headers + @extra_headers || self.class.default_extra_headers + end + + def server_url=(url) + if url.nil? + @server_url = nil + else + url = url.to_s + url += '/' unless url.end_with?('/') + @server_url = URI.parse(url) + end + end + + private + + def proxy_from_environment + proxy = ENV.fetch('http_proxy', nil) || ENV.fetch('HTTP_PROXY', nil) + return unless proxy + + no_proxy = ENV.fetch('no_proxy', nil) || ENV.fetch('NO_PROXY', nil) + proxy = "http://#{proxy}" unless proxy.start_with?('http://') + Proxy.new(http: proxy, no_proxy: no_proxy) + end + end + end +end diff --git a/rb/lib/selenium/webdriver/common/driver.rb b/rb/lib/selenium/webdriver/common/driver.rb index 444b40410f148..568ba48dd6668 100644 --- a/rb/lib/selenium/webdriver/common/driver.rb +++ b/rb/lib/selenium/webdriver/common/driver.rb @@ -319,9 +319,9 @@ def ref attr_reader :bridge - def create_bridge(caps:, url:, http_client: nil) + def create_bridge(caps:, http_client:) klass = caps['webSocketUrl'] ? Remote::BiDiBridge : Remote::Bridge - klass.new(http_client: http_client, url: url).tap do |bridge| + klass.new(http_client: http_client).tap do |bridge| bridge.create_session(caps) end end diff --git a/rb/lib/selenium/webdriver/common/local_driver.rb b/rb/lib/selenium/webdriver/common/local_driver.rb index f9cbb8773093b..f48d18ac136d0 100644 --- a/rb/lib/selenium/webdriver/common/local_driver.rb +++ b/rb/lib/selenium/webdriver/common/local_driver.rb @@ -20,21 +20,30 @@ module Selenium module WebDriver module LocalDriver - def initialize_local_driver(options, service, url) - raise ArgumentError, "Can't initialize #{self.class} with :url" if url + def initialize_local_driver(options, service, url, http_client, client_config) + assert_local_arguments(url, http_client, client_config) service ||= Service.send(browser) caps = process_options(options, service) - url = service_url(service) + http_client ||= Remote::Http::Default.new(client_config: client_config) + http_client.server_url = service_url(service) begin - yield(caps, url) if block_given? + yield(caps, http_client) if block_given? rescue Selenium::WebDriver::Error::WebDriverError @service_manager&.stop raise end end + def assert_local_arguments(url, http_client, client_config) + if url || client_config&.server_url + raise ArgumentError, "Can't set the server URL for #{self.class}; the service provides it" + elsif http_client && client_config + raise Error::WebDriverError, 'Cannot use both :http_client and :client_config' + end + end + def service_url(service) @service_manager = service.launch @service_manager.uri diff --git a/rb/lib/selenium/webdriver/edge/driver.rb b/rb/lib/selenium/webdriver/edge/driver.rb index ba47b997f97d0..2bd19193875a4 100644 --- a/rb/lib/selenium/webdriver/edge/driver.rb +++ b/rb/lib/selenium/webdriver/edge/driver.rb @@ -30,9 +30,9 @@ module Edge class Driver < Chromium::Driver include LocalDriver - def initialize(options: nil, service: nil, url: nil, **) - initialize_local_driver(options, service, url) do |caps, driver_url| - super(caps: caps, url: driver_url, **) + def initialize(options: nil, service: nil, url: nil, http_client: nil, client_config: nil, **) + initialize_local_driver(options, service, url, http_client, client_config) do |caps, client| + super(caps: caps, http_client: client, **) end end diff --git a/rb/lib/selenium/webdriver/firefox/driver.rb b/rb/lib/selenium/webdriver/firefox/driver.rb index d12f7f6b68a77..3c2afa1d82848 100644 --- a/rb/lib/selenium/webdriver/firefox/driver.rb +++ b/rb/lib/selenium/webdriver/firefox/driver.rb @@ -36,9 +36,9 @@ class Driver < WebDriver::Driver include LocalDriver - def initialize(options: nil, service: nil, url: nil, **) - initialize_local_driver(options, service, url) do |caps, driver_url| - super(caps: caps, url: driver_url, **) + def initialize(options: nil, service: nil, url: nil, http_client: nil, client_config: nil, **) + initialize_local_driver(options, service, url, http_client, client_config) do |caps, client| + super(caps: caps, http_client: client, **) end end diff --git a/rb/lib/selenium/webdriver/ie/driver.rb b/rb/lib/selenium/webdriver/ie/driver.rb index 7dd86a25f3a79..3718c21b86b7d 100644 --- a/rb/lib/selenium/webdriver/ie/driver.rb +++ b/rb/lib/selenium/webdriver/ie/driver.rb @@ -31,9 +31,9 @@ class Driver < WebDriver::Driver include LocalDriver - def initialize(options: nil, service: nil, url: nil, **) - initialize_local_driver(options, service, url) do |caps, driver_url| - super(caps: caps, url: driver_url, **) + def initialize(options: nil, service: nil, url: nil, http_client: nil, client_config: nil, **) + initialize_local_driver(options, service, url, http_client, client_config) do |caps, client| + super(caps: caps, http_client: client, **) end end diff --git a/rb/lib/selenium/webdriver/remote/bridge.rb b/rb/lib/selenium/webdriver/remote/bridge.rb index ca35dc42ab89c..7db5d3b759386 100644 --- a/rb/lib/selenium/webdriver/remote/bridge.rb +++ b/rb/lib/selenium/webdriver/remote/bridge.rb @@ -51,20 +51,13 @@ def element_class end # - # Initializes the bridge with the given server URL - # @param [String, URI] url url for the remote server - # @param [Object] http_client an HTTP client instance that implements the same protocol as Http::Default + # @param [Object] http_client a configured HTTP client implementing the same protocol as Http::Default # @api private # - def initialize(url:, http_client: nil) - uri = url.is_a?(URI) ? url : URI.parse(url) - uri.path += '/' unless uri.path.end_with?('/') - - @http = http_client || Http::Default.new - @http.server_url = uri + def initialize(http_client:) + @http = http_client @file_detector = nil - @locator_converter = self.class.locator_converter end diff --git a/rb/lib/selenium/webdriver/remote/driver.rb b/rb/lib/selenium/webdriver/remote/driver.rb index c74177b016005..ebf2fc0ce6fb3 100644 --- a/rb/lib/selenium/webdriver/remote/driver.rb +++ b/rb/lib/selenium/webdriver/remote/driver.rb @@ -31,12 +31,15 @@ class Driver < WebDriver::Driver include DriverExtensions::HasFileDownloads include DriverExtensions::HasSessionEvents - def initialize(capabilities: nil, options: nil, service: nil, url: nil, **) + def initialize(capabilities: nil, options: nil, service: nil, url: nil, http_client: nil, client_config: nil, + **) raise ArgumentError, "Can not set :service object on #{self.class}" if service + raise Error::WebDriverError, 'Cannot use both :http_client and :client_config' if http_client && client_config - url ||= "http://#{Platform.localhost}:4444/wd/hub" caps = process_options(options, capabilities) - super(caps: caps, url: url, **) + http_client ||= Remote::Http::Default.new(client_config: client_config) + http_client.server_url = url || client_config&.server_url || "http://#{Platform.localhost}:4444/wd/hub" + super(caps: caps, http_client: http_client, **) @bridge.file_detector = ->((filename, *)) { File.exist?(filename) && filename.to_s } command_list = @bridge.command_list @bridge.extend(WebDriver::Remote::Features) diff --git a/rb/lib/selenium/webdriver/remote/http/common.rb b/rb/lib/selenium/webdriver/remote/http/common.rb index 6e4f387ebdb08..1491fda03038e 100644 --- a/rb/lib/selenium/webdriver/remote/http/common.rb +++ b/rb/lib/selenium/webdriver/remote/http/common.rb @@ -31,15 +31,32 @@ class Common BINARY_ENCODINGS = [Encoding::BINARY, Encoding::ASCII_8BIT].freeze class << self - attr_accessor :extra_headers - attr_writer :user_agent - def user_agent - @user_agent ||= "selenium/#{WebDriver::VERSION} (ruby #{Platform.os})" + ClientConfig.default_user_agent + end + + def user_agent=(value) + ClientConfig.default_user_agent = value + end + + def extra_headers + ClientConfig.default_extra_headers + end + + def extra_headers=(value) + ClientConfig.default_extra_headers = value end end - attr_writer :server_url + attr_reader :client_config + + def initialize(client_config: nil) + @client_config = client_config || ClientConfig.new + end + + def server_url=(url) + client_config.server_url = url + end def quit_errors [IOError] @@ -76,17 +93,13 @@ def call(verb, url, command_hash) def common_headers @common_headers ||= begin headers = DEFAULT_HEADERS.dup - headers['User-Agent'] = Common.user_agent - headers = headers.merge(Common.extra_headers || {}) - - headers + headers['User-Agent'] = client_config.user_agent + headers.merge(client_config.extra_headers || {}) end end def server_url - return @server_url if @server_url - - raise Error::WebDriverError, 'server_url not set' + client_config.server_url || raise(Error::WebDriverError, 'server_url not set') end def request(*) diff --git a/rb/lib/selenium/webdriver/remote/http/default.rb b/rb/lib/selenium/webdriver/remote/http/default.rb index 2677eefc2de15..57b4bdbc72938 100644 --- a/rb/lib/selenium/webdriver/remote/http/default.rb +++ b/rb/lib/selenium/webdriver/remote/http/default.rb @@ -24,19 +24,41 @@ module Remote module Http # @api private class Default < Common - attr_writer :proxy - - attr_accessor :open_timeout, :read_timeout - # Initializes object. # Warning: Setting {#open_timeout} to non-nil values will cause a separate thread to spawn. # Debuggers that freeze the process will not be able to evaluate any operations if that happens. + # @param [ClientConfig] client_config - Configuration used to build the HTTP client. # @param [Numeric] open_timeout - Open timeout to apply to HTTP client. # @param [Numeric] read_timeout - Read timeout (seconds) to apply to HTTP client. - def initialize(open_timeout: nil, read_timeout: nil) - @open_timeout = open_timeout - @read_timeout = read_timeout - super() + def initialize(client_config: nil, open_timeout: nil, read_timeout: nil) + if client_config && (open_timeout || read_timeout) + raise Error::WebDriverError, 'Cannot use both :client_config and :open_timeout/:read_timeout' + end + + client_config ||= ClientConfig.new + client_config.open_timeout = open_timeout if open_timeout + client_config.read_timeout = read_timeout if read_timeout + super(client_config: client_config) + end + + def open_timeout + client_config.open_timeout + end + + def read_timeout + client_config.read_timeout + end + + def open_timeout=(value) + client_config.open_timeout = value + end + + def read_timeout=(value) + client_config.read_timeout = value + end + + def proxy=(value) + client_config.proxy = value end def close @@ -94,16 +116,20 @@ def request(verb, url, headers, payload, redirects = 0) end if response.is_a? Net::HTTPRedirection - WebDriver.logger.debug("Redirect to #{response['Location']}; times: #{redirects}") - raise Error::WebDriverError, 'too many redirects' if redirects >= MAX_REDIRECTS - - request(:get, URI.parse(response['Location']), DEFAULT_HEADERS.dup, nil, redirects + 1) + follow_redirect(response, redirects) else WebDriver.logger.debug(" <<< #{response.instance_variable_get(:@header).inspect}", id: :header) create_response response.code, response.body, response.content_type end end + def follow_redirect(response, redirects) + WebDriver.logger.debug("Redirect to #{response['Location']}; times: #{redirects}") + raise Error::WebDriverError, 'too many redirects' if redirects >= client_config.max_redirects + + request(:get, URI.parse(response['Location']), DEFAULT_HEADERS.dup, nil, redirects + 1) + end + def new_request_for(verb, url, headers, payload) req = Net::HTTP.const_get(verb.to_s.capitalize).new(url.path, headers) @@ -120,30 +146,23 @@ def response_for(request) def new_http_client if use_proxy? - url = @proxy.http + url = proxy.http unless proxy.respond_to?(:http) && url raise Error::WebDriverError, - "expected HTTP proxy, got #{@proxy.inspect}" + "expected HTTP proxy, got #{proxy.inspect}" end - proxy = URI.parse(url) + proxy_uri = URI.parse(url) - Net::HTTP.new(server_url.host, server_url.port, proxy.host, proxy.port, proxy.user, proxy.password) + Net::HTTP.new(server_url.host, server_url.port, + proxy_uri.host, proxy_uri.port, proxy_uri.user, proxy_uri.password) else Net::HTTP.new server_url.host, server_url.port end end def proxy - @proxy ||= begin - proxy = ENV.fetch('http_proxy', nil) || ENV.fetch('HTTP_PROXY', nil) - no_proxy = ENV.fetch('no_proxy', nil) || ENV.fetch('NO_PROXY', nil) - - if proxy - proxy = "http://#{proxy}" unless proxy.start_with?('http://') - Proxy.new(http: proxy, no_proxy: no_proxy) - end - end + client_config.proxy end def use_proxy? diff --git a/rb/lib/selenium/webdriver/safari/driver.rb b/rb/lib/selenium/webdriver/safari/driver.rb index 83cedefb73645..ad121ddfc05a7 100644 --- a/rb/lib/selenium/webdriver/safari/driver.rb +++ b/rb/lib/selenium/webdriver/safari/driver.rb @@ -31,9 +31,9 @@ class Driver < WebDriver::Driver include LocalDriver - def initialize(options: nil, service: nil, url: nil, **) - initialize_local_driver(options, service, url) do |caps, driver_url| - super(caps: caps, url: driver_url, **) + def initialize(options: nil, service: nil, url: nil, http_client: nil, client_config: nil, **) + initialize_local_driver(options, service, url, http_client, client_config) do |caps, client| + super(caps: caps, http_client: client, **) end end diff --git a/rb/sig/lib/selenium/webdriver/chrome/driver.rbs b/rb/sig/lib/selenium/webdriver/chrome/driver.rbs index 92653102f9a90..7612d20fc27f3 100644 --- a/rb/sig/lib/selenium/webdriver/chrome/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/chrome/driver.rbs @@ -4,7 +4,7 @@ module Selenium class Driver < Chromium::Driver include LocalDriver - def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void def browser: () -> Symbol diff --git a/rb/sig/lib/selenium/webdriver/common/client_config.rbs b/rb/sig/lib/selenium/webdriver/common/client_config.rbs new file mode 100644 index 0000000000000..5d2f4dff3847a --- /dev/null +++ b/rb/sig/lib/selenium/webdriver/common/client_config.rbs @@ -0,0 +1,36 @@ +module Selenium + module WebDriver + + class ClientConfig + def self.default_user_agent: -> String + def self.default_user_agent=: (String? value) -> void + attr_accessor self.default_extra_headers: Hash[String, String]? + + attr_accessor open_timeout: Integer? + attr_accessor read_timeout: Integer? + attr_accessor max_redirects: Integer? + attr_accessor proxy: Selenium::WebDriver::Proxy? + def extra_headers: -> Hash[String, String]? + attr_writer extra_headers: Hash[String, String]? + def user_agent: -> String + attr_writer user_agent: String? + attr_reader server_url: URI? + + def server_url=: ((String | URI)? url) -> void + + def initialize: ( + ?open_timeout: Integer?, + ?read_timeout: Integer?, + ?max_redirects: Integer?, + ?proxy: Selenium::WebDriver::Proxy?, + ?extra_headers: Hash[String, String]?, + ?user_agent: String?, + ?server_url: (String | URI)? + ) -> void + + private + + def proxy_from_environment: () -> Selenium::WebDriver::Proxy? + end + end +end diff --git a/rb/sig/lib/selenium/webdriver/common/driver.rbs b/rb/sig/lib/selenium/webdriver/common/driver.rbs index 8a95acc35109a..6854f77d8b131 100644 --- a/rb/sig/lib/selenium/webdriver/common/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/common/driver.rbs @@ -69,7 +69,7 @@ module Selenium attr_reader bridge: untyped - def create_bridge: (caps: untyped, url: untyped, ?http_client: untyped) -> untyped + def create_bridge: (caps: Remote::Capabilities, http_client: Remote::Http::Common) -> Remote::Bridge def service_url: (untyped service) -> untyped diff --git a/rb/sig/lib/selenium/webdriver/common/local_driver.rbs b/rb/sig/lib/selenium/webdriver/common/local_driver.rbs index ae8e719f13813..ba133577f403b 100644 --- a/rb/sig/lib/selenium/webdriver/common/local_driver.rbs +++ b/rb/sig/lib/selenium/webdriver/common/local_driver.rbs @@ -3,7 +3,9 @@ module Selenium module LocalDriver include _Driver - def initialize_local_driver: (untyped options, untyped service, untyped url) ?{ (untyped, untyped) -> void } -> void + def initialize_local_driver: (untyped options, untyped service, untyped url, untyped http_client, ClientConfig? client_config) ?{ (untyped, Remote::Http::Common) -> void } -> void + + def assert_local_arguments: (untyped url, untyped http_client, ClientConfig? client_config) -> void def process_options: (untyped options, untyped service) -> untyped end diff --git a/rb/sig/lib/selenium/webdriver/edge/driver.rbs b/rb/sig/lib/selenium/webdriver/edge/driver.rbs index 3f0886323a0ab..774829ca56cc1 100644 --- a/rb/sig/lib/selenium/webdriver/edge/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/edge/driver.rbs @@ -4,7 +4,7 @@ module Selenium class Driver < Chromium::Driver include LocalDriver - def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void def browser: () -> :edge diff --git a/rb/sig/lib/selenium/webdriver/firefox/driver.rbs b/rb/sig/lib/selenium/webdriver/firefox/driver.rbs index 06484a1f13cfa..7af098d16739d 100644 --- a/rb/sig/lib/selenium/webdriver/firefox/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/firefox/driver.rbs @@ -6,7 +6,7 @@ module Selenium include LocalDriver - def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void def browser: () -> Symbol end diff --git a/rb/sig/lib/selenium/webdriver/ie/driver.rbs b/rb/sig/lib/selenium/webdriver/ie/driver.rbs index 29b8247beed03..d938231cd9545 100644 --- a/rb/sig/lib/selenium/webdriver/ie/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/ie/driver.rbs @@ -6,7 +6,7 @@ module Selenium include LocalDriver - def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void def browser: () -> Symbol end diff --git a/rb/sig/lib/selenium/webdriver/remote/bridge.rbs b/rb/sig/lib/selenium/webdriver/remote/bridge.rbs index 5089913bfb60e..1a63a7d70031e 100644 --- a/rb/sig/lib/selenium/webdriver/remote/bridge.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/bridge.rbs @@ -30,7 +30,7 @@ module Selenium attr_reader capabilities: untyped - def initialize: (url: String | URI, ?http_client: untyped?) -> void + def initialize: (http_client: untyped) -> void def bidi: -> BiDi diff --git a/rb/sig/lib/selenium/webdriver/remote/driver.rbs b/rb/sig/lib/selenium/webdriver/remote/driver.rbs index 09a7f48693a3e..ff740f8432820 100644 --- a/rb/sig/lib/selenium/webdriver/remote/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/driver.rbs @@ -8,7 +8,7 @@ module Selenium include DriverExtensions::HasFileDownloads - def initialize: (?capabilities: untyped?, ?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?capabilities: untyped?, ?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void private diff --git a/rb/sig/lib/selenium/webdriver/remote/http/common.rbs b/rb/sig/lib/selenium/webdriver/remote/http/common.rbs index ac400c63d6f11..d20308971c825 100644 --- a/rb/sig/lib/selenium/webdriver/remote/http/common.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/http/common.rbs @@ -13,13 +13,21 @@ module Selenium @common_headers: Hash[String, untyped] - attr_accessor self.extra_headers: Hash[String, untyped] - - attr_writer self.user_agent: String + @client_config: ClientConfig def self.user_agent: -> String - attr_writer server_url: String + def self.user_agent=: (String? value) -> void + + def self.extra_headers: -> Hash[String, untyped]? + + def self.extra_headers=: (Hash[String, untyped]? value) -> void + + attr_reader client_config: ClientConfig + + def initialize: (?client_config: ClientConfig?) -> void + + def server_url=: ((String | URI)? url) -> void def quit_errors: () -> Array[untyped] @@ -31,7 +39,7 @@ module Selenium def common_headers: -> Hash[String, untyped] - def server_url: () -> String + def server_url: () -> URI def request: (*untyped) -> untyped diff --git a/rb/sig/lib/selenium/webdriver/remote/http/default.rbs b/rb/sig/lib/selenium/webdriver/remote/http/default.rbs index 7fb7a41c9ac7a..941fcb94813ea 100644 --- a/rb/sig/lib/selenium/webdriver/remote/http/default.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/http/default.rbs @@ -4,21 +4,19 @@ module Selenium module Http # @api private class Default < Common - @open_timeout: untyped - - @read_timeout: untyped - @http: untyped - @proxy: untyped + def initialize: (?client_config: ClientConfig?, ?open_timeout: Numeric?, ?read_timeout: Numeric?) -> void - attr_writer proxy: untyped + def open_timeout: () -> Numeric? - attr_accessor open_timeout: untyped + def read_timeout: () -> Numeric? - attr_accessor read_timeout: untyped + def open_timeout=: (Numeric? value) -> void - def initialize: (?open_timeout: untyped?, ?read_timeout: untyped?) -> void + def read_timeout=: (Numeric? value) -> void + + def proxy=: (Proxy? value) -> void def close: () -> untyped @@ -32,6 +30,8 @@ module Selenium def request: (untyped verb, untyped url, untyped headers, untyped payload, ?::Integer redirects) -> untyped + def follow_redirect: (untyped response, ::Integer redirects) -> untyped + def new_request_for: (untyped verb, untyped url, untyped headers, untyped payload) -> untyped def response_for: (untyped request) -> untyped @@ -40,7 +40,7 @@ module Selenium def proxy: () -> untyped - def use_proxy?: () -> untyped + def use_proxy?: () -> bool end end end diff --git a/rb/sig/lib/selenium/webdriver/safari/driver.rbs b/rb/sig/lib/selenium/webdriver/safari/driver.rbs index 5d26b33854dc2..2edf45b86c930 100644 --- a/rb/sig/lib/selenium/webdriver/safari/driver.rbs +++ b/rb/sig/lib/selenium/webdriver/safari/driver.rbs @@ -6,7 +6,7 @@ module Selenium include LocalDriver - def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, **untyped opts) -> void + def initialize: (?options: untyped?, ?service: untyped?, ?url: untyped?, ?http_client: untyped?, ?client_config: ClientConfig?, **untyped opts) -> void def browser: () -> Symbol end diff --git a/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb b/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb index 798365f131e50..1c08bd68ebb0e 100644 --- a/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/chrome/driver_spec.rb @@ -92,6 +92,19 @@ def expect_request(body: nil, endpoint: nil) described_class.new(options: Options.firefox) }.to raise_exception(ArgumentError, ':options must be an instance of Selenium::WebDriver::Chrome::Options') end + + it 'does not accept a server URL set on the client_config' do + config = WebDriver::ClientConfig.new(server_url: 'http://example.com') + expect { + described_class.new(client_config: config) + }.to raise_exception(ArgumentError, /Can't set the server URL/) + end + + it 'rejects both http_client and client_config' do + expect { + described_class.new(http_client: Remote::Http::Default.new, client_config: WebDriver::ClientConfig.new) + }.to raise_exception(Error::WebDriverError, /cannot use both/i) + end end end # Chrome end # WebDriver diff --git a/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb b/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb index c5d82d1a276f5..9e9e3e65b697e 100644 --- a/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/chrome/service_spec.rb @@ -131,7 +131,7 @@ module Chrome it 'errors when :url is provided' do expect { driver.new(url: 'http://example.com:4321') - }.to raise_error(ArgumentError, "Can't initialize Selenium::WebDriver::Chrome::Driver with :url") + }.to raise_error(ArgumentError, /Can't set the server URL for/) end it 'is created when :url is not provided' do diff --git a/rb/spec/unit/selenium/webdriver/edge/service_spec.rb b/rb/spec/unit/selenium/webdriver/edge/service_spec.rb index 00d713e3b13f2..9d5a7346cac43 100644 --- a/rb/spec/unit/selenium/webdriver/edge/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/edge/service_spec.rb @@ -132,7 +132,7 @@ module Edge expect { driver.new(url: 'http://example.com:4321') - }.to raise_error(ArgumentError, "Can't initialize Selenium::WebDriver::Edge::Driver with :url") + }.to raise_error(ArgumentError, /Can't set the server URL for/) expect(described_class).not_to have_received(:new) end diff --git a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb index d667230ca60eb..fad58519c3153 100644 --- a/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/firefox/service_spec.rb @@ -191,7 +191,7 @@ module Firefox it 'is not created when :url is provided' do expect { driver.new(url: 'http://example.com:4321') - }.to raise_error(ArgumentError, "Can't initialize Selenium::WebDriver::Firefox::Driver with :url") + }.to raise_error(ArgumentError, /Can't set the server URL for/) end it 'is created when :url is not provided' do diff --git a/rb/spec/unit/selenium/webdriver/ie/service_spec.rb b/rb/spec/unit/selenium/webdriver/ie/service_spec.rb index a77a5c781ac2d..0d1ae3af4c87d 100644 --- a/rb/spec/unit/selenium/webdriver/ie/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/ie/service_spec.rb @@ -129,7 +129,7 @@ module IE it 'is not created when :url is provided' do expect { driver.new(url: 'http://example.com:4321') - }.to raise_error(ArgumentError, "Can't initialize Selenium::WebDriver::IE::Driver with :url") + }.to raise_error(ArgumentError, /Can't set the server URL for/) end it 'is created when :url is not provided' do diff --git a/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb b/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb index cc8bf707045c3..a1fba2f3ef9a2 100644 --- a/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb @@ -23,10 +23,12 @@ module Selenium module WebDriver module Remote describe Bridge do - describe '.add_command' do - let(:http) { WebDriver::Remote::Http::Default.new } - let(:bridge) { described_class.new(http_client: http, url: 'http://localhost') } + let(:http) do + WebDriver::Remote::Http::Default.new.tap { |client| client.server_url = 'http://localhost' } + end + let(:bridge) { described_class.new(http_client: http) } + describe '.add_command' do before do allow(http).to receive(:request) .with(any_args) @@ -51,15 +53,14 @@ module Remote end describe '#initialize' do - it 'raises ArgumentError if passed invalid options' do - expect { described_class.new(foo: 'bar') }.to raise_error(ArgumentError) + it 'uses the provided http client' do + custom_http = instance_double(Remote::Http::Default) + bridge = described_class.new(http_client: custom_http) + expect(bridge.http).to eq(custom_http) end end describe '#create_session' do - let(:http) { WebDriver::Remote::Http::Default.new } - let(:bridge) { described_class.new(http_client: http, url: 'http://localhost') } - it 'accepts Hash' do payload = JSON.generate( capabilities: { @@ -127,7 +128,6 @@ module Remote describe '#upload' do it 'raises WebDriverError if uploading non-files' do expect { - bridge = described_class.new(url: 'http://localhost') bridge.extend(WebDriver::Remote::Features) bridge.upload('NotAFile') }.to raise_error(Error::WebDriverError) @@ -136,16 +136,12 @@ module Remote describe '#quit' do it 'respects quit_errors' do - bridge = described_class.new(url: 'http://localhost') allow(bridge).to receive(:execute).with(:delete_session).and_raise(IOError) expect { bridge.quit }.not_to raise_error end end describe 'finding elements' do - let(:http) { WebDriver::Remote::Http::Default.new } - let(:bridge) { described_class.new(http_client: http, url: 'http://localhost') } - before do allow(http).to receive(:request) .with(:post, URI('http://localhost/session'), any_args) diff --git a/rb/spec/unit/selenium/webdriver/remote/driver_spec.rb b/rb/spec/unit/selenium/webdriver/remote/driver_spec.rb index 8b625ccf33e3f..48fb69e51c855 100644 --- a/rb/spec/unit/selenium/webdriver/remote/driver_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/driver_spec.rb @@ -55,6 +55,31 @@ def expect_request(body: nil, endpoint: nil) expect(driver.send(:bridge).http).to eq client end + it 'builds an HTTP client from the provided client_config' do + server = 'https://example.com:4646/wd/hub' + expect_request(endpoint: "#{server}/session") + config = ClientConfig.new(server_url: server, read_timeout: 25) + + driver = described_class.new(options: Options.chrome, client_config: config) + expect(driver.send(:bridge).http.read_timeout).to eq(25) + end + + it 'records the resolved server URL on the client_config' do + server = 'https://example.com:4646/wd/hub' + expect_request(endpoint: "#{server}/session") + config = ClientConfig.new(read_timeout: 25) + + described_class.new(options: Options.chrome, url: server, client_config: config) + expect(config.server_url).to eq(URI.parse("#{server}/")) + end + + it 'errors when both http_client and client_config are provided' do + config = ClientConfig.new(server_url: 'http://localhost') + expect { + described_class.new(options: Options.chrome, http_client: Remote::Http::Default.new, client_config: config) + }.to raise_error(Error::WebDriverError, /cannot use both/i) + end + it 'sets a default file detector' do expect_request diff --git a/rb/spec/unit/selenium/webdriver/remote/features_spec.rb b/rb/spec/unit/selenium/webdriver/remote/features_spec.rb index 78ff901fa51cf..532ac99b10b17 100644 --- a/rb/spec/unit/selenium/webdriver/remote/features_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/features_spec.rb @@ -23,8 +23,10 @@ module Selenium module WebDriver module Remote describe Features do - let(:http) { WebDriver::Remote::Http::Default.new } - let(:bridge) { Bridge.new(http_client: http, url: 'http://localhost') } + let(:http) do + WebDriver::Remote::Http::Default.new.tap { |client| client.server_url = 'http://localhost' } + end + let(:bridge) { Bridge.new(http_client: http) } before do allow(http).to receive(:request) diff --git a/rb/spec/unit/selenium/webdriver/remote/http/common_spec.rb b/rb/spec/unit/selenium/webdriver/remote/http/common_spec.rb index 968780a799533..878bf59f25c36 100644 --- a/rb/spec/unit/selenium/webdriver/remote/http/common_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/http/common_spec.rb @@ -24,8 +24,10 @@ module WebDriver module Remote module Http describe Common do + let(:client_config) { ClientConfig.new } + subject(:common) do - common = described_class.new + common = described_class.new(client_config: client_config) common.server_url = URI.parse('http://server') allow(common).to receive(:request) @@ -33,8 +35,8 @@ module Http end after do - described_class.extra_headers = nil - described_class.user_agent = nil + ClientConfig.default_extra_headers = nil + ClientConfig.default_user_agent = nil end it 'sends non-empty body header for POST requests without command data' do @@ -55,17 +57,31 @@ module Http hash_including('User-Agent' => a_string_matching(user_agent_regexp)), '{}') end - it 'allows registering extra headers' do - described_class.extra_headers = {'Foo' => 'bar'} + context 'when the client config registers extra headers' do + let(:client_config) { ClientConfig.new(extra_headers: {'Foo' => 'bar'}) } - common.call(:post, 'session', nil) + it 'sends them' do + common.call(:post, 'session', nil) - expect(common).to have_received(:request) - .with(:post, URI.parse('http://server/session'), - hash_including('Foo' => 'bar'), '{}') + expect(common).to have_received(:request) + .with(:post, URI.parse('http://server/session'), + hash_including('Foo' => 'bar'), '{}') + end + end + + context 'when the client config overrides the User-Agent' do + let(:client_config) { ClientConfig.new(user_agent: 'rspec/1.0 (ruby 3.2)') } + + it 'uses it' do + common.call(:post, 'session', nil) + + expect(common).to have_received(:request) + .with(:post, URI.parse('http://server/session'), + hash_including('User-Agent' => 'rspec/1.0 (ruby 3.2)'), '{}') + end end - it 'allows overriding default User-Agent' do + it 'still honors the Http::Common.user_agent= setter' do described_class.user_agent = 'rspec/1.0 (ruby 3.2)' common.call(:post, 'session', nil) diff --git a/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb b/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb index 21eff54011c37..52d43cee428c8 100644 --- a/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb @@ -31,15 +31,17 @@ module Http client end - it 'assigns default timeout to nil' do + it 'assigns default timeouts' do http = client.send :http expect(http.open_timeout).to eq 60 - expect(http.read_timeout).to eq 60 + expect(http.read_timeout).to eq 120 end describe '#initialize' do - let(:client) { described_class.new(read_timeout: 22, open_timeout: 23) } + let(:client) do + described_class.new(client_config: ClientConfig.new(read_timeout: 22, open_timeout: 23)) + end it 'accepts read timeout options' do expect(client.open_timeout).to eq 23 @@ -48,6 +50,51 @@ module Http it 'accepts open timeout options' do expect(client.read_timeout).to eq 22 end + + it 'still accepts open_timeout/read_timeout directly' do + client = described_class.new(open_timeout: 23, read_timeout: 22) + expect(client.open_timeout).to eq 23 + expect(client.read_timeout).to eq 22 + end + + it 'errors when given both a client_config and timeout options' do + expect { + described_class.new(client_config: ClientConfig.new, open_timeout: 23) + }.to raise_error(Error::WebDriverError, /Cannot use both/) + end + end + + describe 'with a client_config' do + it 'configures the client from the config' do + proxy = instance_double(Proxy) + config = ClientConfig.new( + open_timeout: 15, + read_timeout: 25, + max_redirects: 5, + proxy: proxy, + extra_headers: {'X-Custom-Header' => 'custom_value'}, + user_agent: 'Custom User Agent' + ) + client = described_class.new(client_config: config) + + expect(client.open_timeout).to eq(15) + expect(client.read_timeout).to eq(25) + expect(client.client_config.max_redirects).to eq(5) + expect(client.client_config.proxy).to eq(proxy) + expect(client.client_config.extra_headers).to eq('X-Custom-Header' => 'custom_value') + expect(client.client_config.user_agent).to eq('Custom User Agent') + end + + it 'does not leak config headers onto the global Http::Common' do + config = ClientConfig.new( + extra_headers: {'X-Custom-Header' => 'custom_value'}, + user_agent: 'Custom User Agent' + ) + described_class.new(client_config: config) + + expect(Common.extra_headers).to be_nil + expect(Common.user_agent).to eq("selenium/#{WebDriver::VERSION} (ruby #{Platform.os})") + end end it 'uses the specified proxy' do @@ -135,6 +182,20 @@ module Http }.to raise_error(Errno::ECONNREFUSED, %r{using proxy: http://localhost:1234}) end end + + it 'stops following redirects after the configured max_redirects' do + client = described_class.new(client_config: ClientConfig.new(max_redirects: 2)) + client.server_url = URI.parse('http://example.com') + http = client.send :http + + redirect = Net::HTTPFound.new('1.1', '302', 'Found') + redirect['Location'] = 'http://example.com/next' + allow(http).to receive(:request).and_return(redirect) + + expect { + client.call(:get, 'http://example.com/start', nil) + }.to raise_error(Error::WebDriverError, /too many redirects/) + end end end # Http end # Remote diff --git a/rb/spec/unit/selenium/webdriver/safari/service_spec.rb b/rb/spec/unit/selenium/webdriver/safari/service_spec.rb index 9ae84dba3be74..4ed1c067108fc 100644 --- a/rb/spec/unit/selenium/webdriver/safari/service_spec.rb +++ b/rb/spec/unit/selenium/webdriver/safari/service_spec.rb @@ -94,7 +94,7 @@ module Safari it 'is not created when :url is provided' do expect { driver.new(url: 'http://example.com:4321') - }.to raise_error(ArgumentError, "Can't initialize Selenium::WebDriver::Safari::Driver with :url") + }.to raise_error(ArgumentError, /Can't set the server URL for/) end it 'is created when :url is not provided' do From 8f5d153eda42a39b1aeb5ef5ff35747b472aed2e Mon Sep 17 00:00:00 2001 From: Titus Fortner Date: Sun, 21 Jun 2026 12:48:53 -0500 Subject: [PATCH 2/2] [rb] document ClientConfig with YARD and use a real Proxy in specs --- rb/lib/selenium/webdriver/common/client_config.rb | 9 +++++++++ .../unit/selenium/webdriver/remote/http/default_spec.rb | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/rb/lib/selenium/webdriver/common/client_config.rb b/rb/lib/selenium/webdriver/common/client_config.rb index 635aafedc91f1..42c80a024f237 100644 --- a/rb/lib/selenium/webdriver/common/client_config.rb +++ b/rb/lib/selenium/webdriver/common/client_config.rb @@ -39,6 +39,15 @@ def default_user_agent attr_writer :extra_headers, :user_agent attr_reader :server_url + # + # @param [Numeric] open_timeout Seconds to wait for the connection to open. + # @param [Numeric] read_timeout Seconds to wait for a response. + # @param [Integer] max_redirects Maximum number of redirects to follow. + # @param [Proxy, Hash] proxy Proxy to use for the connection. + # @param [Hash] extra_headers Additional headers to send with each request. + # @param [String] user_agent Value to send as the User-Agent header. + # @param [String, URI] server_url URL of the server to connect to. + # def initialize(open_timeout: 60, read_timeout: 120, max_redirects: 20, diff --git a/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb b/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb index 52d43cee428c8..6523a1e3a5185 100644 --- a/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/http/default_spec.rb @@ -66,7 +66,7 @@ module Http describe 'with a client_config' do it 'configures the client from the config' do - proxy = instance_double(Proxy) + proxy = Proxy.new(http: 'http://proxy.example:8080') config = ClientConfig.new( open_timeout: 15, read_timeout: 25,