bpo-22708: Fix http protocol version in CONNECT method, support IDN #742
Conversation
Original author of this patch demianbrecht@gmail.com
|
FYI for any other core dev, Demian Brecht has signed the CLA. |
There was a problem hiding this comment.
Please also include the NEWS entry in the original patch and append "Patch by Demian Brecht.".
| @@ -1877,6 +1877,16 @@ def test_connect_with_tunnel(self): | ||
| # This test should be removed when CONNECT gets the HTTP/1.1 blessing | ||
| self.assertNotIn(b'Host: proxy.com', self.conn.sock.data) | ||
|
|
||
| def test_connect_with_tunnel_idna(self): | ||
| dest = '\u03b4\u03c0\u03b8.gr' | ||
| expected = 'CONNECT %s:%d HTTP/1.1\r\n'.encode('ascii') % ( |
There was a problem hiding this comment.
Is there a reason to not use b'CONNECT %s:%d HTTP/1.1\r\n' instead? We already know that "HTTP/1.1", dest.encode('idna') and client.HTTP_PORT don't contain any non-ASCII chars so there is no need to encode it ASCII.
|
And please rebase your branch so we can run the tests on AppVeyor. |
| @@ -1877,6 +1877,16 @@ def test_connect_with_tunnel(self): | ||
| # This test should be removed when CONNECT gets the HTTP/1.1 blessing |
There was a problem hiding this comment.
This comment is now redundant and can be removed, but I don't see a reason to remove the test itself.
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
|
Superseded by PR #8305. |
Original author of this patch demianbrecht@gmail.com
HTTP/1.0 not supported CONNECT method, fix http version.
Support for IDN.
https://bugs.python.org/issue22708