From 54bead8354dc0829a647a877567097c340a4edc9 Mon Sep 17 00:00:00 2001 From: titusfortner Date: Mon, 11 Dec 2023 12:34:20 -0600 Subject: [PATCH 1/2] update proxyType values to match w3c spec --- dotnet/src/webdriver/Proxy.cs | 19 +++++++------ java/src/org/openqa/selenium/Proxy.java | 22 ++++++--------- py/selenium/webdriver/common/proxy.py | 27 +++++++------------ rb/lib/selenium/webdriver/common/proxy.rb | 7 +++-- rb/spec/unit/selenium/webdriver/proxy_spec.rb | 4 --- 5 files changed, 29 insertions(+), 50 deletions(-) diff --git a/dotnet/src/webdriver/Proxy.cs b/dotnet/src/webdriver/Proxy.cs index 733a86f4553e4..a63a11cd61772 100644 --- a/dotnet/src/webdriver/Proxy.cs +++ b/dotnet/src/webdriver/Proxy.cs @@ -27,16 +27,12 @@ namespace OpenQA.Selenium /// /// Describes the kind of proxy. /// - /// - /// Keep these in sync with the Firefox preferences numbers: - /// http://kb.mozillazine.org/Network.proxy.type - /// public enum ProxyKind { /// - /// Direct connection, no proxy (default on Windows). + /// Direct connection, no proxy. /// - Direct = 0, + Direct, /// /// Manual proxy settings (e.g., for httpProxy). @@ -51,15 +47,15 @@ public enum ProxyKind /// /// Use proxy automatic detection. /// - AutoDetect = 4, + AutoDetect, /// - /// Use the system values for proxy settings (default on Linux). + /// Use the system values for proxy settings. /// System, /// - /// No proxy type is specified. + /// No proxy type is specified. This must be changed before use /// Unspecified } @@ -488,7 +484,10 @@ internal Dictionary ToLegacyCapability() private Dictionary AsDictionary(bool isSpecCompliant) { Dictionary serializedDictionary = null; - if (this.proxyKind != ProxyKind.Unspecified) + if (this.proxyKind == ProxyKind.Unspecified) { + throw new InvalidOperationException("proxyKind must be set before use"); + } + else { serializedDictionary = new Dictionary(); if (this.proxyKind == ProxyKind.ProxyAutoConfigure) diff --git a/java/src/org/openqa/selenium/Proxy.java b/java/src/org/openqa/selenium/Proxy.java index ca264597802ae..f6db9bc4a798a 100644 --- a/java/src/org/openqa/selenium/Proxy.java +++ b/java/src/org/openqa/selenium/Proxy.java @@ -36,19 +36,12 @@ public class Proxy { public enum ProxyType { - // Keep these in sync with the Firefox preferences numbers: - // http://kb.mozillazine.org/Network.proxy.type - - DIRECT("direct"), // Direct connection, no proxy (default on Windows) + DIRECT("direct"), // Direct connection, no proxy MANUAL("manual"), // Manual proxy settings (e.g. for httpProxy) PAC("pac"), // Proxy auto-configuration from URL - - RESERVED_1("reserved_1"), // Never used (but reserved in Firefox) - AUTODETECT("autodetect"), // Proxy auto-detection (presumably with WPAD) - SYSTEM("system"), // Use system settings (default on Linux) - - UNSPECIFIED("unspecified"); + SYSTEM("system"), // Use system settings + UNSPECIFIED("unspecified"); // This must be changed before using private final String type; @@ -127,7 +120,9 @@ public Proxy(Map raw) { public Map toJson() { Map m = new HashMap<>(); - if (proxyType != ProxyType.UNSPECIFIED) { + if (proxyType == ProxyType.UNSPECIFIED) { + throw new IllegalStateException("proxyType must be specified before use"); + } else { m.put(PROXY_TYPE, proxyType.toString()); } if (ftpProxy != null) { @@ -165,7 +160,7 @@ public Map toJson() { /** * Gets the {@link ProxyType}. This can signal if set to use a direct connection (without proxy), - * manually set proxy settings, auto-configured proxy settings, or whether to use the default + * manually set proxy settings, autoconfigured proxy settings, or whether to use the default * system proxy settings. It defaults to {@link ProxyType#UNSPECIFIED}. * * @return the proxy type employed @@ -198,7 +193,7 @@ public boolean isAutodetect() { /** * Specifies whether to autodetect proxy settings. * - * @param autodetect set to true to use proxy auto detection, false to leave proxy settings + * @param autodetect set to true to use proxy auto-detection, false to leave proxy settings * unspecified * @return reference to self */ @@ -455,7 +450,6 @@ public String toString() { builder.append("pac: ").append(getProxyAutoconfigUrl()); break; - case RESERVED_1: case UNSPECIFIED: break; } diff --git a/py/selenium/webdriver/common/proxy.py b/py/selenium/webdriver/common/proxy.py index 145b5fb9548c9..635785119b8fd 100644 --- a/py/selenium/webdriver/common/proxy.py +++ b/py/selenium/webdriver/common/proxy.py @@ -17,28 +17,17 @@ """The Proxy implementation.""" -class ProxyTypeFactory: - """Factory for proxy types.""" - - @staticmethod - def make(ff_value, string): - return {"ff_value": ff_value, "string": string} - - class ProxyType: """Set of possible types of proxy. - Each proxy type has 2 properties: 'ff_value' is value of Firefox profile preference, 'string' is id of proxy type. """ - DIRECT = ProxyTypeFactory.make(0, "DIRECT") # Direct connection, no proxy (default on Windows). - MANUAL = ProxyTypeFactory.make(1, "MANUAL") # Manual proxy settings (e.g., for httpProxy). - PAC = ProxyTypeFactory.make(2, "PAC") # Proxy autoconfiguration from URL. - RESERVED_1 = ProxyTypeFactory.make(3, "RESERVED1") # Never used. - AUTODETECT = ProxyTypeFactory.make(4, "AUTODETECT") # Proxy autodetection (presumably with WPAD). - SYSTEM = ProxyTypeFactory.make(5, "SYSTEM") # Use system settings (default on Linux). - UNSPECIFIED = ProxyTypeFactory.make(6, "UNSPECIFIED") # Not initialized (for internal use). + DIRECT = "DIRECT" # Direct connection, no proxy + MANUAL = "MANUAL" # Manual proxy settings (e.g., for httpProxy). + PAC = "PAC" # Proxy autoconfiguration from URL. + AUTODETECT = "AUTODETECT" # Proxy auto-detection (presumably with WPAD). + SYSTEM = "SYSTEM" # Use system settings @classmethod def load(cls, value): @@ -72,7 +61,7 @@ class Proxy: """Proxy contains information about proxy type and necessary proxy settings.""" - proxyType = ProxyType.UNSPECIFIED + proxyType = None autodetect = False ftpProxy = "" httpProxy = "" @@ -281,12 +270,14 @@ def proxy_type(self, value) -> None: self.proxyType = value def _verify_proxy_type_compatibility(self, compatible_proxy): - if self.proxyType not in (ProxyType.UNSPECIFIED, compatible_proxy): + if self.proxyType not in (None, compatible_proxy): raise ValueError( f"Specified proxy type ({compatible_proxy}) not compatible with current setting ({self.proxyType})" ) def to_capabilities(self): + if not self.proxyType: + raise ValueError("proxyType must be specified before use") proxy_caps = {"proxyType": self.proxyType["string"].lower()} proxies = [ "autodetect", diff --git a/rb/lib/selenium/webdriver/common/proxy.rb b/rb/lib/selenium/webdriver/common/proxy.rb index fd32711e0a632..e68c638dc3fdf 100644 --- a/rb/lib/selenium/webdriver/common/proxy.rb +++ b/rb/lib/selenium/webdriver/common/proxy.rb @@ -21,11 +21,11 @@ module Selenium module WebDriver class Proxy TYPES = { - direct: 'DIRECT', # Direct connection, no proxy (default on Windows). + direct: 'DIRECT', # Direct connection, no proxy. manual: 'MANUAL', # Manual proxy settings (e.g., for httpProxy). pac: 'PAC', # Proxy autoconfiguration from URL. - auto_detect: 'AUTODETECT', # Proxy autodetection (presumably with WPAD). - system: 'SYSTEM' # Use system settings (default on Linux). + auto_detect: 'AUTODETECT', # Proxy auto-detection (presumably with WPAD). + system: 'SYSTEM' # Use system settings. }.freeze ALLOWED = {type: 'proxyType', @@ -44,7 +44,6 @@ class Proxy def self.json_create(data) data['proxyType'] = data['proxyType'].downcase.to_sym - return if data['proxyType'] == :unspecified proxy = new diff --git a/rb/spec/unit/selenium/webdriver/proxy_spec.rb b/rb/spec/unit/selenium/webdriver/proxy_spec.rb index 4dd76c9e2325b..06d481bdae20c 100644 --- a/rb/spec/unit/selenium/webdriver/proxy_spec.rb +++ b/rb/spec/unit/selenium/webdriver/proxy_spec.rb @@ -118,10 +118,6 @@ module WebDriver expect(proxy).to eq(other) end - - it 'deserializes to nil if proxyType is UNSPECIFIED' do - expect(described_class.json_create('proxyType' => 'UNSPECIFIED')).to be_nil - end end end # WebDriver end # Selenium From feaa53c25f28464a01a871751185d31c9ed208ed Mon Sep 17 00:00:00 2001 From: titusfortner Date: Tue, 12 Dec 2023 12:48:45 -0600 Subject: [PATCH 2/2] fix python errors --- py/selenium/webdriver/common/proxy.py | 7 +++---- py/test/selenium/webdriver/common/proxy_tests.py | 10 ++++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/py/selenium/webdriver/common/proxy.py b/py/selenium/webdriver/common/proxy.py index 635785119b8fd..ac0c36b4b71fe 100644 --- a/py/selenium/webdriver/common/proxy.py +++ b/py/selenium/webdriver/common/proxy.py @@ -35,9 +35,8 @@ def load(cls, value): value = value["string"] value = str(value).upper() for attr in dir(cls): - attr_value = getattr(cls, attr) - if isinstance(attr_value, dict) and "string" in attr_value and attr_value["string"] == value: - return attr_value + if getattr(cls, attr) == value: + return value raise Exception(f"No proxy type is found for {value}") @@ -278,7 +277,7 @@ def _verify_proxy_type_compatibility(self, compatible_proxy): def to_capabilities(self): if not self.proxyType: raise ValueError("proxyType must be specified before use") - proxy_caps = {"proxyType": self.proxyType["string"].lower()} + proxy_caps = {"proxyType": self.proxyType.lower()} proxies = [ "autodetect", "ftpProxy", diff --git a/py/test/selenium/webdriver/common/proxy_tests.py b/py/test/selenium/webdriver/common/proxy_tests.py index 3ec0946c874a1..7e748565488a9 100644 --- a/py/test/selenium/webdriver/common/proxy_tests.py +++ b/py/test/selenium/webdriver/common/proxy_tests.py @@ -120,9 +120,9 @@ def test_can_init_pacproxy(): assert PAC_PROXY["proxyAutoconfigUrl"] == proxy.proxy_autoconfig_url -def test_can_init_empty_proxy(): +def test_errors_on_empty_proxy(): proxy = Proxy() - assert ProxyType.UNSPECIFIED == proxy.proxy_type + assert proxy.proxy_type is None assert "" == proxy.http_proxy assert "" == proxy.ftp_proxy assert "" == proxy.no_proxy @@ -135,8 +135,6 @@ def test_can_init_empty_proxy(): assert proxy.socks_version is None options = ArgOptions() - options.proxy = proxy - proxy_capabilities = {} - proxy_capabilities["proxyType"] = "unspecified" - assert proxy_capabilities == options.to_capabilities().get("proxy") + with pytest.raises(ValueError): + options.proxy = proxy