Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Nov 9, 2025

User description

🔗 Related Issues

relates to #16282

💥 What does this PR do?

This pull request introduces support for advanced HTTP and WebSocket configuration in all major Selenium WebDriver classes through a new client_config parameter. The change allows users to pass a ClientConfig instance for fine-grained control over connection settings, which takes precedence over legacy parameters like keep_alive. The constructors for Chrome, Edge, Firefox, Internet Explorer, and Safari drivers are updated to accept and normalize this new configuration, ensuring consistent behavior and clear documentation for users.

Major feature: Advanced connection configuration

  • Added an optional client_config parameter (type: ClientConfig) to the constructors of all major WebDriver classes (chrome, edge, firefox, ie, safari), enabling advanced HTTP/WebSocket settings. The parameter is documented to take precedence over keep_alive and other legacy options, and example usage is provided in each class docstring. [1] [2] [3] [4] [5]

  • For each driver, the constructor now normalizes client_config by always setting remote_server_addr to the local driver service URL. If a client_config is provided by the user, its fields are copied and merged with the service URL, ensuring local drivers always connect to the correct endpoint. [1] [2] [3] [4]

Integration and documentation

  • The new client_config parameter is passed through to the corresponding remote connection classes (ChromiumRemoteConnection, FirefoxRemoteConnection, SafariRemoteConnection, RemoteConnection), allowing these classes to use the advanced configuration for HTTP and WebSocket connections. [1] [2] [3] [4]

  • Imports for ClientConfig are added to all affected driver files to support the new parameter. [1] [2] [3] [4] [5]

  • Docstrings for all affected constructors are updated to clearly explain the purpose and precedence of client_config, including usage examples for both default and custom configurations. [1] [2] [3] [4] [5]

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement


Description

  • Added client_config parameter to all major WebDriver classes

  • Enables advanced HTTP/WebSocket configuration for local drivers

  • Normalizes remote_server_addr to service URL for local drivers

  • Comprehensive unit tests for ClientConfig support across drivers


Diagram Walkthrough

flowchart LR
  A["WebDriver Classes<br/>Chrome, Edge, Firefox, IE, Safari"] -->|"accept client_config"| B["ClientConfig Instance"]
  B -->|"normalize remote_server_addr"| C["Service URL"]
  B -->|"preserve user settings"| D["HTTP/WebSocket Config"]
  C --> E["RemoteConnection"]
  D --> E
  E -->|"establish connection"| F["Local Driver Service"]
Loading

File Walkthrough

Relevant files
Enhancement
7 files
webdriver.py
Add ClientConfig parameter to Chrome WebDriver                     
+20/-0   
webdriver.py
Implement ClientConfig normalization in ChromiumDriver     
+33/-0   
webdriver.py
Add ClientConfig parameter to Edge WebDriver                         
+17/-1   
webdriver.py
Add ClientConfig parameter to Firefox WebDriver                   
+44/-0   
webdriver.py
Add ClientConfig parameter to IE WebDriver                             
+42/-1   
webdriver.py
Add ClientConfig parameter to Safari WebDriver                     
+45/-1   
client_config.py
Make remote_server_addr optional in ClientConfig                 
+1/-1     
Tests
6 files
webdriver_timeout_test.py
Add functional tests for ClientConfig timeout handling     
+349/-0 
chrome_webdriver_tests.py
Add unit tests for Chrome ClientConfig support                     
+193/-0 
webdriver_tests.py
Add comprehensive unit tests for ChromiumDriver ClientConfig
+407/-0 
edge_webdriver_tests.py
Add unit tests for Edge ClientConfig support                         
+188/-0 
firefox_webdriver_tests.py
Add unit tests for Firefox ClientConfig support                   
+191/-0 
ie_webdriver_tests.py
Add unit tests for IE ClientConfig support                             
+192/-0 

@selenium-ci selenium-ci added the C-py Python Bindings label Nov 9, 2025
@iampopovich
Copy link
Contributor Author

iampopovich commented Nov 9, 2025

https://github.com/iampopovich/selenium/actions/runs/19208403903
The existing tests are passing. I'll study them in more detail soon and write new tests if necessary.
@cgoldberg jfyi

Add unit tests for ClientConfig support across multiple WebDriver implementations

Add unit tests for ClientConfig support in Firefox, IE, and Chrome WebDrivers

- Implemented unit tests for FirefoxDriver to validate ClientConfig handling.
- Added tests for IE WebDriver to ensure proper ClientConfig acceptance and passing.
- Created tests for Chrome WebDriver to verify ClientConfig integration.
- All tests check for correct handling of remote server address, keep-alive settings, and timeouts.
- Fixed assertion issues in tests to compare ClientConfig attributes correctly.
@iampopovich
Copy link
Contributor Author

