Return 400 on a malformed request envelope instead of 500 (#19)

sdk/python/src/ldp_protocol/delegate.py

@@ -2,8 +2,9 @@
 
 from __future__ import annotations
 
+import json
 from abc import ABC, abstractmethod
-from typing import Any
+from typing import TYPE_CHECKING, Any
 from uuid import uuid4
 
 from ldp_protocol.types.capability import LdpCapability
@@ -15,6 +16,9 @@
 from ldp_protocol.types.trust import TrustDomain
 from ldp_protocol.types.verification import VerificationStatus
 
+if TYPE_CHECKING:
+    from starlette.applications import Starlette
+
 
 class LdpDelegate(ABC):
     """Base class for LDP delegates.
@@ -321,8 +325,12 @@ def _handle_session_close(self, envelope: LdpEnvelope) -> LdpEnvelope:
             body=LdpMessageBody.session_close(reason="acknowledged"),
         )
 
-    def run(self, host: str = "0.0.0.0", port: int = 8090) -> None:
-        """Run the delegate as an HTTP server using Starlette + uvicorn.
+    def _build_app(self) -> "Starlette":
+        """Build the Starlette ASGI app that exposes this delegate over HTTP.
+
+        Separated from ``run`` so the routes — in particular the envelope
+        parsing in ``/ldp/messages`` — can be exercised with a test client
+        without binding a socket.
 
         Requires: pip install ldp-protocol[server]
         """
@@ -331,15 +339,23 @@ def run(self, host: str = "0.0.0.0", port: int = 8090) -> None:
             from starlette.requests import Request
             from starlette.responses import JSONResponse
             from starlette.routing import Route
-            import uvicorn
         except ImportError:
-            raise ImportError(
-                "Server dependencies not installed. Run: pip install ldp-protocol[server]"
-            )
+            raise ImportError("Starlette is not installed. Run: pip install ldp-protocol[server]")
+
+        from pydantic import ValidationError
 
         identity = self.identity
         delegate = self
 
+        def _bad_request(message: str) -> JSONResponse:
+            # A malformed request is the peer's error, not ours: answer with a
+            # structured LDP error at 400 rather than leaking the parse failure
+            # as an unhandled 500.
+            return JSONResponse(
+                {"error": LdpError.transport("MALFORMED_ENVELOPE", message).model_dump()},
+                status_code=400,
+            )
+
         async def handle_identity(request: Request) -> JSONResponse:
             # Serve the honest, advertised view (modes narrowed to installed
             # codecs) so discovery agrees with what negotiation will accept.
@@ -350,12 +366,24 @@ async def handle_capabilities(request: Request) -> JSONResponse:
             return JSONResponse({"capabilities": caps})
 
         async def handle_messages(request: Request) -> JSONResponse:
-            data = await request.json()
-            envelope = LdpEnvelope.model_validate(data)
+            # Parse the envelope defensively. Bad JSON (JSONDecodeError) or a bad
+            # encoding (UnicodeDecodeError) is the peer's fault → 400; a malformed
+            # but well-encoded envelope raises ValidationError → 400. Anything
+            # else (a server/transport fault, or a bug inside handle_message)
+            # propagates as a 500 — it is ours, not the peer's, and must stay
+            # observable rather than be mislabelled a bad request.
+            try:
+                data = await request.json()
+            except (json.JSONDecodeError, UnicodeDecodeError):
+                return _bad_request("Request body is not valid JSON")
+            try:
+                envelope = LdpEnvelope.model_validate(data)
+            except ValidationError:
+                return _bad_request("Request body is not a valid LDP envelope")
             response = await delegate.handle_message(envelope)
             return JSONResponse(response.model_dump(by_alias=True))
 
-        app = Starlette(
+        return Starlette(
             routes=[
                 Route("/.well-known/ldp-identity", handle_identity, methods=["GET"]),
                 Route("/ldp/identity", handle_identity, methods=["GET"]),
@@ -364,6 +392,18 @@ async def handle_messages(request: Request) -> JSONResponse:
             ],
         )
 
+    def run(self, host: str = "0.0.0.0", port: int = 8090) -> None:
+        """Run the delegate as an HTTP server using Starlette + uvicorn.
+
+        Requires: pip install ldp-protocol[server]
+        """
+        try:
+            import uvicorn
+        except ImportError:
+            raise ImportError("uvicorn is not installed. Run: pip install ldp-protocol[server]")
+
+        app = self._build_app()
+        identity = self.identity
         self.identity.endpoint = f"http://{host}:{port}"
         print(f"LDP Delegate '{identity.name}' starting on {host}:{port}")
         print(f"  ID: {identity.delegate_id}")

sdk/python/tests/test_delegate.py

@@ -1,6 +1,7 @@
 """Tests for LDP delegate base class."""
 
 import pytest
+
 from ldp_protocol.delegate import LdpDelegate
 from ldp_protocol.types import (
     LdpCapability,
@@ -376,3 +377,58 @@ async def test_default_hooks_are_noops(self):
             "s1", "client", "delegate", body=LdpMessageBody.session_close("done")
         )
         assert (await delegate.handle_message(close)).body.type == "SESSION_CLOSE"
+
+
+class TestHttpRouteEnvelopeParsing:
+    """The /ldp/messages route must answer malformed input with a 400 (a
+    structured LDP error), not leak the parse failure as an unhandled 500."""
+
+    def _client(self):
+        # starlette ships in the optional [server] extra, not [dev]: skip only
+        # the route tests (not the whole module) when it is absent.
+        testclient = pytest.importorskip("starlette.testclient")
+        return testclient.TestClient(_make_delegate()._build_app())
+
+    def test_malformed_json_returns_400(self):
+        resp = self._client().post("/ldp/messages", content=b"{ not valid json")
+        assert resp.status_code == 400
+        assert resp.json()["error"]["code"] == "MALFORMED_ENVELOPE"
+
+    def test_invalid_envelope_returns_400(self):
+        # Well-formed JSON, but not a valid envelope (the required `body` is
+        # absent) — model_validate raises, and the route must map it to 400.
+        resp = self._client().post("/ldp/messages", json={"session_id": "s1"})
+        assert resp.status_code == 400
+        assert resp.json()["error"]["code"] == "MALFORMED_ENVELOPE"
+
+    def test_valid_envelope_still_dispatches(self):
+        # The hardening must not change the happy path: a real envelope still
+        # routes through handle_message and returns 200.
+        env = LdpEnvelope.create(
+            "", "client", "delegate", body=LdpMessageBody.hello("client", [PayloadMode.TEXT])
+        )
+        resp = self._client().post("/ldp/messages", json=env.model_dump(by_alias=True))
+        assert resp.status_code == 200
+        assert resp.json()["body"]["type"] == "CAPABILITY_MANIFEST"
+
+    def test_identity_route_unaffected(self):
+        resp = self._client().get("/ldp/identity")
+        assert resp.status_code == 200
+        assert resp.json()["delegate_id"] == "ldp:delegate:echo-test"
+
+    def test_session_accept_serializes_payload_mode(self):
+        # A real SESSION_PROPOSE -> SESSION_ACCEPT round-trip pushes a PayloadMode
+        # enum through JSONResponse; confirm it serializes to its string value
+        # over the wire (the StrEnum path) rather than failing or leaking a repr.
+        env = LdpEnvelope.create(
+            "s1",
+            "client",
+            "delegate",
+            body=LdpMessageBody.session_propose(
+                {"preferred_payload_modes": ["text"], "trust_domain": "default"}
+            ),
+        )
+        resp = self._client().post("/ldp/messages", json=env.model_dump(by_alias=True))
+        assert resp.status_code == 200
+        assert resp.json()["body"]["type"] == "SESSION_ACCEPT"
+        assert resp.json()["body"]["negotiated_mode"] == "text"