aiohttp icon indicating copy to clipboard operation
aiohttp copied to clipboard

defaulting to bind to legacy ip address only

Open ukleinek opened this issue 5 years ago • 5 comments

🐞 Describe the bug

aiohttp.web.run_app defaults to bind to 0.0.0.0

💡 To Reproduce

In a Python shell:

from aiohttp import web
app = web.Application()
web.run_app(app)

With that running check bound addresses:

$ netstat -a -n | grep 8080
tcp        0      0 0.0.0.0:8080            0.0.0.0:*               LISTEN     

💡 Expected behavior

This should bind to all available address families.

Something like the following change could improve the situation (but I fail to setup a working test environment, so won't propose a PR):

diff --git a/aiohttp/web_runner.py b/aiohttp/web_runner.py
index cdd5ac7ce54c..9009fd8e7a53 100644
--- a/aiohttp/web_runner.py
+++ b/aiohttp/web_runner.py
@@ -85,8 +85,6 @@ class TCPSite(BaseSite):
                  reuse_port: Optional[bool]=None) -> None:
         super().__init__(runner, shutdown_timeout=shutdown_timeout,
                          ssl_context=ssl_context, backlog=backlog)
-        if host is None:
-            host = "0.0.0.0"
         self._host = host
         if port is None:
             port = 8443 if self._ssl_context else 8080
@@ -97,7 +95,16 @@ class TCPSite(BaseSite):
     @property
     def name(self) -> str:
         scheme = 'https' if self._ssl_context else 'http'
-        return str(URL.build(scheme=scheme, host=self._host, port=self._port))
+        if self._host:
+            host = self._host
+        elif self._server and self._server.sockets:
+            # There might be more than one socket (e.g. for ipv4 and ipv6);
+            # pick the first as arbitrary
+            host = self._server.sockets[0].getsockname()[0]
+        else:
+            # fall back to ipv4 bind all address (though we might not have ipv4)
+            host = '0.0.0.0'
+        return str(URL.build(scheme=scheme, host=host, port=self._port))
 
     async def start(self) -> None:
         await super().start()

📋 Your version of the Python

$ python3 --version
Python 3.7.3

ukleinek avatar May 25 '20 14:05 ukleinek

What improvement is the proposed change meant to make (what does it solve)? You've only described what it currently does and then said the code is an improvement...

Dreamsorcerer avatar Aug 23 '24 19:08 Dreamsorcerer

The ask was probably to use :: (IPv6+IPv4 dual-stack) instead of 0.0.0.0 (IPv4-only). However, https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_server implies that's the behavior with host=None already. Perhaps, we should just close this issue.

webknjaz avatar Aug 24 '24 00:08 webknjaz

Rechecking the issue today, the reproducer indeed binds to both address families. The only remainder seems to be the irritating message ======== Running on http://0.0.0.0:8080 ========.

Maybe just do:

diff --git a/aiohttp/web_runner.py b/aiohttp/web_runner.py
index f36fcb1ea561..c40164e65229 100644
--- a/aiohttp/web_runner.py
+++ b/aiohttp/web_runner.py
@@ -108,7 +108,7 @@ class TCPSite(BaseSite):
     @property
     def name(self) -> str:
         scheme = "https" if self._ssl_context else "http"
-        host = "0.0.0.0" if not self._host else self._host
+        host = "localhost" if not self._host else self._host
         return str(URL.build(scheme=scheme, host=host, port=self._port))
 
     async def start(self) -> None:

(or use the IMHO simpler host = self._host or "localhost").

There are also a few references to 0.0.0.0 in the docs that might need review.

ukleinek avatar Oct 15 '24 07:10 ukleinek

localhost == 127.0.0.1

I think it'd be incorrect to use localhost in this situation. Given that IPv4 is what most would use locally, I'm not sure it's really worth worrying about the displayed value here. Otherwise, something like <all interfaces> may be more appropriate?

Dreamsorcerer avatar Oct 15 '24 12:10 Dreamsorcerer

A relevant downside of <all interfaces> is that it doesn't work as is in a browser. Actually I was surprised to learn that http://0.0.0.0/ actually works.

While localhost resolves to both ::1 and 127.0.0.1 on my end, after thinking again I agree that using localhost is not optimal either because it's wrong for other reasons.

ukleinek avatar Oct 15 '24 13:10 ukleinek

I encountered a similar issue with aiohttp not automatically trying IPv6 when connecting to localhost.

Even when using TCPConnector(family=0) to try both IPv4 and IPv6:

connector = aiohttp.TCPConnector(family=0)  # 0 means try both IPv4 and IPv6
async with aiohttp.ClientSession(connector=connector) as session:
    async with session.get(url, timeout=timeout) as response:
        # handle response

The connection still fails with "Cannot connect to host localhost:5173 ssl:default [Connect call failed ('127.0.0.1', 5173)]" when the service is only listening on [::1].

The current workaround is to explicitly use "http://[::1]:port" instead of "http://localhost:port", but it would be nice if the library could handle this automatically when family=0 is specified.

razorinc avatar Jan 29 '25 19:01 razorinc