@iampopovich iampopovich marked this pull request as ready for review November 12, 2025 13:40
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 12, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The new client configuration handling and driver instantiation add no audit trail for
critical actions like service start and connection setup, but this library code may
intentionally avoid logging.

Referred Code
# Normalize client_config: always set remote_server_addr to service.service_url
# for local drivers (it cannot be overridden by user for local drivers)
if client_config is None:
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=keep_alive,
        timeout=120,
    )
else:
    # Always create new ClientConfig with service.service_url and copy all user fields
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=client_config.keep_alive,
        proxy=client_config.proxy,
        ignore_certificates=client_config.ignore_certificates,
        timeout=client_config.timeout,
        ca_certs=client_config.ca_certs,
        username=client_config.username,
        password=client_config.password,
        auth_type=client_config.auth_type,
        token=client_config.token,


 ... (clipped 15 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge cases partial: The normalization overwrites user-provided remote_server_addr without explicit validation
or warnings and relies on external connection classes for failures, which may be
acceptable but is not evident in this diff.

Referred Code
# Normalize client_config: always set remote_server_addr to service.service_url
# for local drivers (it cannot be overridden by user for local drivers)
if client_config is None:
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=keep_alive,
        timeout=120,
    )
else:
    # Always create new ClientConfig with service.service_url and copy all user fields
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=client_config.keep_alive,
        proxy=client_config.proxy,
        ignore_certificates=client_config.ignore_certificates,
        timeout=client_config.timeout,
        ca_certs=client_config.ca_certs,
        username=client_config.username,
        password=client_config.password,
        auth_type=client_config.auth_type,
        token=client_config.token,


 ... (clipped 5 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Optional endpoint: Making remote_server_addr optional shifts validation to later stages; while local drivers
set it, ensuring it is non-empty for all paths is not fully verifiable from this diff.

Referred Code
def __init__(
    self,
    remote_server_addr: Optional[str] = None,
    keep_alive: Optional[bool] = True,
    proxy: Optional[Proxy] = Proxy(raw={"proxyType": ProxyType.SYSTEM}),
    ignore_certificates: Optional[bool] = False,
    init_args_for_pool_manager: Optional[dict] = None,

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 12, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Refactor duplicated and brittle code

The client_config normalization logic is duplicated and brittle across four
webdriver classes. This should be refactored into a single, shared utility
function that programmatically copies attributes to improve maintainability.

Examples:

py/selenium/webdriver/chromium/webdriver.py [66-89]
        if client_config is None:
            client_config = ClientConfig(
                remote_server_addr=self.service.service_url,
                keep_alive=keep_alive,
                timeout=120,
            )
        else:
            # Always create new ClientConfig with service.service_url and copy all user fields
            client_config = ClientConfig(
                remote_server_addr=self.service.service_url,

 ... (clipped 14 lines)
py/selenium/webdriver/firefox/webdriver.py [80-103]
        if client_config is None:
            client_config = ClientConfig(
                remote_server_addr=self.service.service_url,
                keep_alive=keep_alive,
                timeout=120,
            )
        else:
            # Always create new ClientConfig with service.service_url and copy all user fields
            client_config = ClientConfig(
                remote_server_addr=self.service.service_url,

 ... (clipped 14 lines)

Solution Walkthrough:

Before:

# In ChromiumDriver, FirefoxDriver, IEDriver, SafariDriver...

if client_config is None:
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=keep_alive,
        timeout=120,
    )
else:
    # Brittle: all attributes are manually copied
    client_config = ClientConfig(
        remote_server_addr=self.service.service_url,
        keep_alive=client_config.keep_alive,
        proxy=client_config.proxy,
        ignore_certificates=client_config.ignore_certificates,
        timeout=client_config.timeout,
        # ... and so on for all other attributes
    )

After:

# In a new shared utility function
def normalize_local_driver_config(service_url, user_config=None, **defaults):
    if user_config is None:
        return ClientConfig(remote_server_addr=service_url, **defaults)

    # Programmatically copy attributes to avoid brittleness
    config_args = vars(user_config).copy()
    config_args["remote_server_addr"] = service_url
    return ClientConfig(**config_args)

# In ChromiumDriver, FirefoxDriver, etc.
client_config = normalize_local_driver_config(
    self.service.service_url,
    user_config=client_config,
    keep_alive=keep_alive,
    timeout=120
)
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies significant code duplication and a brittle, hard-to-maintain implementation for client_config normalization across multiple files, proposing a valid refactoring that improves code quality and future maintainability.

Medium
Possible issue
Improve ClientConfig cloning to be robust
Suggestion Impact:The commit replaced the manual ClientConfig reconstruction with a utility function normalize_local_driver_config, which centralizes and likely robustly clones/normalizes the config instead of explicitly listing attributes. This addresses the brittleness highlighted by the suggestion.

code diff:

-        # Normalize client_config: always set remote_server_addr to service.service_url
-        # for local drivers (it cannot be overridden by user for local drivers)
-        if client_config is None:
-            client_config = ClientConfig(
-                remote_server_addr=self.service.service_url,
-                keep_alive=keep_alive,
-                timeout=120,
-            )
-        else:
-            # Always create new ClientConfig with service.service_url and copy all user fields
-            client_config = ClientConfig(
-                remote_server_addr=self.service.service_url,
-                keep_alive=client_config.keep_alive,
-                proxy=client_config.proxy,
-                ignore_certificates=client_config.ignore_certificates,
-                timeout=client_config.timeout,
-                ca_certs=client_config.ca_certs,
-                username=client_config.username,
-                password=client_config.password,
-                auth_type=client_config.auth_type,
-                token=client_config.token,
-                user_agent=client_config.user_agent,
-                extra_headers=client_config.extra_headers,
-                websocket_timeout=client_config.websocket_timeout,
-                websocket_interval=client_config.websocket_interval,
-            )
+        client_config = normalize_local_driver_config(
+            self.service.service_url, user_config=client_config, keep_alive=keep_alive, timeout=120
+        )

Refactor the manual copying of ClientConfig attributes to a more robust method
using dict to prevent missing attributes like init_args_for_pool_manager.

py/selenium/webdriver/chromium/webdriver.py [73-89]

 # Always create new ClientConfig with service.service_url and copy all user fields
-client_config = ClientConfig(
-    remote_server_addr=self.service.service_url,
-    keep_alive=client_config.keep_alive,
-    proxy=client_config.proxy,
-    ignore_certificates=client_config.ignore_certificates,
-    timeout=client_config.timeout,
-    ca_certs=client_config.ca_certs,
-    username=client_config.username,
-    password=client_config.password,
-    auth_type=client_config.auth_type,
-    token=client_config.token,
-    user_agent=client_config.user_agent,
-    extra_headers=client_config.extra_headers,
-    websocket_timeout=client_config.websocket_timeout,
-    websocket_interval=client_config.websocket_interval,
-)
+config_dict = client_config.__dict__.copy()
+config_dict["remote_server_addr"] = self.service.service_url
+client_config = ClientConfig(**config_dict)

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the manual attribute copying is brittle and misses the init_args_for_pool_manager attribute, which is a potential bug. The proposed change makes the code more robust and maintainable.

Medium
Learned
best practice
Validate client_config parameters

Validate client_config fields (e.g., timeouts non-negative numbers) and raise a
clear ValueError before constructing the connection.

py/selenium/webdriver/chromium/webdriver.py [32-89]

 def __init__(
     self,
     browser_name: Optional[str] = None,
     vendor_prefix: Optional[str] = None,
     options: Optional[ChromiumOptions] = None,
     service: Optional[ChromiumService] = None,
     keep_alive: bool = True,
     client_config: Optional[ClientConfig] = None,
 ) -> None:
-...
+    ...
+    # Normalize client_config and validate
     if client_config is None:
         client_config = ClientConfig(
             remote_server_addr=self.service.service_url,
             keep_alive=keep_alive,
             timeout=120,
         )
     else:
-        # Always create new ClientConfig with service.service_url and copy all user fields
+        # Basic validation for external I/O related parameters
+        if client_config.timeout is not None and client_config.timeout < 0:
+            raise ValueError("client_config.timeout must be >= 0")
+        if client_config.websocket_timeout is not None and client_config.websocket_timeout <= 0:
+            raise ValueError("client_config.websocket_timeout must be > 0")
+        if client_config.websocket_interval is not None and client_config.websocket_interval <= 0:
+            raise ValueError("client_config.websocket_interval must be > 0")
+
         client_config = ClientConfig(
             remote_server_addr=self.service.service_url,
             keep_alive=client_config.keep_alive,
             proxy=client_config.proxy,
             ignore_certificates=client_config.ignore_certificates,
             timeout=client_config.timeout,
             ca_certs=client_config.ca_certs,
             username=client_config.username,
             password=client_config.password,
             auth_type=client_config.auth_type,
             token=client_config.token,
             user_agent=client_config.user_agent,
             extra_headers=client_config.extra_headers,
             websocket_timeout=client_config.websocket_timeout,
             websocket_interval=client_config.websocket_interval,
         )

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Guard external API and I/O configuration with validation and clear errors to avoid crashes (validate constructor args and enforce reasonable types/ranges).

Low
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants