shazow/urllib3 — pull request #284 by Anorov (2013-11-23 03:36:35+00:00)
Title: SOCKS proxy support

Here's the SOCKS proxy support patch.

I added new SOCKS Connection classes in `connection.py`, and added a `_is_socks` attribute to the proxy URL that is passed to `ProxyManager`. The `_is_socks` attribute is used by the connection pools so that HTTP proxy negotiation steps aren't taken when the proxy has a SOCKS scheme. This could possibly be refactored (separate `socks_proxy` and `http_proxy` attributes?); tell me how you feel about the `_is_socks` solution.

I also touched up some other parts of the codebase, as follows, which I noticed while working through files and debugging.

* Fixed inconsistent indentation in a few places.
* Fixed some typos in comments.
* Changed `__str__` methods to `__repr__`. `__str__` automatically defers to `__repr__` if it isn't already defined, but the reverse isn't true. So this keeps the same behavior while also allowing easier debugging from the REPL.
* Added a newline in the middle of the proxy connection exception message, to make it a little easier to read.

I believe this branch should currently work fine for regular usage; it seems to have no trouble with any SOCKS proxies.

However, I had many confusing issues and bugs while trying to modify the test suite. The tests are currently broken (I added `raise SkipTest()` in the proxy tests, for the time being). I am using Twisted for the SOCKS4 and SOCKS5 proxy servers, and added that to the test requirements.

The test issues seem to stem from the Tornado HTTP and HTTPS dummy servers. On my computer, they spat out odd SSL errors (like `SSLError: [Errno 1] _ssl.c:504: error:1411B072:SSL routines:SSL3_GET_NEW_SESSION_TICKET:bad message type
` and many others), and would hang seemingly randomly.

I upgraded the Tornado version in `test-requirements.txt` because the version listed before appeared to have a bug that prevented me (or others) from running in IPv6 mode, see here: https://github.com/facebook/tornado/pull/593; I had to upgrade to get IPv6-related tests to pass. I'm not sure if the upgrade has anything to do with the current issues.

I think it has something to do with trying to run Tornado in separate threads. I was not able to get multithreading to work with Twisted, so I spawned a new process for each SOCKS proxy server instead; Twisted seems to work fine, as I can connect to and use the proxy servers while the tests are running.

I would appreciate if someone could modify the tests so the HTTP servers work properly, or give me some advice on how to fix the issue. And of course, I'd appreciate any suggestions or changes for the SOCKS patch itself.