From 6f37b9596ade1b971e9c23d7adf78f79f0e2f05e Mon Sep 17 00:00:00 2001 From: titusfortner Date: Wed, 22 Oct 2025 00:14:17 -0500 Subject: [PATCH 1/3] [rb] implement client config class --- rb/lib/selenium/webdriver/common.rb | 1 + .../webdriver/common/client_config.rb | 46 +++++++++++++++ rb/lib/selenium/webdriver/common/driver.rb | 4 +- rb/lib/selenium/webdriver/remote/bridge.rb | 50 ++++++++++++++-- .../selenium/webdriver/remote/http/common.rb | 4 ++ .../webdriver/common/client_config.rbs | 28 +++++++++ .../lib/selenium/webdriver/common/driver.rbs | 2 +- .../lib/selenium/webdriver/remote/bridge.rbs | 10 +++- .../selenium/webdriver/remote/http/common.rbs | 10 ++-- .../webdriver/remote/http/default.rbs | 16 ++--- .../selenium/webdriver/remote/bridge_spec.rb | 59 ++++++++++++++++++- 11 files changed, 206 insertions(+), 24 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/lib/selenium/webdriver/common.rb b/rb/lib/selenium/webdriver/common.rb index 2e00562c1b7b4..952cac0f93ca1 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..2cec130cd403a --- /dev/null +++ b/rb/lib/selenium/webdriver/common/client_config.rb @@ -0,0 +1,46 @@ +# 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. + +module Selenium + module WebDriver + # + # Immutable Configuration for HTTP clients. + # + + ClientConfig = Data.define( + :open_timeout, + :read_timeout, + :proxy, + :extra_headers, + :user_agent, + :server_url + ) do + def initialize( + open_timeout: nil, + read_timeout: nil, + proxy: nil, + extra_headers: nil, + user_agent: nil, + server_url: nil + ) + super + end + end + end +end diff --git a/rb/lib/selenium/webdriver/common/driver.rb b/rb/lib/selenium/webdriver/common/driver.rb index 94a29513e2752..24b2c0330c810 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:, url: nil, http_client: nil, client_config: nil) klass = caps['webSocketUrl'] ? Remote::BiDiBridge : Remote::Bridge - klass.new(http_client: http_client, url: url).tap do |bridge| + klass.new(http_client: http_client, url: url, client_config: client_config).tap do |bridge| bridge.create_session(caps) end end diff --git a/rb/lib/selenium/webdriver/remote/bridge.rb b/rb/lib/selenium/webdriver/remote/bridge.rb index d4318a25b0088..40a1fb6210058 100644 --- a/rb/lib/selenium/webdriver/remote/bridge.rb +++ b/rb/lib/selenium/webdriver/remote/bridge.rb @@ -54,17 +54,18 @@ def element_class # 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 [ClientConfig] client_config configuration for the HTTP client # @api private # - def initialize(url:, http_client: nil) - uri = url.is_a?(URI) ? url : URI.parse(url) - uri.path += '/' unless uri.path.end_with?('/') + def initialize(url: nil, http_client: nil, client_config: nil) + if http_client && client_config + raise Error::WebDriverError, 'Cannot specify both http_client and client_config' + end - @http = http_client || Http::Default.new - @http.server_url = uri - @file_detector = nil + @http = http_client || create_http_client(client_config, url: url) + @file_detector = nil @locator_converter = self.class.locator_converter end @@ -93,6 +94,8 @@ def create_session(capabilities) extend(WebDriver::Safari::Features) when 'internet explorer' extend(WebDriver::IE::Features) + else + raise Error::WebDriverError, "Unknown browser name: #{@capabilities[:browser_name]}" end end @@ -662,6 +665,41 @@ def prepare_capabilities_payload(capabilities) capabilities = {alwaysMatch: capabilities} if !capabilities['alwaysMatch'] && !capabilities['firstMatch'] {capabilities: capabilities} end + + def create_http_client(client_config, url: nil) + http = build_http_client(client_config) + validate_server_url_args(url, client_config) + http.server_url = normalize_url(url || client_config&.server_url) + http + end + + def build_http_client(client_config) + if client_config + http = Http::Default.new(open_timeout: client_config.open_timeout, + read_timeout: client_config.read_timeout) + http.proxy = client_config.proxy if client_config.proxy + + Http::Common.extra_headers = client_config.extra_headers if client_config.extra_headers + Http::Common.user_agent = client_config.user_agent if client_config.user_agent + else + http = Http::Default.new + end + http + end + + def validate_server_url_args(url, client_config) + if url && client_config&.server_url + raise Error::WebDriverError, 'Cannot specify url in both keyword and http_client' + elsif url.nil? && !client_config&.server_url + raise Error::WebDriverError, 'No server URL provided' + end + end + + def normalize_url(url) + url = URI.parse(url) if url && !url.is_a?(URI) + url&.path += '/' unless url&.path&.end_with?('/') + url + end end # Bridge end # Remote end # WebDriver diff --git a/rb/lib/selenium/webdriver/remote/http/common.rb b/rb/lib/selenium/webdriver/remote/http/common.rb index 2f287e2703132..cd1e831dafcdb 100644 --- a/rb/lib/selenium/webdriver/remote/http/common.rb +++ b/rb/lib/selenium/webdriver/remote/http/common.rb @@ -40,6 +40,10 @@ def user_agent attr_writer :server_url + def server_url? + !@server_url.nil? + end + def quit_errors [IOError] end 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..719d4a5f0b0cd --- /dev/null +++ b/rb/sig/lib/selenium/webdriver/common/client_config.rbs @@ -0,0 +1,28 @@ +module Selenium + module WebDriver + + ClientConfig: Class + + class ClientConfig + attr_reader open_timeout: Integer? + attr_reader read_timeout: Integer? + attr_reader proxy: Selenium::WebDriver::Proxy? + attr_reader extra_headers: Hash[String, String]? + attr_reader user_agent: String? + attr_reader server_url: (String | URI)? + + def initialize: ( + ?open_timeout: Integer?, + ?read_timeout: Integer?, + ?proxy: Selenium::WebDriver::Proxy?, + ?extra_headers: Hash[String, String]?, + ?user_agent: String?, + ?server_url: (String | URI)? + ) -> void + + def with: (**untyped) -> ClientConfig + def to_h: -> Hash[Symbol, untyped] + def deconstruct_keys: (Array[Symbol]?) -> Hash[Symbol, untyped] + 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..9d38640686c46 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, url: (String | URI)?, ?http_client: Remote::Http::Common?, ?client_config: ClientConfig?) -> Remote::Bridge def service_url: (untyped service) -> untyped diff --git a/rb/sig/lib/selenium/webdriver/remote/bridge.rbs b/rb/sig/lib/selenium/webdriver/remote/bridge.rbs index dfc248728e82e..9844298b42764 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: (?url: (String | URI)?, ?http_client: untyped?, ?client_config: ClientConfig?) -> void def bidi: -> WebDriver::Error::WebDriverError @@ -248,6 +248,14 @@ module Selenium def convert_locator: (untyped how, untyped what) -> ::Array[untyped] + def create_http_client: (?ClientConfig? client_config, ?url: (String | URI)?) -> Http::Common + + def build_http_client: (ClientConfig? client_config) -> Http::Common + + def validate_server_url_args: ((String | URI)? url, ClientConfig? client_config) -> void + + def normalize_url: ((String | URI)? url) -> URI? + ESCAPE_CSS_REGEXP: ::Regexp UNICODE_CODE_POINT: 30 diff --git a/rb/sig/lib/selenium/webdriver/remote/http/common.rbs b/rb/sig/lib/selenium/webdriver/remote/http/common.rbs index dfafbbf8456bf..3f9fca9e87657 100644 --- a/rb/sig/lib/selenium/webdriver/remote/http/common.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/http/common.rbs @@ -11,13 +11,13 @@ module Selenium @common_headers: Hash[String, untyped] - attr_accessor self.extra_headers: Hash[String, untyped] + attr_accessor self.extra_headers: Hash[String, untyped]? - attr_writer self.user_agent: String + attr_writer self.user_agent: String? def self.user_agent: -> String - attr_writer server_url: String + attr_writer server_url?: URI def quit_errors: () -> Array[untyped] @@ -25,11 +25,13 @@ module Selenium def call: (untyped verb, untyped url, untyped command_hash) -> untyped + def server_url?: -> bool + private 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..f7f45df08472f 100644 --- a/rb/sig/lib/selenium/webdriver/remote/http/default.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/http/default.rbs @@ -4,21 +4,21 @@ module Selenium module Http # @api private class Default < Common - @open_timeout: untyped + @open_timeout: Integer - @read_timeout: untyped + @read_timeout: Integer @http: untyped - @proxy: untyped + @proxy: Proxy - attr_writer proxy: untyped + attr_writer proxy: Proxy? - attr_accessor open_timeout: untyped + attr_accessor open_timeout: Integer - attr_accessor read_timeout: untyped + attr_accessor read_timeout: Integer - def initialize: (?open_timeout: untyped?, ?read_timeout: untyped?) -> void + def initialize: (?open_timeout: Integer?, ?read_timeout: Integer?) -> void def close: () -> 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/spec/unit/selenium/webdriver/remote/bridge_spec.rb b/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb index cc8bf707045c3..f450d3259aa43 100644 --- a/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb +++ b/rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb @@ -51,8 +51,63 @@ 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 'creates default http client' do + bridge = described_class.new(url: 'http://localhost') + expect(bridge.http).to be_an_instance_of(WebDriver::Remote::Http::Default) + end + + it 'uses provided implementation of http client' do + url = 'http://localhost' + custom_http = instance_double(Remote::Http::Default, server_url?: false, 'server_url=': url) + bridge = described_class.new(http_client: custom_http, url: url) + expect(bridge.http).to eq(custom_http) + end + + it 'errors if http client and client config are both provided' do + custom_http = instance_double(Remote::Http::Default) + client_config = ClientConfig.new(server_url: 'http://localhost') + expect { + described_class.new(http_client: custom_http, client_config: client_config) + }.to raise_error(Error::WebDriverError, /cannot specify both/i) + end + + it 'errors if neither http client nor client config are provided' do + expect { + described_class.new + }.to raise_error(Error::WebDriverError, /no server url provided/i) + end + + it 'errors if url specified in keyword and client config' do + client_config = ClientConfig.new(server_url: 'http://localhost') + expect { + described_class.new(url: 'http://localhost', client_config: client_config) + }.to raise_error(Error::WebDriverError, /Cannot specify url in both/i) + end + + it 'creates http client from client_config' do + proxy = instance_double(Proxy) + extra_headers = {'X-Custom-Header' => 'custom_value'} + custom_user_agent = 'Custom User Agent' + client_config = ClientConfig.new( + open_timeout: 15, + read_timeout: 25, + proxy: proxy, + extra_headers: extra_headers, + user_agent: custom_user_agent, + server_url: 'http://localhost' + ) + original_user_agent = WebDriver::Remote::Http::Common.user_agent + bridge = described_class.new(client_config: client_config) + + expect(bridge.http).to be_an_instance_of(WebDriver::Remote::Http::Default) + expect(bridge.http.open_timeout).to eq(15) + expect(bridge.http.read_timeout).to eq(25) + expect(bridge.http.instance_variable_get(:@proxy)).to eq(proxy) + expect(WebDriver::Remote::Http::Common.extra_headers).to eq(extra_headers) + expect(WebDriver::Remote::Http::Common.user_agent).to eq(custom_user_agent) + ensure + WebDriver::Remote::Http::Common.extra_headers = nil + WebDriver::Remote::Http::Common.user_agent = original_user_agent end end From 343d32b7ec736c9935f9cc3bfcefbc8cb412a237 Mon Sep 17 00:00:00 2001 From: titusfortner Date: Wed, 22 Oct 2025 01:05:55 -0500 Subject: [PATCH 2/3] fix typing issues --- rb/lib/selenium/webdriver/remote/http/default.rb | 4 ++-- rb/sig/lib/selenium/webdriver/remote/http/default.rbs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rb/lib/selenium/webdriver/remote/http/default.rb b/rb/lib/selenium/webdriver/remote/http/default.rb index 2677eefc2de15..f1937109d5631 100644 --- a/rb/lib/selenium/webdriver/remote/http/default.rb +++ b/rb/lib/selenium/webdriver/remote/http/default.rb @@ -34,8 +34,8 @@ class Default < Common # @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 + @open_timeout = open_timeout if open_timeout + @read_timeout = read_timeout if read_timeout super() end diff --git a/rb/sig/lib/selenium/webdriver/remote/http/default.rbs b/rb/sig/lib/selenium/webdriver/remote/http/default.rbs index f7f45df08472f..1f74544f3a143 100644 --- a/rb/sig/lib/selenium/webdriver/remote/http/default.rbs +++ b/rb/sig/lib/selenium/webdriver/remote/http/default.rbs @@ -4,9 +4,9 @@ module Selenium module Http # @api private class Default < Common - @open_timeout: Integer + @open_timeout: Numeric - @read_timeout: Integer + @read_timeout: Numeric @http: untyped @@ -14,11 +14,11 @@ module Selenium attr_writer proxy: Proxy? - attr_accessor open_timeout: Integer + attr_accessor open_timeout: Numeric - attr_accessor read_timeout: Integer + attr_accessor read_timeout: Numeric - def initialize: (?open_timeout: Integer?, ?read_timeout: Integer?) -> void + def initialize: (?open_timeout: Numeric?, ?read_timeout: Numeric?) -> void def close: () -> untyped From 2114724f8c1e0cbaf7bbb158ebf7c2d27236110a Mon Sep 17 00:00:00 2001 From: titusfortner Date: Wed, 22 Oct 2025 08:45:26 -0500 Subject: [PATCH 3/3] validate and set the url directly from bridge constructor --- rb/lib/selenium/webdriver/remote/bridge.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rb/lib/selenium/webdriver/remote/bridge.rb b/rb/lib/selenium/webdriver/remote/bridge.rb index 40a1fb6210058..28b136f575caa 100644 --- a/rb/lib/selenium/webdriver/remote/bridge.rb +++ b/rb/lib/selenium/webdriver/remote/bridge.rb @@ -63,7 +63,9 @@ def initialize(url: nil, http_client: nil, client_config: nil) raise Error::WebDriverError, 'Cannot specify both http_client and client_config' end - @http = http_client || create_http_client(client_config, url: url) + @http = http_client || build_http_client(client_config) + validate_server_url_args(url, client_config) + @http.server_url = normalize_url(url || client_config&.server_url) @file_detector = nil @locator_converter = self.class.locator_converter @@ -90,12 +92,12 @@ def create_session(capabilities) extend(WebDriver::Firefox::Features) when 'msedge', 'MicrosoftEdge' extend(WebDriver::Edge::Features) - when 'Safari', 'Safari Technology Preview' + when 'safari', 'Safari', 'Safari Technology Preview' extend(WebDriver::Safari::Features) when 'internet explorer' extend(WebDriver::IE::Features) else - raise Error::WebDriverError, "Unknown browser name: #{@capabilities[:browser_name]}" + WebDriver.logger.info "Unknown browser: #{capabilities[:browser_name]}", id: :session end end