shouldn `env_proxy` affect after proxy that provided by users from code

[NewUserHa]

Is your feature request related to a problem?

aiohttp/aiohttp/client.py

Lines 502 to 507 in e79b2d5

	if proxy is None:
	proxy = self._default_proxy
	if proxy_auth is None:
	proxy_auth = self._default_proxy_auth

aiohttp/aiohttp/client.py

Lines 609 to 617 in e79b2d5

	if proxy is not None:
	proxy = URL(proxy)
	elif self._trust_env:
	with suppress(LookupError):
	proxy, proxy_auth = get_env_proxy_for_url(url)
	req = self._request_class(
	method,

currently, the async def _request(...)

• uses proxy from env first than from code
• it at the same time causes calling get env proxy from env and registry in each request(aka _request()) call.

Describe the solution you'd like

keep env proxy in session, and use after code proxy that provided by users
also should throw error for socks5 proxy since aiohttp does not work with socks5 currently

Describe alternatives you've considered

N/A

Related component

Client

Additional context

No response

Code of Conduct

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

[asvetlov]

Sorry, I re-read the message several times and still cannot figure out what is the question.

[NewUserHa]

• when both proxy settings in environment variable and ClientSession(proxy='...') exist, the async def _request(...) will use the former. see line 614 and 504, the _default_proxy in 504 is read from latter.
• also here get_env_proxy_for_url(url) will read env and registry each time _request(...) is called, however the env proxy should be kept in session rather than lookup for each request
• aiohttp should explicitly throw error for socks5 proxy if provided, since aiohttp only support HTTP proxy does not support socks5 proxy at all

[asvetlov]

Thanks for the clarification.

[Dreamsorcerer]

Can you create a test in a PR to reproduce the issue?
Looks to me like the provided proxy parameter will override the env var, which I think is the expected behaviour.

[NewUserHa]

was mistake.
correct, the proxy user provided does will override the env var by

 if proxy is not None: 
     proxy = URL(proxy) 
 elif self._trust_env: 
     with suppress(LookupError): 
         proxy, proxy_auth = get_env_proxy_for_url(url) 

the issues only are:

• get_env_proxy_for_url(url) will read env and registry each time _request(...) is called
• the env proxy maybe should be kept in session rather than lookup for each request
• aiohttp should explicitly throw error if socks5 proxy provided, since aiohttp only support HTTP proxy and does not support socks5 proxy at all, and currently will ignore the schema of URL of the provided proxy cause the proxy server directly close the connection

[Dreamsorcerer]

Could probably cache one of those functions if you wanted to open a PR.
Not sure if there was any reason to not cache them originally (maybe memory use, or being able to update the file while an application is running)...

[NewUserHa]

aiohttp/aiohttp/helpers.py

Lines 306 to 320 in 1fa237f

	def get_env_proxy_for_url(url: URL) -> Tuple[URL, Optional[BasicAuth]]:
	"""Get a permitted proxy for the given URL from the env."""
	if url.host is not None and proxy_bypass(url.host):
	raise LookupError(f"Proxying is disallowed for `{url.host!r}`")
	proxies_in_env = proxies_from_env()
	try:
	proxy_info = proxies_in_env[url.scheme]
	except KeyError:
	raise LookupError(f"No proxies found for `{url!s}` in the env")
	else:
	return proxy_info.proxy, proxy_info.proxy_auth

but the proxies_from_env() is no parameter, compared to cache it, I think it can just be stored in session level.
and the explicitly throw error for socks5 proxy not supported should be in a different PR?

[Dreamsorcerer]

Yeah, I think that sounds reasonable.

[NewUserHa]

the get_env_proxy_for_url issue part needs to wait python/cpython#127767 to work