Skip to content

Expose the proxy bypass option #1109

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

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Expose the proxy bypass option #1109

merged 1 commit into from
Dec 7, 2021

Conversation

grahamhar
Copy link

In some scenarios there will be a mixture of requests some will need a proxy some won't. This adds and exposes the proxy bypass option.

I have only tested this in chrome on linux, can you help by suggesting:

  • What tests need to be added.
  • Documentation to be added.
  • Updates for other browsers.

@mdmintz
Copy link
Member

mdmintz commented Dec 6, 2021

There are some things that should be fixed first:

You need to flake8 all your code and make sure there are no issues there. I see some long lines that would fail right away.

You left debug statements in your code:
logging.exception('Big issue from GRAHAM')

You mixed args by inserting proxy_bypass_list in the middle of grouped args. It should go after proxy_pass.

    proxy_string,
    proxy_bypass_list,
    proxy_auth,
    proxy_user,
    proxy_pass,

@grahamhar
Copy link
Author

I think I addressed all those points @mdmintz

@grahamhar grahamhar marked this pull request as ready for review December 6, 2021 18:43
@mdmintz
Copy link
Member

mdmintz commented Dec 6, 2021

@grahamhar According to https://www.chromium.org/developers/design-documents/network-settings , it's a semi-colon separated list, not a comma-separated one. Have you tried an example with multiple domains bypassed?

--proxy-bypass-list=(<trailing_domain>|<ip-address>)[:<port>][;...]

This tells chrome to bypass any specified proxy for the given semi-colon-separated list of hosts. This flag must be used (or rather, only has an effect) in tandem with --proxy-server.
Note that trailing-domain matching doesn't require "." separators so "*google.com" will match "igoogle.com" for example.

For example,
  --proxy-server="foopy:8080" --proxy-bypass-list="*.google.com;*foo.com;127.0.0.1:8080"
will use the proxy server "foopy" on port 8080 for all hosts except those pointing to *.google.com, those pointing to *foo.com and those pointing to localhost on port 8080. 
igoogle.com requests would still be proxied. ifoo.com requests would not be proxied since *foo, not *.foo was specified. 

@mdmintz
Copy link
Member

mdmintz commented Dec 6, 2021

@grahamhar It's mostly for the Python-comment description, but I'd check for a semicolon-separated list, as a comma could appear in valid urls such as data:,Hello.

@grahamhar
Copy link
Author

@grahamhar It's mostly for the Python-comment description, but I'd check for a semicolon-separated list, as a comma could appear in valid urls such as data:,Hello.

I think the bypass list only covers domains and IP address not URL so commas are not actually valid. I'll update the comment to reflect it should be a semi colon separated list.

@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar I added more comments. Looks like there are still a few places where proxy_bypass_list was not in the correct location in browser_launcher.py.

@grahamhar
Copy link
Author

@grahamhar I added more comments. Looks like there are still a few places where proxy_bypass_list was not in the correct location in browser_launcher.py.

Sorry, not sure how I missed them. Updated now.

@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar On your line 943 of browser_launcher.py, it looks like you're calling chrome_options = _set_chrome_options( without the proxy_bypass_list as a variable, which breaks my internal tests that include selenium grid tests. Those tests are private because they include private authentication codes to private proxy servers, but I'm happy to walk through all issues I encounter like this.

@grahamhar
Copy link
Author

@mdmintz I think there will possibly be quite a few gaps like this. I only managed to test using the pycharm fixture. Let me know any more changes and I'll try to update.

@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar Sounds good. You'll need to add that into get_remote_driver( to make that new line also work.

@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar Your current line 1017 of browser_launcher.py is missing the proxy_bypass_list in firefox_options = _set_firefox_options(.

@mdmintz mdmintz merged commit 1035ab9 into seleniumbase:master Dec 7, 2021
@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar Merged! Nice work. It'll be part of the next release.

@mdmintz
Copy link
Member

mdmintz commented Dec 7, 2021

@grahamhar It has been released as part of https://github.com/seleniumbase/SeleniumBase/releases/tag/v2.2.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants