Add dialer interface according to golang.org/x/net/proxy. #42
Conversation
Need to be tested, I'll add some simple test.
|
I actually regret to have the |
|
You are right, it could be moved to another separate package, I'll refactor my code, thanks for pointing that out :) |
|
I think your code could be organized into a standalone package providing proxy support via e.g. Shadowsocks, SOCKS5, HTTP proxy, etc. That will be more compatible with the design of Also, I'm thinking about separating the reusable packages and the demo server/client to use them. |
Now you may use github.com/shadowsocks/go-shadowsocks2/proxy to use the dialer interface.
|
I've moved dialer interface to a separate package named proxy. I don't know if this is ok, and please comment for change if this name is too generic or too ambiguous. |
|
The code looks pretty good now. What do you think if you make it available as its own repo on GitHub? |
|
Sorry for the looooong delay, I had some serious about my graduation design and it took whole June to get around with it, and I just have spare time now to reply this issue. Making as own repo is fine, but I would recommend to include this within primary repo, or at least as a project under shadowsocks organization to make this interface used by more people. Anyway, thanks for the review :) |
| return | ||
| } | ||
|
|
||
| func (dialer *ProxyDialer) Dial(network, addr string) (c net.Conn, err error) { |
There was a problem hiding this comment.
You should call this DialTCP, as in net.DialTCP.
Unless you also support UDP or other networks.
In any case, you should be checking the value of the network parameter.
| Dial: dialer.Dial, | ||
| }, | ||
| } | ||
| resp, err := client.Get("http://ifconfig.co") |
There was a problem hiding this comment.
Your unit tests shouldn't depend on external services.
Why don't you run a simple TCP service locally instead?
Here is an example where I do that: https://github.com/Jigsaw-Code/outline-ss-server/blob/master/shadowsocks/tcp_test.go
This allow us to use shadowsocks as upstream proxy in go program more easily, and a (like-to-be) standard interface would be nice.
I've attached some testcases with pull request, and all tests passed with forked version.