Skip to content

[py][bidi]: support accept_insecure_certs and proxy parameters in create_user_context #15983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

navin772
Copy link
Member

@navin772 navin772 commented Jun 30, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Adds support for accept_insecure_certs and proxy parameters in create_user_context for Browser module - https://w3c.github.io/webdriver-bidi/#command-browser-createUserContext

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add accept_insecure_certs and proxy parameters to BiDi create_user_context

  • Implement to_bidi_dict() method for proxy configuration conversion

  • Add comprehensive tests for new BiDi browser functionality


Changes diagram

flowchart LR
  A["Browser.create_user_context()"] --> B["Optional Parameters"]
  B --> C["accept_insecure_certs"]
  B --> D["proxy"]
  D --> E["Proxy.to_bidi_dict()"]
  E --> F["BiDi Format Conversion"]
  C --> G["BiDi Command"]
  F --> G
  G --> H["User Context Created"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
browser.py
Enhanced create_user_context with proxy and cert options 

py/selenium/webdriver/common/bidi/browser.py

  • Add optional accept_insecure_certs and proxy parameters to
    create_user_context method
  • Import Optional type and Proxy class for type annotations
  • Build parameters dictionary conditionally based on provided arguments
  • +18/-2   
    proxy.py
    Add BiDi format conversion for proxy settings                       

    py/selenium/webdriver/common/proxy.py

  • Add to_bidi_dict() method to convert proxy settings to BiDi format
  • Handle manual proxy type with HTTP, SSL, SOCKS configurations
  • Support PAC proxy type with autoconfig URL
  • Convert noProxy from string to list format for BiDi compatibility
  • +36/-0   
    Tests
    bidi_browser_tests.py
    Add tests for BiDi proxy and cert features                             

    py/test/selenium/webdriver/common/bidi_browser_tests.py

  • Add comprehensive tests for accept_insecure_certs parameter
    functionality
  • Test direct and manual proxy configurations with fake proxy server
  • Test combined usage of both new parameters
  • Include helper functions for fake proxy server and port allocation
  • +142/-0 

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added C-py Python Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Jun 30, 2025
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    1234 - Not compliant

    Non-compliant requirements:

    • Fix JavaScript execution in link href on click() for Firefox
    • Ensure compatibility with Firefox 42.0 32bit on 64bit machine
    • Restore functionality that worked in version 2.47.1 but broke in 2.48.0/2.48.2

    5678 - Not compliant

    Non-compliant requirements:

    • Fix ChromeDriver ConnectFailure error on Ubuntu 16.04.4
    • Resolve connection refused errors for subsequent ChromeDriver instances
    • Ensure first instance works without console errors
    • Support Chrome 65.0.3325.181 with ChromeDriver 2.35

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Type Safety

    The method accesses self.proxyType["string"] without null checking, which could raise KeyError if proxyType is not properly initialized or doesn't contain the "string" key.

    proxy_type = self.proxyType["string"].lower()
    result = {"proxyType": proxy_type}
    Resource Cleanup

    The fake proxy server cleanup in exception scenarios may not execute properly, potentially leaving servers running and ports occupied.

    finally:
        driver.browser.remove_user_context(user_context)
        fake_proxy_server.shutdown()
        fake_proxy_server.server_close()

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 30, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Validate SOCKS version values

    Add validation to ensure socksVersion is a valid value (4 or 5) before adding it
    to the result. Invalid SOCKS versions could cause runtime errors in the BiDi
    protocol.

    py/selenium/webdriver/common/proxy.py [346-347]

     if self.socksVersion is not None:
    +    if self.socksVersion not in [4, 5]:
    +        raise ValueError("socksVersion must be 4 or 5")
         result["socksVersion"] = self.socksVersion
    • Apply / Chat
    Suggestion importance[1-10]: 7

    __

    Why: This suggestion correctly adds validation for socksVersion, improving the robustness of the to_bidi_dict method by preventing invalid values from being sent.

    Medium
    Learned
    best practice
    Add null checks for dictionary access

    The code accesses self.proxyType["string"] without checking if self.proxyType
    exists or if the "string" key is present. Add validation to prevent KeyError
    exceptions when these properties are None or missing.

    py/selenium/webdriver/common/proxy.py [329-337]

     def to_bidi_dict(self):
         """Convert proxy settings to BiDi format.
     
         Returns:
         -------
             dict: Proxy configuration in BiDi format.
         """
    +    if not self.proxyType or "string" not in self.proxyType:
    +        raise ValueError("proxyType must be set with a valid string value")
         proxy_type = self.proxyType["string"].lower()
         result = {"proxyType": proxy_type}
    • Apply / Chat
    Suggestion importance[1-10]: 6

    __

    Why:
    Relevant best practice - Add null checks and validation for parameters, properties, and return values before using them to prevent NullReferenceException and other runtime errors

    Low
    General
    Use consistent element locator syntax

    Use the By.TAG_NAME constant instead of the string literal for consistency with
    other test methods in the same file. This maintains code consistency and follows
    the established pattern.

    py/test/selenium/webdriver/common/bidi_browser_tests.py [198]

    -body_text = driver.find_element("tag name", "body").text
    +body_text = driver.find_element(By.TAG_NAME, "body").text
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion correctly points out an inconsistent locator strategy and proposes using By.TAG_NAME, which aligns with the rest of the file and improves code quality.

    Low
    • Update

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-devtools Includes everything BiDi or Chrome DevTools related C-py Python Bindings Review effort 3/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants