-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fixes #3561 - Python 3 proxy ssl connection issue #3699
base: develop
Are you sure you want to change the base?
Fixes #3561 - Python 3 proxy ssl connection issue #3699
Conversation
Having this PR merged would be very helpful for me and anyone else trying to run boto on python 3 with an http(s) proxy. |
Signed-off-by: Kai Xia(夏恺) <xiaket@gmail.com>
I'm blocked on this, can someone please merge this change? |
Hi @jamesls, @dstufft, (you two look most active of the boto project team members ...) I understand boto is no longer the recommended way forward, but it is still used by projects such as Ansible. Could you have a look at this PR, and see if it could be merged. boto isn't yet 'dead' according to the README. This PR addresses an issue using proxies (as usual in enterprise customers) with boto in Python3. In particular, effecting s3 and route53. Thanks and regards, |
When will this fix be merged? |
Yes, merge would be really nice, as ec2.py is only working on my end with this fix with python 3.7 |
please merge this , this is pending from couple of years now. |
+1 - yes, please merge |
+1, please merge |
All checks have passed and this fix works locally. Why is this still open? |
Please merge! |
* fix some string conversion errors in the python script * force installation of python3 packages * Apply boto/boto#3699 manually to make the boto lib work with Py3 version of http lib
When will this be merged?? |
@varun1231 given that it's been nearly 3 years since I made this PR, it seems unlikely that it will happen any time soon! Also, as there's been no activity on this repository for almost a year, I think it's fair to say this is abandonware and should be avoided. |
When will this be merged ? |
Not ideal, but I'm working around the issue until it's merged: |
This doesn't seem to work for me. I get the following warning, while the error persists:
|
# See discussion about this config option at | ||
# https://groups.google.com/forum/?fromgroups#!topic/boto-dev/teenFvOq2Cc | ||
if config.getbool('Boto', 'send_crlf_after_proxy_auth_headers', False): | ||
sock.sendall("\r\n") | ||
sock.sendall(("\r\n").encode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for .encode()
... just use b'\r\n'
directly
else: | ||
sock.sendall("\r\n") | ||
resp = http_client.HTTPResponse(sock, strict=True, debuglevel=self.debug) | ||
sock.sendall(("\r\n").encode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here -- use b'\r\n'
instead
sock.sendall("\r\n") | ||
resp = http_client.HTTPResponse(sock, strict=True, debuglevel=self.debug) | ||
sock.sendall(("\r\n").encode()) | ||
kwargs = {'sock': sock, 'debuglevel': self.debug} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sock
is not a kwarg (i know Python treats it as one, but imo it's bad form to do so for required params), and debuglevel is in both versions. so seems like it'd be more straightforward to only make the backwards compat logic optional.
kwargs = {}
if six.PY2:
kwargs['strict'] = True
resp = http_client.HTTPResponse(sock, debuglevel=self.debug, **kwargs)
@colin-nolan when is this being merged? We have a similar issue |
@taylor-nguyen I am not a maintainer of this project so I can't tell you. However, given that this PR hasn't been merged in over 3 years, I doubt that it's ever going to be! |
you probably want to look at https://github.com/boto/boto3 instead for Python 3 support |
This worked for me (i.e. referencing the PR source branch directly, rather then the PR itself):
|
The problem is we use Ansible and it still relies on boto. @colin-nolan are you able to address review comments to get it merged? Having these changes in would greatly benefit anyone instead of applying this branch directly |
@@ -793,18 +793,21 @@ def proxy_ssl(self, host=None, port=None): | |||
else: | |||
sock = socket.create_connection((self.proxy, int(self.proxy_port))) | |||
boto.log.debug("Proxy connection: CONNECT %s HTTP/1.0\r\n", host) | |||
sock.sendall("CONNECT %s HTTP/1.0\r\n" % host) | |||
sock.sendall("User-Agent: %s\r\n" % UserAgent) | |||
sock.sendall(("CONNECT %s HTTP/1.0\r\n" % host).encode()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to explicitly say encode("UTF-8")
That's the default under py3, but py2 it depends on sys.getdefaultencoding().
It's likely ok the way it is, but it provides consistency.
Ditto for the other places encode() is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking, non-ASCII hostnames should be "idna"-encoded, rather than UTF-8 or a system's default, as IDNA is the standard for internationalized domain names.
We still use boto and I believe its on the latest , how do we patch it with this ? |
Please merge this branch. The community.aws.route53 module is unusable on python 3.6.8 without the patch included in this branch. I was originally using boto 2.49.0, but found this thread from here. I used the following to install this branch:
I'm now on boto 2.46.1, and the community.aws.route53 module works, but obviously, this should only be a temporary workaround. |
Seconding other comments here. I think we should merge. There has been good discussion and some code changes aren't "perfect" but right now it's broken on py3. Some people are still on boto2. I've had to monkeypatch this issue for production code. |
It would be great to get this merged, it causes so much trouble for us atm :( |
Pull request to finally get the changes in #2718 merged into boto.
proxy_ssl
connection test to show the compatibility issue identified in Boto S3 throws type error in production environment #3561.Alternative to pull requests #2718, #3695.
Fixes #3561; closes #3695.