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

[navin772]

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.

[qodo-merge-pro]

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()

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	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

[cgoldberg]

I know we already do it in a few places, but I'm not crazy about hitting external sites in the test suite (badssl.com, example.com, herokuapp.com). Can you think of any way we can test this locally? If it's too complex to setup, it's better to hit them than not test... but if we can come up with a way to avoid it, that would be best.

[navin772]

Can you think of any way we can test this locally?

@cgoldberg I think that will require adding custom test webpages to selenium in common/src/web but I am still not sure how we will simulate a badssl connection, that will involve certificates maybe.

And since during the test run, the webserver serves pages via localhost, we cannot test proxy featues because browsers bypass proxy settings for localhost (so the test will fail), hence we need to test on external sites.

If the above issues could be solved, we can move to local only tests.

Currently, I am facing one issue in the CI - the browser in the CI is auto upgrading HTTP requests to HTTPS (maybe due to browser security rules) and my 'Fake' proxy supports only HTTP (creating a HTTPS proxy is quite complex just for a test). All tests pass locally.

So, any help with this is much appreciated!

[cgoldberg]

I'll try to think of some ideas for local testing... but yea, creating a site with a different domain and invalid cert just for testing is not trivial. I guess we can keep it in the test suite, and if it becomes an issue in terms of reliability, we can decide to disable them.

[navin772]

Currently, I am facing one issue in the CI - the browser in the CI is auto upgrading HTTP requests to HTTPS (maybe due to browser security rules) and my 'Fake' proxy supports only HTTP (creating a HTTPS proxy is quite complex just for a test).

Seems like only chrome is doing this, Edge browser test passes in the CI. This can be security rule from chrome as seen in some forums.

I will see if something can be done to get it working in the CI else I will skip it for Chrome.