wrap_socket does not accept server_hostname in 2.7.x (breaks with gevent)

[sharoonthomas]

The recent backport of SNI related features into 2.7 series has resulted in urllib looking at python 2.7.8+ as python 3.2+ and ssl_.py calls wrap_socket with the server_hostname as argument. While this works well when not monkey patched with gevent, I am reporting the bug here since one of the features of this library is gevent compat.

[shazow]

Mmm that is no fun indeed. @sharoonthomas Do you have a fix in mind?

Also we should setup a travisci build for gevent someday.

[sigmavirus24]

I don't understand why this is a bug here. Yes we shoot for gevent compatibility, but shouldn't gevent be looking at Python 2.7.x compatibility? This seems like two issues:

1. Us not detecting gevent and knowing it's broken
2. gevent not being current with Python 2.7.8+

[sharoonthomas]

@shazow I have sent a pr #483 which refactors how the tests are run to include gevent tests for 2.6 and 2.7. The 2.7 test with gevent seems to fail for coverage reasons - not for this bug, because it runs using 2.7.8. Is this something you wanted to do ?

[shazow]

@sharoonthomas Hmm yea, we'd need to fix the coverage error. Would you be up for that? :)

Super appreciated btw, you're awesome.

[sharoonthomas]

@shazow took a look at it. I don't think its something I can test effectively with the knowledge I have about urllib3. Sorry about that !

[zappe]

Any ETA on when this can be fixed?

[shazow]

@zappe Would you like to take a look at @sharoonthomas's work and try to fill out the test coverage?

[zappe]

@shazow, I'm afraid I don't have the knowledge, sorry.

[ssbarnea]

3 months later and we still have this bug open even if we do have a patch. I suppose it would make sense to remove Python 2.7 compatibility in this case, sarcasm intended ;)

[Lukasa]

@ssbarnea Such is the nature of open source. It's an orphaned patch which needs some love.

[ssbarnea]

@Lukasa I solved the problem locally by removing the grequests dependency and reusing the support from inside requests.

[shazow]

@ssbarnea I'm not sure what kind of response you're expecting. None of us are being paid to work on urllib3; I invite you to fix this: http://urllib3.readthedocs.org/en/latest/#sponsorship

[sharoonthomas]

Sorry everyone, haven't had the time to fix the test case. Will try to do it sometime next week (if no one else does it by then)

Sent from my iPhone

[shazow]

@sharoonthomas Somebody added a $15 bounty on this, you should grab it if you can finish it soon. :)

[sharoonthomas]

@shazow I am keeping a watch on this patch, but a fully working patch on gevent for this cannot be sent until gevent/gevent#477 is fixed. To be specific:

• SSLSocket implementation in gevent does not accept server_hostname (though python 2.7.9 does).
• ssl. sslwrap was removed by python 2.7.9 (this one is easier to get around).

For anyone who might pick off from here the code is on my fork (https://github.com/sharoonthomas/urllib3/compare/issue-482)

[shazow]

@sharoonthomas Ah thanks for the updates, makes sense. Thanks for keeping an eye on this!

[shazow]

@sprin Thanks for bumping up the bounty to $115. :)

Looks like the referenced gevent issue is making progress. If we're in a super-hurry, we may use a polyfill patch from the gevent thread or somesuch.

[sprin]

Patching gevent appears to be the preferred solution. I think a patch to gevent should be awarded the bounty, but it made sense to add to the bounty started here.

I have not been able to use any of the proposed polyfills with success when Py 2.7.9, gevent, and requests are used together.

[shazow]

Agree that incentives given for the gevent contributors might be better suited in this scenario. :) Maybe we should poke the gevent thread and offer up for one of them to come claim the bounty once they close their bug.

[sharoonthomas]

@shazow +1

[Lukasa]

🍰 ✨

[zeevt]

if you patch gevent like this: https://github.com/zeevt/gevent/commit/beafef169b43e5a19e42e69c115ac278c6abdce4 it works and passes my tests (sends SNI etc.)

[sigmavirus24]

Except that we don't carry patches for gevent @zeevt

[shazow]

Is this fixed in the latest gevent? Can anyone confirm?

[transfluxus]

This error popped up a while ago in my application? Is it something easily fixable? upgrading urllib or something?

[Lukasa]

@transfluxus My understanding is that this is a gevent bug. Try upgrading gevent.

[sethmlarson]

Since there hasn't been any movement or additional reports here going to assume this is resolved.