aio-libs / aiobotocore

asyncio support for botocore library using aiohttp
https://aiobotocore.aio-libs.org
Apache License 2.0
1.16k stars 182 forks source link

Proxy not working due to signature mismatch #1070

Closed MPK1 closed 6 months ago

MPK1 commented 8 months ago

Description

The method _setup_proxy_ssl_context expects a string but is passed a dict here. At the parse_url() call, the connection establishment will fail as the dict cannot be parsed as url and does not even contain the url (which is why I can't think of an easy fix right now).

In commit fbfeb1242ab1f378f6d4fe84b5d3dacdd8874189, the signature was changed but the call was not updated. Reverting the changes applied to _setup_proxy_ssl_context does solve the problem.

In botocore, _setup_proxy_ssl_context is called in the send method where the request is available.

Checklist

pip freeze results

aiobotocore==2.9.0
aiohttp==3.9.1
aioitertools==0.11.0
aiosignal==1.3.1
attrs==23.2.0
botocore==1.33.13
certifi==2023.11.17
frozenlist==1.4.1
fsspec==2023.12.2
idna==3.6
jmespath==1.0.1
multidict==6.0.4
python-dateutil==2.8.2
s3fs==2023.12.2
six==1.16.0
urllib3==2.0.7
wrapt==1.16.0
yarl==1.9.4

Environment:

thehesiod commented 8 months ago

oh man quite a bit of work (and research) is needed here. Going to take a bit to get to. Welcome to PRs!

MPK1 commented 8 months ago

oh man quite a bit of work (and research) is needed here. Going to take a bit to get to. Welcome to PRs!

That's what it also looked like to me...

thehesiod commented 8 months ago

probably my fault, or bit rot

jakob-keller commented 8 months ago

Do we even need _setup_proxy_ssl_context?

According to aiohttp documentation:

When supplying a custom ssl.SSLContext instance, bear in mind that it will be used not only to establish a TLS session with the HTTPS endpoint you’re hitting but also to establish a TLS tunnel to the HTTPS proxy. To avoid surprises, make sure to set up the trust chain that would recognize TLS certificates used by both the endpoint and the proxy.

Maybe we should think about ways to merge both TLS configurations into a single SSLContext?