diff --git a/changelog.d/11157.misc b/changelog.d/11157.misc
new file mode 100644
index 0000000000..75444c51d1
--- /dev/null
+++ b/changelog.d/11157.misc
@@ -0,0 +1 @@
+Only allow old Element/Riot Android clients to send read receipts without a request body. All other clients must include a request body as required by the specification. Contributed by @rogersheu.
diff --git a/synapse/rest/client/receipts.py b/synapse/rest/client/receipts.py
index 9770413c61..2b25b9aad6 100644
--- a/synapse/rest/client/receipts.py
+++ b/synapse/rest/client/receipts.py
@@ -13,10 +13,12 @@
# limitations under the License.
import logging
+import re
from typing import TYPE_CHECKING, Tuple
from synapse.api.constants import ReadReceiptEventFields
from synapse.api.errors import Codes, SynapseError
+from synapse.http import get_request_user_agent
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
from synapse.http.site import SynapseRequest
@@ -24,6 +26,8 @@ from synapse.types import JsonDict
from ._base import client_patterns
+pattern = re.compile(r"(?:Element|SchildiChat)/1\.[012]\.")
+
if TYPE_CHECKING:
from synapse.server import HomeServer
@@ -52,7 +56,13 @@ class ReceiptRestServlet(RestServlet):
if receipt_type != "m.read":
raise SynapseError(400, "Receipt type must be 'm.read'")
- body = parse_json_object_from_request(request, allow_empty_body=True)
+ # Do not allow older SchildiChat and Element Android clients (prior to Element/1.[012].x) to send an empty body.
+ user_agent = get_request_user_agent(request)
+ allow_empty_body = False
+ if "Android" in user_agent:
+ if pattern.match(user_agent) or "Riot" in user_agent:
+ allow_empty_body = True
+ body = parse_json_object_from_request(request, allow_empty_body)
hidden = body.get(ReadReceiptEventFields.MSC2285_HIDDEN, False)
if not isinstance(hidden, bool):
diff --git a/tests/rest/client/test_sync.py b/tests/rest/client/test_sync.py
index 95be369d4b..c427686376 100644
--- a/tests/rest/client/test_sync.py
+++ b/tests/rest/client/test_sync.py
@@ -14,6 +14,8 @@
# limitations under the License.
import json
+from parameterized import parameterized
+
import synapse.rest.admin
from synapse.api.constants import (
EventContentFields,
@@ -417,7 +419,30 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
# Test that the first user can't see the other user's hidden read receipt
self.assertEqual(self._get_read_receipt(), None)
- def test_read_receipt_with_empty_body(self):
+ @parameterized.expand(
+ [
+ # Old Element version, expected to send an empty body
+ (
+ "agent1",
+ "Element/1.2.2 (Linux; U; Android 9; MatrixAndroidSDK_X 0.0.1)",
+ 200,
+ ),
+ # Old SchildiChat version, expected to send an empty body
+ ("agent2", "SchildiChat/1.2.1 (Android 10)", 200),
+ # Expected 400: Denies empty body starting at version 1.3+
+ ("agent3", "Element/1.3.6 (Android 10)", 400),
+ ("agent4", "SchildiChat/1.3.6 (Android 11)", 400),
+ # Contains "Riot": Receipts with empty bodies expected
+ ("agent5", "Element (Riot.im) (Android 9)", 200),
+ # Expected 400: Does not contain "Android"
+ ("agent6", "Element/1.2.1", 400),
+ # Expected 400: Different format, missing "/" after Element; existing build that should allow empty bodies, but minimal ongoing usage
+ ("agent7", "Element dbg/1.1.8-dev (Android)", 400),
+ ]
+ )
+ def test_read_receipt_with_empty_body(
+ self, name, user_agent: str, expected_status_code: int
+ ):
# Send a message as the first user
res = self.helper.send(self.room_id, body="hello", tok=self.tok)
@@ -426,8 +451,9 @@ class ReadReceiptsTestCase(unittest.HomeserverTestCase):
"POST",
"/rooms/%s/receipt/m.read/%s" % (self.room_id, res["event_id"]),
access_token=self.tok2,
+ custom_headers=[("User-Agent", user_agent)],
)
- self.assertEqual(channel.code, 200)
+ self.assertEqual(channel.code, expected_status_code)
def _get_read_receipt(self):
"""Syncs and returns the read receipt."""
|
