Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix SSL/HTTPS on Python 3.1 #500

Merged
merged 0 commits into from

2 participants

@phihag

Currently, https:// requests with simple_httpclient fail on Python 3.1 with two unrelated errors. This pull request fixes both of them.

@bdarnell
Owner

Tornado does not currently support Python 3.1, and as I recall the biggest source of problems was not SSL but the fact that some of the functions for e.g. url parsing have some encoding-related issues. I'd rather not introduce such a complex and fragile change for 3.1 compatibility unless we can also resolve the other issues. What's the current state of the test suite (python -m tornado.test.runtests) on 3.1?

@phihag

Yes, the tests all fail because of parse_qs_bytes. Since I'm not using that function (or much of the web functionality at all), this personally doesn't affect me.

Short rant on tornado.escape.parse_qs_bytes: The signature of this function is fundamentally flawed: It takes a byte string and returns character strings (among others), but doesn't take an encoding argument, and doesn't specify a standard encoding. It cannot possibly work. And unless I'm mistaken, most (if not all) users will want a {str:str} dictionary anyways.

I also have no idea why it's "too painful" to use bytes as keys in a Python 3 dictionary. Additionally, the selection of latin1 as encoding seems strange. It may be my mistake, but I understand RFC 3986 2.5 as strongly suggesting, if not mandating, UTF-8 encoding of URLs, and I'd expect the incoming URL (or part thereof) to be decoded from bytes to a character string (with UTF-8 as encoding) before anything else.

I also don't understand the implementation. latin1 happens to be 8bit-transparent, but as I see it, the original author really wanted raw_unicode_escape. In light of latin1's 8bit transparency, I have absolutely no idea why errors='strict' is set in the implementation. Also, I'd like to see a dictionary expression used, but that's probably just personal preference.

There's also an easy fix for Python 3.1's missing encoding= parameter of urllib.parse.parse_qs - just decode the bytes beforehand. /end rant

In any case, the proposed changes are all ifed to explicitely run on Python 3.1 only (the ciphers check also affects 3.0), so the worst they could do is further break Python 3.1. And obviously I'm biased because of my use pattern, but since when is "another component also breaks on the platform" a reason not to fix an unrelated component?

@bdarnell
Owner

parse_qs_bytes mostly deals in bytes; it returns str for keys because otherwise you'd have to mark up all your string literals as byte literals when the keys are nearly always ascii. It's true that most callers want {str: [str]} maps, but the encoding is not known at the time parse_qs is called so we must preserve bytes until they can be decoded later. (I think you're right that errors='strict' is probably meaningless here. dict expressions cannot be used because this codebase is compatible with python 2.5+)

Decoding the bytes beforehand is not a substitute for the 3.2 encoding argument to parse_qs: the encoding argument applies not to the input string, but to the result of decoding percent-escaped bytes.

I'm fine with changing the version check around the set_ciphers call, but the monkey patching of 3.1's "broken ssl" has no real place in tornado. I'd be OK with including something like this if it made 3.1 a supported platform (or even a quasi-supported platform like windows), but if tornado is not going to at least semi-officially support 3.1 then the hacks necessary to make 3.1's ssl module viable are best distributed as either a separate module or a separate fork of tornado.

@phihag phihag merged commit f4c037c into tornadoweb:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 0 additions and 0 deletions.
Something went wrong with that request. Please try again.