# HG changeset patch # User Goffi # Date 1668532054 -3600 # Node ID acc9dfc8ba8dc4dc5c4a5c28333249213f7fc06d # Parent 9b5092225e460b5de4ea80aaa42d129940ce91a6 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). diff -r 9b5092225e46 -r acc9dfc8ba8d sat/plugins/plugin_comp_ap_gateway/http_server.py --- 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) diff -r 9b5092225e46 -r acc9dfc8ba8d tests/unit/test_ap-gateway.py --- 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 ) )