changeset 3981:acc9dfc8ba8d

component AP gateway: parse body immediately on `POST` request: the body is parsed immediately during a `POST` request: this avoids duplication of code, and allows to check the body data before continuing (will be used to filter some requests in a future patch).
author Goffi <goffi@goffi.org>
date Tue, 15 Nov 2022 18:07:34 +0100
parents 9b5092225e46
children 74f7c10a48bc
files sat/plugins/plugin_comp_ap_gateway/http_server.py tests/unit/test_ap-gateway.py
diffstat 2 files changed, 41 insertions(+), 32 deletions(-) [+]
line wrap: on
line diff
--- a/sat/plugins/plugin_comp_ap_gateway/http_server.py	Tue Nov 15 18:03:07 2022 +0100
+++ b/sat/plugins/plugin_comp_ap_gateway/http_server.py	Tue Nov 15 18:07:34 2022 +0100
@@ -560,6 +560,7 @@
     async def APActorRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: jid.JID,
         node: Optional[str],
         ap_account: str,
@@ -662,6 +663,7 @@
     async def APOutboxPageRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: jid.JID,
         node: Optional[str],
         ap_account: str,
@@ -709,7 +711,7 @@
                 )
                 for item in reversed(items)
             ]
-        data = {
+        ret_data = {
             "@context": ["https://www.w3.org/ns/activitystreams"],
             "id": url,
             "type": "OrderedCollectionPage",
@@ -724,19 +726,20 @@
                 last= metadata["rsm"]["last"]
             except KeyError:
                 last = None
-            data["prev"] = f"{base_url}?{parse.urlencode({'after': last})}"
+            ret_data["prev"] = f"{base_url}?{parse.urlencode({'after': last})}"
         if metadata["rsm"]["index"] != 0:
             try:
                 first= metadata["rsm"]["first"]
             except KeyError:
                 first = None
-            data["next"] = f"{base_url}?{parse.urlencode({'before': first})}"
+            ret_data["next"] = f"{base_url}?{parse.urlencode({'before': first})}"
 
-        return data
+        return ret_data
 
     async def APOutboxRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: jid.JID,
         node: Optional[str],
         ap_account: str,
@@ -794,24 +797,16 @@
     async def APInboxRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: Optional[jid.JID],
         node: Optional[str],
         ap_account: Optional[str],
         ap_url: str,
         signing_actor: Optional[str]
     ) -> None:
+        assert data is not None
         if signing_actor is None:
             raise exceptions.InternalError("signing_actor must be set for inbox requests")
-        try:
-            data = json.load(request.content)
-            if not isinstance(data, dict):
-                raise ValueError("data should be an object")
-        except (json.JSONDecodeError, ValueError) as e:
-            return self.responseCode(
-                request,
-                http.BAD_REQUEST,
-                f"invalid json in inbox request: {e}"
-            )
         await self.checkSigningActor(data, signing_actor)
         activity_type = (data.get("type") or "").lower()
         if not activity_type in ACTIVITY_TYPES_LOWER:
@@ -844,6 +839,7 @@
     async def APFollowersRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: jid.JID,
         node: Optional[str],
         ap_account: Optional[str],
@@ -882,6 +878,7 @@
     async def APFollowingRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict],
         account_jid: jid.JID,
         node: Optional[str],
         ap_account: Optional[str],
@@ -921,6 +918,7 @@
     async def APRequest(
         self,
         request: "HTTPRequest",
+        data: Optional[dict] = None,
         signing_actor: Optional[str] = None
     ) -> None:
         if self.apg.verbose:
@@ -929,14 +927,8 @@
                 "",
                 f"<<< got {request.method.decode()} request - {request.uri.decode()}"
             ]
-            try:
-                data = json.load(request.content)
-            except (json.JSONDecodeError, ValueError):
-                pass
-            else:
+            if data is not None:
                 to_log.append(pformat(data))
