Also, https://github.com/cmullaparthi/ibrowse/blob/master/src/ibrowse_socks5.erl
will use google’s DNS server (8.8.8.8) if DNS resolution fails. That’s nutty (and, again, dangerous for Tor users). It also hardcodes the timeouts. No, I don’t feel compelled to take the upstream version. B. On 5 Jan 2014, at 12:45, Robert Newson <[email protected]> wrote: > > The main reason is that I had no idea there was an upstream version. :) > > My motivation was to enable CouchDB replication over Tor, which is why I only > implemented connections where the domainname resolution happens through the > proxy. The upstream patch only works the other way (and is therefore leaky > and unsafe for Tor users). > > An earlier version of my work *did* check all the rest of the response but it > became clear that proxying works without any of that information, so I > simplified the code. I *do* check that the reply is successful, the details > afterwards did not matter in my case. When using Tor as the SOCKS proxy, both > the address and port are returned as 0. > > Thanks for the review, and it highlights a neat to improve our github > interoperation, none of this discussion appears on the pull request. :/ > > B. > > > On 5 Jan 2014, at 05:57, Benoit Chesneau <[email protected]> wrote: > >> Totally +1 to have such features in couchdb, which is what I suggested on >> IRC. >> >> >> I made a quick review, comments are following: >> >> - Why not using the version already available in iBrowse upstream: >> >> https://github.com/cmullaparthi/ibrowse/blob/master/src/ibrowse_socks5.erl >> >> Like I said on IRC it is not correct and doesn't offer ssl upgrade like >> your module (or the one I did in hackney) but wouldn't it be better to >> support the upstream version by patching it? At least to start from it? >> >> - The current patch only connect to a proxy using the domain resolution >> which is not possible in every case. The module should also provide a >> direct connection using an IPV6 ro IPV4 address. Something like: >> >> https://github.com/benoitc/hackney/blob/master/src/hackney_socks5.erl#L180 >> >> can be added here: >> >> https://github.com/apache/couchdb/pull/125/files#diff-e86abc9dc33f7a1a35b55861f001470fR79 >> >> - Right now the proposed solution only check the first bits of the final >> response in the handshake and doesn't check if it get valid response >> returning a domain or ipv6/v4 depending. Not sure if it's needed but maybe >> we should check it: >> >> https://github.com/apache/couchdb/pull/125/files#diff-e86abc9dc33f7a1a35b55861f001470fR83 >> >> >> >> On Sat, Jan 4, 2014 at 7:35 PM, rnewson <[email protected]> wrote: >> >>> GitHub user rnewson opened a pull request: >>> >>> https://github.com/apache/couchdb/pull/125 >>> >>> SOCKS 5 support for replication >>> >>> Notably, this allows replication over the Tor network (yes, DNS >>> resolution occurs over Tor also). >>> >>> >>> You can merge this pull request into a Git repository by running: >>> >>> $ git pull https://github.com/rnewson/couchdb socks5 >>> >>> Alternatively you can review and apply these changes as the patch at: >>> >>> https://github.com/apache/couchdb/pull/125.patch >>> >>> ---- >>> commit 059fe032d5afe8f668effd2c3f81fa73ebe4031f >>> Author: Robert Newson <[email protected]> >>> Date: 2014-01-04T17:32:00Z >>> >>> Add SOCKS5 module to ibrowse >>> >>> commit 3fbbc234a86600881d6a75d9cece4d2af8c168a3 >>> Author: Robert Newson <[email protected]> >>> Date: 2014-01-04T17:31:40Z >>> >>> Pass protocol to ibrowse >>> >>> commit e529636f0a07c898ba9b1e788deaebffb7f871de >>> Author: Robert Newson <[email protected]> >>> Date: 2014-01-04T17:57:22Z >>> >>> Clarify HTTP proxy fields and variables >>> >>> commit 65ba845f5705885b46842df93f8017af58841177 >>> Author: Robert Newson <[email protected]> >>> Date: 2014-01-04T18:02:59Z >>> >>> Use SOCKS5 proxy if specified >>> >>> ---- >>> >>> >
