URLs that infinitely redirect to an authenticated Location will raise a ValueError instead of TooManyRedirects

[PLPeeters]

Describe the bug

I encountered a URL that seems to always respond with this, even for subsequent authenticated calls:

HTTP/1.1 301 Moved Permanently
Server: nginx/1.18.0 (Ubuntu)
Date: Wed, 09 Oct 2024 07:12:26 GMT
Content-Type: text/html; charset=UTF-8
Connection: keep-alive
X-redirected: 1
Location: http://bourospan@itheka.gr

This then leads to a ValueError once the redirect is handled, instead of a TooManyRedirects error (which is what both Chrome and requests throw for this).

To Reproduce

import aiohttp
import asyncio


async def main():
    async with aiohttp.ClientSession() as session:
        await session.get('http://itheka.gr', allow_redirects=True)


if __name__ == "__main__":
    asyncio.run(main())

Expected behavior

It should raise a TooManyRedirectsError instead. A ValueError gives the impression the user did something wrong, while this is out of their control.

Logs/tracebacks

>>> import aiohttp
>>> import asyncio
>>> 
>>> 
>>> async def main():
...     async with aiohttp.ClientSession() as session:
...         await session.get('http://itheka.gr', allow_redirects=True)
... 
>>> 
>>> if __name__ == "__main__":
...     asyncio.run(main())
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "/opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "<stdin>", line 3, in main
  File "/Users/peetersp/Documents/Swan/website-processors/venv/lib/python3.12/site-packages/aiohttp/client.py", line 591, in _request
    raise ValueError(
ValueError: Cannot combine AUTH argument with credentials encoded in URL

Python Version

$ python --version
Python 3.12.4

aiohttp Version

$ python -m pip show aiohttp
Name: aiohttp
Version: 3.10.9
Summary: Async http client/server framework (asyncio)
Home-page: https://github.com/aio-libs/aiohttp
Author: 
Author-email: 
License: Apache 2
Location: /Users/peetersp/Documents/Swan/website-processors/venv/lib/python3.12/site-packages
Requires: aiohappyeyeballs, aiosignal, attrs, frozenlist, multidict, yarl
Required-by:

multidict Version

$ python -m pip show multidict
Name: multidict
Version: 6.1.0
Summary: multidict implementation
Home-page: https://github.com/aio-libs/multidict
Author: Andrew Svetlov
Author-email: andrew.svetlov@gmail.com
License: Apache 2
Location: /Users/peetersp/Documents/Swan/website-processors/venv/lib/python3.12/site-packages
Requires: 
Required-by: aiohttp, yarl

propcache Version

$ python -m pip show propcache
WARNING: Package(s) not found: propcache

yarl Version

$ python -m pip show yarl
Name: yarl
Version: 1.12.1
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.0
Location: /Users/peetersp/Documents/Swan/website-processors/venv/lib/python3.12/site-packages
Requires: idna, multidict
Required-by: aiohttp

OS

Ubuntu, macOS

Related component

Client

Additional context

The root cause lies here:

aiohttp/aiohttp/client.py

Lines 558 to 562 in c4a3f27

	if auth and auth_from_url:
	raise ValueError(
	"Cannot combine AUTH argument with "
	"credentials encoded in URL"
	)

I believe this could be fixed in one of two ways:

1. Checking whether auth and auth_from_url are different before raising
2. Only checking this for the initial URL and skipping the check for redirects

Option 2 is probably best, as the problem at hand is that URL-based auth can be out of the user's control, so it should only raise a ValueError if it is indeed the user that made a mistake, which can only really be the case for the initial URL in the redirect chain.

I'll happily PR this once I get the go-ahead for the method to use.

Code of Conduct

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

[Dreamsorcerer]

I don't think that check should be happening on redirects, only on the original url parameter. So, option 2.
I'm also wondering if we should be reusing that auth in a redirect at all, but that's a separate issue...

[Dreamsorcerer]

It probably makes sense to just move that check to somewhere before the loop.