fix(fetch): accept socks proxy alias#4340
Open
he-yufeng wants to merge 1 commit into
Open
Conversation
0f61e90 to
6a110d6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Accept
socks://proxy URLs as a SOCKS5 alias in the fetch server.HTTPX accepts
socks5://...but raisesUnknown scheme for proxy URLfor the commonsocks://...form. That can happen either when users pass--proxy-url socks://...or when the surrounding client process providesALL_PROXY=socks://....This keeps the existing proxy behavior for normal
http://,https://, and already-valid SOCKS URLs, but normalizes the invalid alias before creating the HTTPX client.Fixes #767.
Publishing Your Server
Not applicable; this changes the existing fetch server.
Server Details
Motivation and Context
Some tools and desktop environments expose local proxy settings as
socks://127.0.0.1:.... HTTPX does not accept that scheme directly, so fetch fails before it can request the page. Treatingsocks://assocks5://matches the practical meaning users expect and avoids requiring each MCP client to rewrite the proxy URL first.The dependency is also changed to
httpx[socks]so SOCKS support is installed with the fetch server package.How Has This Been Tested?
.\.venv\Scripts\python.exe -m pytest tests\test_server.py::TestFetchUrl::test_fetch_with_proxy tests\test_server.py::TestFetchUrl::test_fetch_accepts_socks_proxy_alias tests\test_server.py::TestFetchUrl::test_fetch_accepts_socks_proxy_from_environment -q-> 3 passed.\.venv\Scripts\python.exe -m ruff check src\mcp_server_fetch\server.py tests\test_server.py-> passed.\.venv\Scripts\python.exe -m py_compile src\mcp_server_fetch\server.py tests\test_server.py-> passed.\.venv\Scripts\python.exe -m build-> passedgit diff --check-> passedI also ran
.\.venv\Scripts\python.exe -m pytest tests\test_server.py -q. It reached 21 passed / 1 failed. The failing test isTestExtractContentFromHtml.test_empty_content_returns_error, where the current Windows pure-Pythonreadabilipyfallback returns an empty string for empty HTML. That is unrelated to the proxy path changed here.Breaking Changes
None.
Types of changes
Checklist
Additional context
The README already documents
--proxy-url; this patch does not introduce a new option. It only accepts a proxy URL form that users commonly inherit from their environment.