Add socks_endpoint kwarg to TorClientEndpoint's constructor #182
Conversation
3 similar comments
| @@ -667,6 +668,7 @@ class TorClientEndpoint(object): | |||
| socks_ports_to_try = [9050, 9150] | |||
|
|
|||
| def __init__(self, host, port, | |||
| socks_socket=None, | |||
| socks_hostname=None, socks_port=None, | |||
There was a problem hiding this comment.
It seems like it'd be better to have just one way to specify "how to connect to SOCKS" -- right now there's _proxy_endpoint_generator= and socks_socket= (and socks_hostname/socks_port) -- can we just have e.g. "socks_endpoint=None" and if it's None, we try some default things? We could change the "list of default things" to be actual endpoint-strings, too (e.g. in case there's a well-known Tor SOCKS unix socket default in future). In any case, I think it'd be better if "socks_socket" were at least "socks_endpoint" so that a user can manually specify any sort of endpoint (including of course UNIXClientEndpoint).
Backwards-compatibilitiy: socks_hostname/socks_port are public API; we can capture **kwargs and do something sensible if these were specified (i.e. pretend they said socks_endpoint=TCP4ClientEndpoint(host, port)).
I see I dropped _proxy_endpoint_generator from "the new stuff" so maybe we can accomplish that here, too?
There was a problem hiding this comment.
ok i've turned the new argument into socks_endpoint... but i did not yet try to change all the default things to endpoints.
Current coverage is 99.41% (diff: 88.88%)@@ master #182 diff @@
==========================================
Files 15 15
Lines 2709 2724 +15
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 2697 2708 +11
- Misses 12 16 +4
Partials 0 0
|
| @@ -667,6 +667,7 @@ class TorClientEndpoint(object): | |||
| socks_ports_to_try = [9050, 9150] | |||
|
|
|||
| def __init__(self, host, port, | |||
| socks_endpoint=None, | |||
| socks_hostname=None, socks_port=None, | |||
| socks_username=None, socks_password=None, | |||
There was a problem hiding this comment.
Should be able to get rid of socks_hostname + socks_port too then, right? (i.e. it's socks_endpoint=TCP4ClientEndpoint(host, port) instead of socks_hostname=host and socks_port=port right?)
here we also fix some of the tests
david415
changed the title
3 similar comments
|
Cool, looks good. Should probably have a test for the backwards-compat thing I guess ... |
fix docstring and add tls option
|
I tested it! It works. I tested with tor configured with a unix socket SOCKS port... |
1 similar comment
| @@ -5,3 +5,4 @@ Twisted>=11.1.0 | |||
| ipaddress>=1.0.16 | |||
| zope.interface>=3.6.1 | |||
| txsocksx>=1.13.0 | |||
| pyopenssl>=16.1.0 | |||
There was a problem hiding this comment.
Doesn't this get added by Twisted? Or is that an optional thing? (I mean: I don't think txtorcon actually directly depends on pyopenssl, does it?)
|
Oh, hmm: need tests for the |
| @@ -677,7 +677,6 @@ def __init__(self, host, port, | |||
| except KeyError: | |||
| pass | |||
|
|
|||
| if self.socks_endpoint is None: | |||
| self._socks_port_iter = iter(self.socks_ports_to_try) | |||
There was a problem hiding this comment.
This isn't actually a duplicate: socks_endpoint might still be None here, if a) the user didn't pass socks_endpoint to begin with and b) also didn't use the old style socks_host/socks_port options... (i.e. if it hits the KeyError).
this adds UNIX domain socket functionality to the TorClientEndpoint... so that we connect to the SOCKS proxy via a unix domain socket.