lib/dialer: Register dialer for socks URL scheme (fixes #4515) #4579
Conversation
|
Hi! You seem to have included a bunch of old commits in this PR, probably by reusing and old branch. Can you take the new commit and put that on a new branch based on current master, and push that? There's various rebase magic I could suggest, but I suspect there are better guides out there for figuring it out. Worst case you can just start a new branch from master (after pulling) and cherry-pick |
lib/dialer/internal.go
Outdated
| // This is a rip off of proxy.FromEnvironment with a custom forward dialer | ||
| func getDialer(forward proxy.Dialer) proxy.Dialer { | ||
| proxy.RegisterDialerType("socks", socksDialerFunction) |
There was a problem hiding this comment.
I'm guessing this could/should be called from an init() or similar, not every time we need a new dialer?
|
@calmh I'm sorry for inconvenience with those unnecessary commits, it's fixed now. |
lib/dialer/internal.go
Outdated
| func getDialer(forward proxy.Dialer) proxy.Dialer { | ||
| func getDialer(forward proxy.Dialer, initCall bool) proxy.Dialer { | ||
| if initCall { | ||
| proxy.RegisterDialerType("socks", socksDialerFunction) |
There was a problem hiding this comment.
I thought about that but doesn't functions used in variable initialization get called before init()? As we get usingProxy variable by calling getDialer in variable initialization, I'd have to essentially copy that code into init() after calling RegisterDialerType. Something like that? https://pastebin.com/39qWkWuL
I may be wrong of course, I'm not a golang expert.
There was a problem hiding this comment.
Or I guess I could just move the proxyDialer and usingProxy initialization from var ( ) block to init()
|
Apart from the triple slash in the comment, looks good to me. |
|
@st-review merge |
st-review
added
the
pr-merged
st-review
added
the
frozen-due-to-age
Purpose
Hello everyone. Just as described in the issue, Syncthing doesn't work with environment variable all_proxy=socks://host:port. This is my attempt at solving this issue.
Testing
I'm not sure if it's proper test for this case but I did set up proxy using ssh:
ssh -D 9898 -f -C -q -N user@my_university -p 22
export ALL_PROXY_NO_FALLBACK=1
export all_proxy=socks://127.0.0.1:9898
STTRACE=dialer bin/syncthing
building current origin/master code I got:
[monitor] 2017/12/07 21:16:27.308095 internal.go:51: DEBUG: Dialer logging disabled, as no proxy was detected
[CQLK3] 2017/12/07 21:16:27.311707 internal.go:51: DEBUG: Dialer logging disabled, as no proxy was detected
after fix:
[CQLK3] 2017/12/07 21:17:15.006258 internal.go:43: INFO: Proxy settings detected
[CQLK3] 2017/12/07 21:17:15.006282 internal.go:45: INFO: Proxy fallback disabled
...
[CQLK3] 2017/12/07 21:17:17.020198 internal.go:59: DEBUG: Dialing tcp address 186.195.228.140:22067 via proxy - success, 127.0.0.1:57400 -> 127.0.0.1:9898
[CQLK3] 2017/12/07 21:17:17.073681 internal.go:59: DEBUG: Dialing tcp address 109.230.199.119:22067 via proxy - success, 127.0.0.1:57402 -> 127.0.0.1:9898
With socks5://127.0.0.1:9898 it works, as expected, in both cases.
Documentation
I think docs should mention that "socks" scheme for all_proxy env variable works the same as socks5 in Using Proxies chapter.