Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port Python 2.7.9 ssl.py #546

Closed
wants to merge 2 commits into from
Closed

Port Python 2.7.9 ssl.py #546

wants to merge 2 commits into from

Conversation

sprin
Copy link
Contributor

@sprin sprin commented Mar 21, 2015

Resolves #477.

In Python 2.7.9, we have new SSL interfaces, that are not quite
the old Python 2 interfaces, nor the same as the Python 3 interfaces.

This creates a new SSL module, _sslgte279.py, for Python 2 versions
greater than or equal 2.7.9, ported from Python 2.7.9's ssl.py:
https://hg.python.org/cpython/file/648dcafa7e5f/Lib/ssl.py.

The process of porting was done by starting with gevent's ssl3.py and
backporting Python 2.7.9's ssl.py.

Fixes these tests under Python 2.7.9:

  • /usr/local/bin/python -u -m monkey_test --Event test_urllib2net.py
  • /usr/local/bin/python -u -m monkey_test test_urllib2net.py
  • /usr/local/bin/python -u test__ssl.py

No new test failures. Still failing since 72119c8:

  • /usr/local/bin/python -u -m monkey_test --Event test_ssl.py
  • /usr/local/bin/python -u -m monkey_test test_ssl.py
  • /usr/local/bin/python -u test__pywsgi.py
  • /usr/local/bin/python -u test__makefile_ref.py
  • /usr/local/bin/python -u test__socket.py
  • /usr/local/bin/python -u test___example_servers.py
  • /usr/local/bin/python -u -m monkey_test --Event test_ftplib.py
  • /usr/local/bin/python -u test__socket_ssl.py
  • /usr/local/bin/python -u -m monkey_test test_ftplib.py
  • /usr/local/bin/python -u test__all__.py

In addition, basic HTTPS requests were tested with urllib3==1.10.2
and requests==2.6.0.

@sprin
Copy link
Contributor Author

sprin commented Mar 21, 2015

One way to review the _sslgte279 module is to compare to ssl.py in Python 2.7.9.

I set up such a comparison on a different branch here: sprin@23e2771

@jamadden
Copy link
Member

I saw some additional failures under PyPy. I fixed several of them (notably test__socket_ssl and test__ssl) in https://github.com/NextThought/gevent/commit/5840cdc8988d7fd8383f01612b39a8cf919bb2d5 (probably requires my other changes on that fork to be fully effective). The failures I'm seeing in test_ftplib and test__pywsgi seem very similar to these first failures; I'll keep trying to track them down.

@jamadden jamadden mentioned this pull request Mar 23, 2015
@sprin
Copy link
Contributor Author

sprin commented Mar 24, 2015

@jamadden: Nice work!

I am hoping we can keep the PR to resolve #477 small so it's more manageable to review and test. How do you feel if we try to integrate your SSL fixes sans the PyPy changes in this PR?

I think if we can get all the tests passing under CPython 2.7.9, there should be a clear case for merging this PR. Then we can follow on with the PyPy changes.

I may be able to put some time in to this later this week.

@jamadden
Copy link
Member

@sprin I'm just happy to see progress made, however that needs to happen :)

I started this work on a separate branch from the main thrust of the PyPy changes and tried to keep the work on it mostly in spirit with what was already in _ssl2 hoping it would be easy to merge if desired. Most PyPy specific things are behind the if PYPY test, although I did encounter an issue with the timeout attribute when running the tests under CPython, so that part of it is not guarded as such. Other than that, there's the changes in the 2.7pypy/test_ssl test module; I know that the main 2.7/test_ssl module and it are divergent, but I don't know how much, I just know that the 2.7/test_ssl module is still failing and I haven't looked into why.

@jamadden jamadden mentioned this pull request Mar 25, 2015
@thedrow
Copy link
Contributor

thedrow commented Apr 13, 2015

What's the status here?
Anything I can do to help?

@Ivoz
Copy link
Contributor

Ivoz commented Apr 13, 2015

One immediate problem is this PR is based on gevent master, which AFAIK is not really ready for release.

It's currently in progress to work on Python 3, however nowhere near finished (see myriad of current PRs). So currently it's not suitable to release on top of for a patch fix. If one was hoping merge this PR and then cut a quick release off master to fix #477 for existing users, I don't see that as currently viable.

Additionally there are a lot of differences between gevent 1.0.1 and master least not in ssl, which I think makes it a little difficult to rebase to older, I could be wrong.

What I have been (slowly) working on is a similar patchfix to gevent 1.0.1's codebase, as getting a patchfix out for gevent for 2.7.9 is obviously desirable, while work goes on to get a 1.1 with py3 compatibility.

You can hopefully see the public branch https://github.com/gevent/gevent/tree/1.0.2 which I intend should be released once code is on it to fix #477. Obviously I'd welcome a PR on top of it.

After we'll need to port onto master as well after, which this PR could be useful for; but not for getting a release out ASAP currently.

@thedrow
Copy link
Contributor

thedrow commented Apr 13, 2015

@LVoz I don't have much bandwidth nor the technical understanding (yet) to contribute such a PR.
I can however try to help with existing test failures and coordination of implementation.

@jamadden Are you going to create a pull request of your branch against this PR?

@jamadden
Copy link
Member

@thedrow I created a PR at sprin#1 for my updates. @sprin was going to review and merge them there, which will update this PR.

@sprin
Copy link
Contributor Author

sprin commented Apr 14, 2015

I reviewed and merged @jamadden's PR, which tackled a lot of the tough compatibility issues and fixed the tests. See sprin#1 for a good discussion of the changes.

I think the next step is to work on rebasing this on top of gevent 1.0.2. @Ivoz, should we move ahead with this strategy? You imply that the 1.0.2 branch is otherwise ready for release.

@jamadden
Copy link
Member

PR #551 created to put these changes in the 1.0.2 branch.

jamadden added a commit that referenced this pull request May 17, 2015
Per #546, rebase the changes on top of gevent 1.0.2. Fixes #477.
@jamadden jamadden closed this in dbe77de May 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ssl broken for python > 2.7.9
4 participants