TLS-in-TLS warning is emitted even if the proxy is plain HTTP

[ffissore]

Describe the bug

When an HTTPS url is access through a plain HTTP proxy, function _warn_about_tls_in_tls is called, providing misleading information

In version 3.8.3, code is https://github.com/aio-libs/aiohttp/blob/v3.8.3/aiohttp/connector.py#L1247-L1249
Shouldn't it be something like this?

if req.is_ssl():
    if proxy_req.is_ssl() and runtime_has_start_tls:
        self._warn_about_tls_in_tls(transport, req)

Or even passing proxy_req to _warn_about_tls_in_tls and let the function decide

To Reproduce

1. Make an HTTPS call through a plain HTTP proxy

Expected behavior

It should not emit a warning

Logs/tracebacks

None

Python Version

$ python --version
Python 3.7.16

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.8.3
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /home/federico/.pyenv/versions/3.7.16/envs/NBX-card/lib/python3.7/site-packages
Requires: aiosignal, async-timeout, asynctest, attrs, charset-normalizer, frozenlist, multidict, typing-extensions, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.0.3
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /home/federico/.pyenv/versions/3.7.16/envs/NBX-card/lib/python3.7/site-packages
Requires: 
Required-by: aiohttp, yarl

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.8.2
Summary: Yet another URL library
Home-page: https://github.com/aio-libs/yarl/
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /home/federico/.pyenv/versions/3.7.16/envs/NBX-card/lib/python3.7/site-packages
Requires: idna, multidict, typing-extensions
Required-by: aiohttp

OS

Ubuntu Linux

Related component

Client

Additional context

No response

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[webknjaz]

@ffissore I suppose that the warning message might need to be tuned. But ultimately, starttls() is used if runtime_has_start_tls: https://github.com/aio-libs/aiohttp/blob/v3.8.3/aiohttp/connector.py#L1325.

FWIW, somebody would need to go down the rabbit hole to figure out the effect of changing the if-clause to judge whether it's correct. I simply don't remember why exactly I've structured the runtime check like this ­it seems like I didn't document this specific bit in c29e5fb or #5992.
FWIW code on master and 3.9 branch is simpler — it doesn't have a non-starttls() path due to Python 3.6 support being dropped (btw @Dreamsorcerer — it looks like aiohttp3.9 still lists py3.6 in trove classifiers for some reason).

Looking into this more, it seems like the proposed conditional clause is correct. Feel free to send a PR.
Also, it'd be nice to have the integration tests fixed to get rid of the xfails. Maybe you or somebody from @aio-libs/triagers would like to take a look...

[zhu]

_warn_about_tls_in_tls is only called in

aiohttp/aiohttp/connector.py

Lines 1247 to 1249 in 30b7a4e

	if req.is_ssl():
	if runtime_has_start_tls:
	self._warn_about_tls_in_tls(transport, req)

and unnecessary check the req is ssl twice.

aiohttp/aiohttp/connector.py

Lines 1040 to 1041 in 30b7a4e

	if req.request_info.url.scheme != "https":
	return

I think pass proxy_req to _warn_about_tls_in_tls is right change:

if req.is_ssl():
    if runtime_has_start_tls:
        self._warn_about_tls_in_tls(transport, proxy_req)