Split up event validation between event and builder

The validator was being run on the EventBuilder objects, and so the
validator only checked a subset of fields. With the upcoming
EventBuilder refactor even fewer fields will be there to validate.

To get around this we split the validation into those that can be run
against an EventBuilder and those run against a fully fledged event.

3 files changed, 61 insertions, 23 deletions

diff --git a/synapse/events/validator.py b/synapse/events/validator.py
index cf184748a1..55d44d093d 100644
--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -24,14 +24,13 @@ class EventValidator(object):
 
     def validate(self, event):
         EventID.from_string(event.event_id)
-        RoomID.from_string(event.room_id)
 
         required = [
-            # "auth_events",
+            "auth_events",
             "content",
-            # "hashes",
+            "hashes",
             "origin",
-            # "prev_events",
+            "prev_events",
             "sender",
             "type",
         ]
@@ -43,33 +42,60 @@ class EventValidator(object):
         # Check that the following keys have string values
         strings = [
             "origin",
-            "sender",
-            "type",
         ]
 
-        if hasattr(event, "state_key"):
-            strings.append("state_key")
-
         for s in strings:
             if not isinstance(getattr(event, s), string_types):
                 raise SynapseError(400, "Not '%s' a string type" % (s,))
 
-        if event.type == EventTypes.Member:
-            if "membership" not in event.content:
-                raise SynapseError(400, "Content has not membership key")
+    def validate_new(self, event):
+        """Validates the event has roughly the right format
 
-            if event.content["membership"] not in Membership.LIST:
-                raise SynapseError(400, "Invalid membership key")
+        Args:
+            event (FrozenEvent)
+        """
+        self.validate_builder(event)
+        self.validate(event)
 
-        # Check that the following keys have dictionary values
-        # TODO
+        UserID.from_string(event.sender)
+
+        if event.type == EventTypes.Message:
+            strings = [
+                "body",
+                "msgtype",
+            ]
 
-        # Check that the following keys have the correct format for DAGs
-        # TODO
+            self._ensure_strings(event.content, strings)
 
-    def validate_new(self, event):
-        self.validate(event)
+        elif event.type == EventTypes.Topic:
+            self._ensure_strings(event.content, ["topic"])
 
+        elif event.type == EventTypes.Name:
+            self._ensure_strings(event.content, ["name"])
+
+    def validate_builder(self, event):
+        """Validates that the builder/event has roughly the right format. Only
+        checks values that we expect a proto event to have, rather than all the
+        fields an event would have
+
+        Args:
+            event (EventBuilder|FrozenEvent)
+        """
+
+        strings = [
+            "room_id",
+            "sender",
+            "type",
+        ]
+
+        if hasattr(event, "state_key"):
+            strings.append("state_key")
+
+        for s in strings:
+            if not isinstance(getattr(event, s), string_types):
+                raise SynapseError(400, "Not '%s' a string type" % (s,))
+
+        RoomID.from_string(event.room_id)
         UserID.from_string(event.sender)
 
         if event.type == EventTypes.Message:
@@ -86,6 +112,13 @@ class EventValidator(object):
         elif event.type == EventTypes.Name:
             self._ensure_strings(event.content, ["name"])
 
+        elif event.type == EventTypes.Member:
+            if "membership" not in event.content:
+                raise SynapseError(400, "Content has not membership key")
+
+            if event.content["membership"] not in Membership.LIST:
+                raise SynapseError(400, "Invalid membership key")
+
     def _ensure_strings(self, d, keys):
         for s in keys:
             if s not in d:
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index a4b771049c..13333818ae 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -2278,7 +2278,7 @@ class FederationHandler(BaseHandler):
             room_version = yield self.store.get_room_version(room_id)
             builder = self.event_builder_factory.new(room_version, event_dict)
 
-            EventValidator().validate_new(builder)
+            EventValidator().validate_builder(builder)
             event, context = yield self.event_creation_handler.create_new_client_event(
                 builder=builder
             )
@@ -2287,6 +2287,8 @@ class FederationHandler(BaseHandler):
                 room_version, event_dict, event, context
             )
 
+            EventValidator().validate_new(event)
+
             try:
                 yield self.auth.check_from_context(event, context)
             except AuthError as e:
@@ -2372,10 +2374,11 @@ class FederationHandler(BaseHandler):
             # auth check code will explode appropriately.
 
         builder = self.event_builder_factory.new(room_version, event_dict)
-        EventValidator().validate_new(builder)
+        EventValidator().validate_builder(builder)
         event, context = yield self.event_creation_handler.create_new_client_event(
             builder=builder,
         )
+        EventValidator().validate_new(event)
         defer.returnValue((event, context))
 
     @defer.inlineCallbacks
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 7aaa4fba33..d2aab25111 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -288,7 +288,7 @@ class EventCreationHandler(object):
 
         builder = self.event_builder_factory.new(room_version, event_dict)
 
-        self.validator.validate_new(builder)
+        self.validator.validate_builder(builder)
 
         if builder.type == EventTypes.Member:
             membership = builder.content.get("membership", None)
@@ -326,6 +326,8 @@ class EventCreationHandler(object):
             prev_events_and_hashes=prev_events_and_hashes,
         )
 
+        self.validator.validate_new(event)
+
         defer.returnValue((event, context))
 
     def _is_exempt_from_privacy_policy(self, builder, requester):