-            finally:
-                request.content.seek(0)
             if self.apg.verbose>=3:
                 headers = "\n".join(
                     f"    {k.decode()}: {v.decode()}"
@@ -990,7 +982,7 @@
             if request_type != "shared_inbox":
                 raise exceptions.DataError(f"Invalid request type: {request_type!r}")
             ret_data = await self.APInboxRequest(
-                request, None, None, None, ap_url, signing_actor
+                request, data, None, None, None, ap_url, signing_actor
             )
         elif request_type == "avatar":
             if len(extra_args) != 1:
@@ -1013,7 +1005,7 @@
                 raise exceptions.DataError(f"Invalid request type: {request_type!r}")
             method = getattr(self, f"AP{request_type.title()}Request")
             ret_data = await method(
-                request, account_jid, node, ap_account, ap_url, signing_actor
+                request, data, account_jid, node, ap_account, ap_url, signing_actor
             )
         if ret_data is not None:
             request.setHeader("content-type", CONTENT_TYPE_AP)
@@ -1027,7 +1019,26 @@
             log.info("\n".join(to_log))
         request.finish()
 
-    async def APPostRequest(self, request: "HTTPRequest"):
+    async def APPostRequest(self, request: "HTTPRequest") -> None:
+        try:
+            data = json.load(request.content)
+            if not isinstance(data, dict):
+                log.warning(f"JSON data should be an object (uri={request.uri.decode()})")
+                self.responseCode(
+                    request,
+                    http.BAD_REQUEST,
+                    f"invalid body, was expecting a JSON object"
+                )
+                request.finish()
+                return
+        except (json.JSONDecodeError, ValueError) as e:
+            self.responseCode(
+                request,
+                http.BAD_REQUEST,
+                f"invalid json in inbox request: {e}"
+            )
+            request.finish()
+            return
         try:
             signing_actor = await self.checkSignature(request)
         except exceptions.EncryptionError as e:
@@ -1050,7 +1061,7 @@
 
         # default response code, may be changed, e.g. in case of exception
         try:
-            return await self.APRequest(request, signing_actor)
+            return await self.APRequest(request, data, signing_actor)
         except Exception as e:
             self._onRequestError(failure.Failure(e), request)
 
--- a/tests/unit/test_ap-gateway.py	Tue Nov 15 18:03:07 2022 +0100
+++ b/tests/unit/test_ap-gateway.py	Tue Nov 15 18:07:34 2022 +0100
@@ -629,7 +629,7 @@
         ap_gateway,
         type_: Optional[str] = None,
         url: Optional[str] = None,
-        doc: Optional[Any] = None,
+        data: Optional[dict] = None,
         query_data: Optional[dict] = None,
         signing_actor: Optional[str] = None,
     ) -> Dict[str, Any]:
@@ -637,8 +637,7 @@
 
         @param type_: one of the AP query type (e.g. "outbox")
         @param url: URL to query (mutually exclusif with type_)
-        @param doc: if set, a document to embed in content of the request (will be
-        converted to JSON)
+        @param data: object used as body of the request
         @param query_data: query data as returned by parse.parse_qs
         @return dict with kw params to use
         """
@@ -661,12 +660,11 @@
         request = Request(MagicMock())
         request.path = path.encode()
         request.uri = uri.encode()
-        if doc is not None:
-            request.content = io.BytesIO(json.dumps(doc).encode())
 
         ap_url = parse.urljoin(f"https://{ap_gateway.public_url}", path)
         kwargs = {
             "request": request,
+            "data": data,
             "account_jid": test_jid,
             "node": None,
             "ap_account": test_jid.full(),
@@ -1510,7 +1508,7 @@
             with patch.object(ap_gateway._p, "sendItems") as sendItems:
                 await ap_gateway.server.resource.APInboxRequest(
                     **self.ap_request_params(
-                        ap_gateway, "inbox", doc=like, signing_actor=TEST_AP_ACTOR_ID
+                        ap_gateway, "inbox", data=like, signing_actor=TEST_AP_ACTOR_ID
                     )
                 )
 
@@ -1614,7 +1612,7 @@
             with patch.object(ap_gateway._p, "sendItems") as sendItems:
                 await ap_gateway.server.resource.APInboxRequest(
                     **self.ap_request_params(
-                        ap_gateway, "inbox", doc=like, signing_actor=TEST_AP_ACTOR_ID
+                        ap_gateway, "inbox", data=like, signing_actor=TEST_AP_ACTOR_ID
                     )
                 )