Repository navigation
Conversation
_use_proxy() adds a scheme to the https proxy unless it starts with
"https" or "socks", so SingleProxy(http="http://h:8080",
https="http://h:8443") ends up with "https://http://h:8443" and every
https request through it fails. Only add a scheme when the URL has none.
A proxy given as plain host:port is an ordinary HTTP proxy for https
URLs as well (the request is tunnelled with CONNECT), so use "http://"
for it, as for the http proxy; "https://" would make httpx speak TLS to
the proxy itself.
Since the proxies are stored under the httpx keys 'http://' and
'https://', Navigator._get_page() looked up pm._proxies.get('http'),
which is always None, so get_next_proxy() never told the proxy
generator (e.g. FreeProxies) which proxy had failed. Use the 'http://'
key.
Add offline tests for the proxy URLs.
The captcha fallback still uses Selenium 3 APIs. Selenium 4.10 removed
the positional executable_path of webdriver.Chrome(), so
webdriver.Chrome('chromedriver', options=options) fails with "TypeError:
WebDriver.__init__() got multiple values for argument 'options'", and
the proxy is set through webdriver.DesiredCapabilities as full URLs with
proxyType "MANUAL", while the W3C proxy capability expects a lowercase
proxyType and host:port entries; changing that shared class attribute
also affects every other webdriver created in the process.
Pass the proxies to Chrome as --proxy-server and to Firefox as a
selenium Proxy on the options (socks5:// proxies such as Tor as
socksProxy). If chromedriver or geckodriver is on PATH, pass it as a
Service; otherwise leave the lookup to Selenium Manager, which does not
support every platform.
Add offline tests with the webdriver classes mocked.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0662e1f48
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| looks for one (or downloads it, where it supports the platform). | ||
| """ | ||
| driver = shutil.which(name) | ||
| return {"service": service_class(executable_path=driver)} if driver else {} |
There was a problem hiding this comment.
service=
When scholarly is installed into an environment that already satisfies the unversioned selenium dependency with Selenium 3 (allowed by setup.py:36 and requirements.txt:10), finding either driver on PATH makes this helper return service=...; Selenium 3's webdriver.Chrome and webdriver.Firefox constructors do not accept that keyword, so both browser attempts raise TypeError and captcha handling cannot start. Add a Selenium 4 lower bound or retain a Selenium 3-compatible invocation.
Useful? React with 👍 / 👎.
Description
Three problems in the proxy code on
develop:_use_proxy()mangles an https proxy that already has a scheme. It prefixeshttps://unless the URL starts withhttpsorsocks, sopg.SingleProxy(http="http://h:8080", https="http://h:8443")storeshttps://http://h:8443and every https request through it fails. Now a scheme is only added when the URL has none ("://" not in url). A barehost:portgetshttp://for https too: it is an ordinary HTTP proxy that tunnels https with CONNECT, whilehttps://would make httpx speak TLS to the proxy itself.'http://'/'https://'(91bada0),Navigator._get_page()callsget_next_proxy(old_proxy=pm._proxies.get('http', None)), which is alwaysNone, so e.g.FreeProxiesnever marks a failing proxy as dirty. Now it reads'http://'.webdriver.Chrome('chromedriver', options=options)fails withTypeError: WebDriver.__init__() got multiple values for argument 'options'(the positional executable path was removed in 4.10). The proxy is set through the sharedwebdriver.DesiredCapabilitiesclass attributes as full URLs withproxyType: "MANUAL", whereas the W3C proxy capability uses a lowercase type andhost:portentries. Now Chrome gets--proxy-server=http=...;https=...and Firefox aselenium.webdriver.common.proxy.Proxyon its options (socks5 proxies such as Tor assocksProxy). Ifchromedriver/geckodriveris on PATH it is passed as aService; otherwise Selenium Manager finds the driver as before.Checklist
developand notmain.TestProxyGeneratorOfflinein test_module.py: proxy URLs, and the Chrome/Firefox drivers with the webdriver classes mocked; no network or browser needed.)Testing
python -B -m unittest test_module.TestProxyGeneratorOfflineon CPython 3.12.13, 3.13.14 and 3.14.3 (Windows), selenium 4.51.0, httpx 0.28.1:test_use_proxy_keeps_given_schemefails ('https://http://127.0.0.1:8443'); with 0002: 3 tests OK('chromedriver',) != (), noservice, nooptions.proxy); with 0003: 6 tests OK on all three versionsflake8with the repo's .flake8: 0 issuesContext: found while testing packages against Python 3.13 for FreeBSD ports (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=292105).