SOCKS5H support for httpclient #25070
Conversation
| @@ -338,7 +347,6 @@ proc body*(response: AsyncResponse): Future[string] {.async.} = | |||
| type | |||
| Proxy* = ref object | |||
| url*: Uri | |||
| auth*: string | |||
| ## Constructs a new `TProxy` object. | ||
| result = Proxy(url: url, auth: auth) |
There was a problem hiding this comment.
Likewise for all these constructors.
There was a problem hiding this comment.
The code is not broken, I've introduced backward compatible (but deprecated) constructors and auth getter. I've removed the field, because of ambiguity redundancy with url, which IMO is a well-established format for providing/storing username/password.
|
Let me expand a bit on API change. Most importantly, it doesn't break existing code! The As a real-world example, take a look at nimble code here and here where it takes deliberate actions to parse user:password out of the url only to pass it to |
|
Well CI failure seems related: Category: stdlib Output: |
|
It does seem that way, but the adjacent PR tests fail with same exact message, I think it's because |
|
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
- Added support for SOCKS5h (h for proxy-side DNS resolving) to httpclient - Deprecated `auth` arguments for `newProxy` constructors, for auth to be embedded in the url. Unfortunately `http://example.com` is not currently reachable from github CI, so the tests fail there for a few days already, I'm not sure what can be done here. (cherry picked from commit 161b321)
autharguments fornewProxyconstructors, for auth to be embedded in the url.Unfortunately
http://example.comis not currently reachable from github CI, so the tests fail there for a few days already, I'm not sure what can be done here.