Merge pull request #100 from lsst-sqre/tickets/DM-34821

[DM-34821] Allow request.client to be None

CHANGELOG.rst

@@ -5,6 +5,11 @@ Change log
 .. Headline template:
    X.Y.Z (YYYY-MM-DD)
 
+3.0.3 (2022-05-16)
+==================
+
+- Correctly handle the possibility that ``request.client`` is ``None``.
+
 3.0.2 (2022-03-25)
 ==================
 

src/safir/dependencies/logger.py

@@ -52,8 +52,9 @@ async def __call__(self, request: Request) -> BoundLogger:
         request_data = {
             "requestMethod": request.method,
             "requestUrl": str(request.url),
-            "remoteIp": request.client.host,
         }
+        if request.client:
+            request_data["remoteIp"] = request.client.host
         user_agent = request.headers.get("User-Agent")
         if user_agent:
             request_data["userAgent"] = user_agent

src/safir/middleware/x_forwarded.py

@@ -87,7 +87,10 @@ async def dispatch(
 
         # Update the request's understanding of the client IP.  This uses an
         # undocumented interface; hopefully it will keep working.
-        request.scope["client"] = (client, request.client.port)
+        if request.client:
+            request.scope["client"] = (client, request.client.port)
+        else:
+            request.scope["client"] = (client, None)
 
         # Ideally this should take the scheme corresponding to the entry in
         # X-Forwarded-For that was chosen, but some proxies (the Kubernetes

tests/middleware/x_forwarded_test.py

@@ -25,6 +25,7 @@ async def test_ok() -> None:
 
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
+        assert request.client
         assert request.client.host == "10.10.10.10"
         assert request.state.forwarded_host == "foo.example.com"
         assert request.url == "https://foo.example.com/"
@@ -49,6 +50,7 @@ async def test_defaults() -> None:
 
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
+        assert request.client
         assert request.client.host == "192.168.0.1"
         assert request.state.forwarded_host == "foo.example.com"
         assert request.url == "http://example.com/"
@@ -73,6 +75,7 @@ async def test_no_forwards() -> None:
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
         assert not request.state.forwarded_host
+        assert request.client
         assert request.client.host == "127.0.0.1"
         assert request.url == "http://example.com/"
         return {}
@@ -88,6 +91,7 @@ async def test_all_filtered() -> None:
 
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
+        assert request.client
         assert request.client.host == "10.10.10.10"
         assert request.state.forwarded_host == "foo.example.com"
         assert request.url == "https://example.com/"
@@ -111,6 +115,7 @@ async def test_one_proto() -> None:
 
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
+        assert request.client
         assert request.client.host == "10.10.10.10"
         assert request.state.forwarded_host == "foo.example.com"
         assert request.url == "https://example.com/"
@@ -135,6 +140,7 @@ async def test_no_proto_or_host() -> None:
     @app.get("/")
     async def handler(request: Request) -> Dict[str, str]:
         assert not request.state.forwarded_host
+        assert request.client
         assert request.client.host == "10.10.10.10"
         assert request.url == "http://example.com/"
         return {}