meejah / txtorcon

Twisted-based asynchronous Tor control protocol implementation. Includes unit-tests, examples, state-tracking code and configuration abstraction.
http://fjblvrw2jrxnhtg67qpbzi45r7ofojaoo3orzykesly2j3c2m3htapid.onion/
MIT License
250 stars 72 forks source link

allow the client endpoint to .connect() multiple times #246

Closed warner closed 7 years ago

warner commented 7 years ago

I think StreamClientEndpoints should be reusable: you should be able to call .connect() multiple times, yielding a different Protocol instance each time. But the TorClientEndpoint constructor creates a single iterator to track the sequence of possible SOCKS ports we should try, so the second time .connect() is called, it uses the next item on this list, which will fail.

This patch removes the iterator and just loops over the original list each time .connect() is called, removing the interaction between calls.

This ought to fix #245. I wasn't able to figure out a good unit test for it.. if you can point me in the right direction, I'll add a test and re-submit the PR.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.0e-05%) to 99.945% when pulling a8abd54bc4c1a2c58bc3d8db3bfefc60d43de477 on warner:245-multiple-connect into 1d882534f596280fe18f5fa5982a9d3eab636654 on meejah:master.

meejah commented 7 years ago

Cool, thanks!

I made a pull-request from my 245-multiple-connect to yours improving a test that sort-of did this but didn't actually get through the whole list (nor check re-using the endpoint).

meejah commented 7 years ago

@warner please see https://github.com/meejah/txtorcon/pull/249