use PyOpenSSL as SSL backend #90

Closed
t-8ch opened this Issue Aug 5, 2012 · 12 comments

Comments

Projects
None yet
5 participants
Contributor

t-8ch commented Aug 5, 2012

PyOpenSSL would make it possible use SNI on versions of python:

Problem:
The connection made by PyOpenSSL does not implement the makefile()-Method.
This is used by httplib. Stacktrace:

Traceback (most recent call last):
  File "/home/t-8ch/Projekte/urllib3/test/with_dummyserver/test_https.py", line 24, in test_simple
    fields={'method': 'GET'})
  File "/home/t-8ch/Projekte/urllib3/urllib3/request.py", line 67, in request
    **urlopen_kw)
  File "/home/t-8ch/Projekte/urllib3/urllib3/request.py", line 80, in request_encode_url
    return self.urlopen(method, url, **urlopen_kw)
  File "/home/t-8ch/Projekte/urllib3/urllib3/connectionpool.py", line 417, in urlopen
    body=body, headers=headers)
  File "/home/t-8ch/Projekte/urllib3/urllib3/connectionpool.py", line 279, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/lib/python3.2/http/client.py", line 1047, in getresponse
    response = self.response_class(self.sock, method=self._method)
  File "/usr/lib/python3.2/http/client.py", line 281, in __init__
    self.fp = sock.makefile("rb")
NotImplementedError: Cannot make file object of SSL.Connection

Ideas to cope with this:

  • Solve #58
  • Use ndg-httpsclient, which is a wrapper around PyOpenSSL and provides a SSLSocket-class which implements makefile()
  • Just take the needed parts (SSLSocket) from ndg-httpsclient. ndg-httpsclient is BSD-licensed. One would also need a minor update to get python3 compability

I am in favour of the last point.

Owner

shazow commented Aug 5, 2012

One more idea:

  • We could more completely replace the http.client.HTTPResponse with our own urllib3.response.HTTPResponse without using makefile, if that makes sense. Looks like we can set the http.client.HTTPConnection.response_class to use our own, though I haven't looked at backwards compatibility issues with Python 2.x.

Upside of this option is that it's a reasonable small step towards #58 and it doesn't require vendoring in additional external code.

Also a question: What are our primary reasons for using PyOpenSSL? I understand it brings SNI to Python 2.x, but is there anything else we really need?

Contributor

t-8ch commented Aug 5, 2012

My reasoning went quite close along the line "There a are people stuck on python2, and we just introduced a second SSL-backend so why not stuff one more in."
pyOpenSSL has support for some generic crypto and random stuff and interface closer to the C-API of openssl. As python3.2 introduced an API, which is modelled after the one of openssl itself this doesn't matter really much.

Replacing the response class sounds interesting.

Owner

shazow commented Aug 5, 2012

Fair enough. I guess this issue is relatively low priority for now. You or anyone else is welcome to take a crack at it if you're feeling inspired. :)

Contributor

t-8ch commented Aug 5, 2012

For the record: Do you want to keep the API of the original HTTPResponse?
https://github.com/benoitc/http-parser seems to be halfway there

Owner

shazow commented Aug 5, 2012

Do you mean the original urllib3 HTTPResponse or httplib HTTPResponse?

It's obviously preferable to keep whatever compatibility we already have, but I'm very open to solid API improvements at the cost of breaking backwards compatibility. Ideally there would be a transition period before complete deprecation.

I've looked at http-parser and I definitely like it. The downside is that we'll be introducing an install-time dependency, but there are big upsides in performance and dropping the dependency on httplib. Though I don't feel it'd be worth introducing the dependency unless we drop the httplib dependency (ie. #58).

Can we please move this to high priority.

Owner

shazow commented Aug 10, 2012

There aren't so much priorities in this project as much as how soon things are likely to get done, which depends on whether the people who need this feature are willing to implement it. :)

Contributor

wolever commented Aug 10, 2012

It might be possible to use a little subclass of http://docs.python.org/library/io.html#io.IOBase to implement an object that could sensibly returned by makefile().

Contributor

t-8ch commented Aug 10, 2012

I have tried something like this.
(Following code is python3)
httplib always calls makefile() like this: sock.makefile("rb")
I then took the makefile() implementation from socket.py and ripped out all
the conditionals.
The whole function would then be condensed down to:

return io.BufferedReader(SocketIO(socket, "r"), io.DEFAULT_BUFFER_SIZE)

But of course this wouldn't work.

Contributor

schlamar commented Mar 25, 2013

Should be done with #156?!

Owner

shazow commented Mar 25, 2013

Well, it's optional in a contrib module. Up to you guys how you interpret that. :P

Contributor

t-8ch commented Mar 25, 2013

I think the contrib module fits perfectly. Closing.

@t-8ch t-8ch closed this Mar 25, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment