Remove validate

changelog.d/16060.misc

@@ -0,0 +1 @@
+Add logging of sender invalid mxids when persisting events and receiving EDUs.

synapse/handlers/device.py

@@ -1106,6 +1106,10 @@ class DeviceListUpdater(DeviceListWorkerUpdater):
             )
         prev_ids = [str(p) for p in prev_ids]  # They may come as ints
 
+        # The result of `is_valid` is not used yet because for now we only want to
+        # log invalid mxids in the wild.
+        UserID.is_valid(user_id, allow_historical_mxids=True)
+
         if get_domain_from_id(user_id) != origin:
             # TODO: Raise?
             logger.warning(

synapse/handlers/devicemessage.py

@@ -109,6 +109,10 @@ class DeviceMessageHandler:
                 origin,
                 sender_user_id,
             )
+        # The result of `is_valid` is not used yet because for now we only want to
+        # log invalid mxids in the wild.
+        UserID.is_valid(sender_user_id, allow_historical_mxids=True)
+
         message_type = content["type"]
         message_id = content["message_id"]
         for user_id, by_device in content["messages"].items():

synapse/handlers/e2e_keys.py

@@ -1593,6 +1593,10 @@ class SigningKeyEduUpdater:
             logger.warning("Got signing key update edu for %r from %r", user_id, origin)
             return
 
+        # The result of `is_valid` is not used yet because for now we only want to
+        # log invalid mxids in the wild.
+        UserID.is_valid(user_id, allow_historical_mxids=True)
+
         room_ids = await self.store.get_rooms_for_user(user_id)
         if not room_ids:
             # We don't share any rooms with this user. Ignore update, as we

synapse/handlers/receipts.py

@@ -117,6 +117,10 @@ class ReceiptsHandler:
         max_batch_id: Optional[int] = None
 
         for receipt in receipts:
+            # The result of `is_valid` is not used yet because for now we only want to
+            # log invalid mxids in the wild.
+            UserID.is_valid(receipt.user_id, allow_historical_mxids=True)
+
             res = await self.store.insert_receipt(
                 receipt.room_id,
                 receipt.receipt_type,

synapse/handlers/typing.py

@@ -370,6 +370,10 @@ class TypingWriterHandler(FollowerTypingHandler):
         room_id = content["room_id"]
         user_id = content["user_id"]
 
+        # The result of `is_valid` is not used yet because for now we only want to
+        # log invalid mxids in the wild.
+        UserID.is_valid(user_id, allow_historical_mxids=True)
+
         # If we're not in the room just ditch the event entirely. This is
         # probably an old server that has come back and thinks we're still in
         # the room (or we've been rejoined to the room by a state reset).

synapse/storage/controllers/persist_events.py

@@ -63,6 +63,7 @@ from synapse.types import (
     PersistedEventPosition,
     RoomStreamToken,
     StateMap,
+    UserID,
     get_domain_from_id,
 )
 from synapse.types.state import StateFilter
@@ -397,6 +398,10 @@ class EventsPersistenceStorageController:
         event_ids: List[str] = []
         partitioned: Dict[str, List[Tuple[EventBase, EventContext]]] = {}
         for event, ctx in events_and_contexts:
+            # The result of `is_valid` is not used yet because for now we only want to
+            # log invalid mxids in the wild.
+            UserID.is_valid(event.user_id, allow_historical_mxids=True)
+
             partitioned.setdefault(event.room_id, []).append((event, ctx))
             event_ids.append(event.event_id)
 

synapse/types/__init__.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import abc
+import logging
 import re
 import string
 from enum import Enum
@@ -64,6 +65,9 @@ if TYPE_CHECKING:
     from synapse.storage.databases.main import DataStore, PurgeEventsStore
     from synapse.storage.databases.main.appservice import ApplicationServiceWorkerStore
 
+logger = logging.getLogger(__name__)
+
+
 # Define a state map type from type/state_key to T (usually an event ID or
 # event)
 T = TypeVar("T")
@@ -306,7 +310,7 @@ class DomainSpecificString(metaclass=abc.ABCMeta):
         return "%s%s:%s" % (self.SIGIL, self.localpart, self.domain)
 
     @classmethod
-    def is_valid(cls: Type[DS], s: str) -> bool:
+    def is_valid(cls: Type[DS], s: str, **kwargs: Any) -> bool:
         """Parses the input string and attempts to ensure it is valid."""
         # TODO: this does not reject an empty localpart or an overly-long string.
         # See https://spec.matrix.org/v1.2/appendices/#identifier-grammar
@@ -329,6 +333,35 @@ class UserID(DomainSpecificString):
 
     SIGIL = "@"
 
+    @classmethod
+    def is_valid(cls: Type[DS], s: str, **kwargs: Any) -> bool:
+        """"""
+        """Parses the user id str and attempts to ensure it is valid per the spec.
+
+        Args:
+            allow_historical_mxids: True to allow historical mxids, which can
+            include all printable ASCII chars minus `:`
+        Returns:
+            False if the user ID is invalid per the spec
+        """
+        allow_historical_mxids = kwargs.get("allow_historical_mxids", False)
+
+        is_valid = DomainSpecificString.is_valid(s)
+
+        if len(s.encode("utf-8")) > 255:
+            logger.warn(
+                f"User ID {s} has more than 255 bytes and is invalid per the spec"
+            )
+            is_valid = False
+        obj = UserID.from_string(s)
+        if contains_invalid_mxid_characters(obj.localpart, allow_historical_mxids):
+            logger.warn(
+                f"localpart of User ID {s} contains invalid characters per the spec"
+            )
+            is_valid = False
+
+        return is_valid
+
 
 @attr.s(slots=True, frozen=True, repr=False)
 class RoomAlias(DomainSpecificString):
@@ -355,22 +388,30 @@ MXID_LOCALPART_ALLOWED_CHARACTERS = set(
     "_-./=+" + string.ascii_lowercase + string.digits
 )
 
+MXID_HISTORICAL_LOCALPART_ALLOWED_CHARACTERS = set(string.printable.replace(":", ""))
+
 # Guest user IDs are purely numeric.
 GUEST_USER_ID_PATTERN = re.compile(r"^\d+$")
 
 
-def contains_invalid_mxid_characters(localpart: str) -> bool:
+def contains_invalid_mxid_characters(
+    localpart: str, allow_historical_mxids: Optional[bool] = False
+) -> bool:
     """Check for characters not allowed in an mxid or groupid localpart
 
     Args:
         localpart: the localpart to be checked
-        use_extended_character_set: True to use the extended allowed characters
-            from MSC4009.
-
+        allow_historical_mxids: True to allow historical mxids, which can
+        include all printable ASCII chars minus `:`
     Returns:
         True if there are any naughty characters
     """
-    return any(c not in MXID_LOCALPART_ALLOWED_CHARACTERS for c in localpart)
+
+    if allow_historical_mxids:
+        allowed_characters = MXID_HISTORICAL_LOCALPART_ALLOWED_CHARACTERS
+    else:
+        allowed_characters = MXID_LOCALPART_ALLOWED_CHARACTERS
+    return any(c not in allowed_characters for c in localpart)
 
 
 UPPER_CASE_PATTERN = re.compile(b"[A-Z_]")