Room version 6 tests

tests/00expect_http_fail.pl

@@ -172,3 +172,13 @@ sub expect_m_not_found
    );
 }
 push @EXPORT, qw( expect_m_not_found );
+
+
+sub expect_m_bad_json
+{
+   my $f = shift;
+   return expect_matrix_error(
+      $f, 400, 'M_BAD_JSON',
+   );
+}
+push @EXPORT, qw( expect_m_bad_json );

tests/50federation/30room-join.pl

@@ -1062,25 +1062,15 @@ sub assert_is_valid_pdu {
       )
    };
 
-push @EXPORT, qw( expect_bad_json );
-
-sub expect_bad_json {
-   my ($f) = @_;
-
-   $f->main::expect_http_400
-   ->then(sub {
-      # done
-      my ($response) = @_;
-      log_if_fail "Response", $response;
-      my $body = decode_json($response->content);
-      log_if_fail "Body", $body;
-
-      assert_eq($body->{errcode}, "M_BAD_JSON", 'responsecode');
-      Future->done(1);
-   });
-};
-
-test "Inbound: room version 6 rejects invalid JSON",
+# A homeserver receiving a `send_join` request for a room version 6 room with
+# a bad JSON value (e.g. a float) should reject the request.
+#
+# To test this we need to:
+# * Send a successful `make_join` request.
+# * Add a "bad" value into the returned prototype event.
+# * Make a request to `send_join`.
+# * Check that the response is M_BAD_JSON.
+test "Inbound: send_join rejects invalid JSON for room version 6 rejects",
    requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER,
                  local_user_and_room_fixtures( room_opts => { room_version => "6" } ),
                  federation_user_id_fixture() ],
@@ -1104,32 +1094,10 @@ sub expect_bad_json {
 
          log_if_fail "make_join body", $body;
 
-         assert_json_keys( $body, qw( event ));
-
          my $protoevent = $body->{event};
 
-         assert_json_keys( $protoevent, qw(
-            auth_events content depth room_id sender state_key type
-         ));
-
-         assert_json_nonempty_list( $protoevent->{auth_events} );
-
-         assert_json_nonempty_list( $protoevent->{prev_events} );
-
-         assert_json_number( $protoevent->{depth} );
-
-         $protoevent->{room_id} eq $room_id or
-            die "Expected 'room_id' to be $room_id";
-         $protoevent->{sender} eq $user_id or
-            die "Expected 'sender' to be $user_id";
-         $protoevent->{state_key} eq $user_id or
-            die "Expected 'state_key' to be $user_id";
-         $protoevent->{type} eq "m.room.member" or
-            die "Expected 'type' to be 'm.room.member'";
-
-         assert_json_keys( my $content = $protoevent->{content}, qw( membership ) );
-         $content->{membership} eq "join" or
-            die "Expected 'membership' to be 'join'";
+         # It is assumed that the make_join response is sane, other tests ensure
+         # this behavior.
 
          my %event = (
             ( map { $_ => $protoevent->{$_} } qw(
@@ -1140,7 +1108,7 @@ sub expect_bad_json {
             origin_server_ts => $inbound_server->time_ms,
          );
          # Insert a "bad" value into the send join, in this case a float.
-         ${event}{contents}{bad_val} = 1.1;
+         ${event}{content}{bad_val} = 1.1;
 
          $datastore->sign_event( \%event );
 
@@ -1150,5 +1118,5 @@ sub expect_bad_json {
             uri      => "/v2/send_join/$room_id/xxx",
             content => \%event,
          )
-      })->main::expect_bad_json;
+      })->main::expect_m_bad_json;
    };

tests/50federation/33room-get-missing-events.pl

@@ -1,7 +1,6 @@
 test "Outbound federation can request missing events",
    requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER,
                  local_user_and_room_fixtures(
-                    user_opts => { with_events => 1 },
                     room_opts => { room_version => "1" },
                    ),
                  federation_user_id_fixture() ],
@@ -412,124 +411,122 @@ sub sytest_user_and_room_fixture {
       });
    };
 
+# A homeserver receiving a response from `get_missing_events` for a version 6
+# room with a bad JSON value (e.g. a float) should discard the bad data.
+#
+# To test this we need to:
+# * Add an event with "bad" data into the room history, but don't send it.
+# * Add a "good" event into the room history and send it.
+# * The homeserver attempts to get the missing event (with the bad data).
+# * Ensure that fetching the event results in an error.
 test "Outbound federation will ignore a missing event with bad JSON for room version 6",
    requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER,
-                 local_user_and_room_fixtures(
-                    user_opts => { with_events => 1 },
-                    room_opts => { room_version => "6" },
-                   ),
-                 federation_user_id_fixture() ],
+                 federated_rooms_fixture( room_opts => { room_version => "6" } ) ],
 
    do => sub {
-      my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id ) = @_;
+      my ( $outbound_client, $inbound_server, $creator, $user_id, @rooms ) = @_;
+
+      my $room = @rooms[0];
+      my $room_id = $room->{room_id};
       my $first_home_server = $creator->server_name;
 
       my $datastore         = $inbound_server->datastore;
 
-      $outbound_client->join_room(
-         server_name => $first_home_server,
-         room_id     => $room_id,
-         user_id     => $user_id,
-         version     => "6",
-      )->then( sub {
-         my ( $room ) = @_;
-
-         # TODO: We happen to know the latest event in the server should be my
-         #   m.room.member state event, but that's a bit fragile
-         my $latest_event = $room->get_current_state_event( "m.room.member", $user_id );
+      # TODO: We happen to know the latest event in the server should be my
+      #   m.room.member state event, but that's a bit fragile
+      my $latest_event = $room->get_current_state_event( "m.room.member", $user_id );
 
-         log_if_fail "Latest event", $latest_event;
+      log_if_fail "Latest event", $latest_event;
 
-         # Generate but don't send an event
-         my $missing_event = $room->create_and_insert_event(
-            type => "m.room.message",
+      # Generate but don't send an event
+      my $missing_event = $room->create_and_insert_event(
+         type => "m.room.message",
 
-            sender  => $user_id,
-            content => {
-               body    => "Message 1",
-               bad_val => 1.1,
-            },
-         );
+         sender  => $user_id,
+         content => {
+            body    => "Message 1",
+            # Insert a bad value here so that this event cannot be fetched.
+            bad_val => 1.1,
+         },
+      );
 
-         log_if_fail "Missing event", $missing_event;
+      log_if_fail "Missing event", $missing_event;
 
-         # Generate another one and do send it so it will refer to the
-         # previous in its prev_events field
-         my $sent_event = $room->create_and_insert_event(
-            type => "m.room.message",
+      # Generate another one and do send it so it will refer to the
+      # previous in its prev_events field
+      my $sent_event = $room->create_and_insert_event(
+         type => "m.room.message",
 
-            # This would be done by $room->create_and_insert_event anyway but lets be
-            #   sure for this test
-            prev_events => $room->make_event_refs( $missing_event ),
+         # This would be done by $room->create_and_insert_event anyway but lets be
+         #   sure for this test
+         prev_events => $room->make_event_refs( $missing_event ),
 
-            sender  => $user_id,
-            content => {
-               body => "Message 2",
-            },
-         );
-         my $sent_event_id = $room->id_for_event( $sent_event );
+         sender  => $user_id,
+         content => {
+            body => "Message 2",
+         },
+      );
+      my $sent_event_id = $room->id_for_event( $sent_event );
 
-         log_if_fail "Sent event", $sent_event;
+      log_if_fail "Sent event", $sent_event;
 
-         Future->needs_all(
-            $inbound_server->await_request_get_missing_events( $room_id )
-            ->then( sub {
-               my ( $req ) = @_;
-               my $body = $req->body_from_json;
+      Future->needs_all(
+         $inbound_server->await_request_get_missing_events( $room_id )
+         ->then( sub {
+            my ( $req ) = @_;
+            my $body = $req->body_from_json;
 
-               log_if_fail "Body", $body;
+            log_if_fail "Body", $body;
 
-               assert_json_keys( $body, qw( earliest_events latest_events limit ));
-               # TODO: min_depth but I have no idea what it does
+            assert_json_keys( $body, qw( earliest_events latest_events limit ));
+            # TODO: min_depth but I have no idea what it does
 
-               assert_json_list( my $earliest = $body->{earliest_events} );
-               @$earliest == 1 or
-                  die "Expected a single 'earliest_event' ID";
-               assert_eq( $earliest->[0], $room->id_for_event( $latest_event ),
-                  'earliest_events[0]' );
+            assert_json_list( my $earliest = $body->{earliest_events} );
+            @$earliest == 1 or
+               die "Expected a single 'earliest_event' ID";
+            assert_eq( $earliest->[0], $room->id_for_event( $latest_event ),
+               'earliest_events[0]' );
 
-               assert_json_list( my $latest = $body->{latest_events} );
-               @$latest == 1 or
-                  die "Expected a single 'latest_events' ID";
-               assert_eq( $latest->[0], $sent_event_id,
-                  'latest_events[0]' );
-
-               my @events = $datastore->get_backfill_events(
-                  start_at    => $latest,
-                  stop_before => $earliest,
-                  limit       => $body->{limit},
-               );
+            assert_json_list( my $latest = $body->{latest_events} );
+            @$latest == 1 or
+               die "Expected a single 'latest_events' ID";
+            assert_eq( $latest->[0], $sent_event_id,
+               'latest_events[0]' );
 
-               log_if_fail "Backfilling", @events;
+            my @events = $datastore->get_backfill_events(
+               start_at    => $latest,
+               stop_before => $earliest,
+               limit       => $body->{limit},
+            );
 
-               $req->respond_json( {
-                  events => \@events,
-               } );
+            log_if_fail "Backfilling", @events;
 
-               log_if_fail "Done here";
+            $req->respond_json( {
+               events => \@events,
+            } );
 
-               Future->done(1);
-            }),
+            Future->done;
+         }),
 
-            # Can't use send_event here because that checks none were rejected.
-            $outbound_client->send_transaction(
-               destination => $first_home_server,
-               pdus => [ $sent_event ],
-            )->then( sub {
-               my ( $body ) = @_;
+         # Can't use send_event here because that checks none were rejected.
+         $outbound_client->send_transaction(
+            destination => $first_home_server,
+            pdus => [ $sent_event ],
+         )->then( sub {
+            my ( $body ) = @_;
 
-               log_if_fail "Send response", $body;
+            log_if_fail "Send response", $body;
 
-               assert_json_keys( $body, 'pdus' );
-               # 'pdus' is a map from event id to error details.
-               my $pdus = $body->{pdus};
+            assert_json_keys( $body, 'pdus' );
+            # 'pdus' is a map from event id to error details.
+            my $pdus = $body->{pdus};
 
-               # Sending the event fails since fetching the event results in
-               # invalid JSON.
-               assert_json_keys( $pdus, $sent_event_id );
+            # Sending the event fails since fetching the event results in
+            # invalid JSON, thus we expect an error for the sent PDU.
+            assert_json_keys( $pdus, $sent_event_id );
+            assert_json_keys( $pdus->{$sent_event_id}, qw( error ) );
 
-               Future->done;
-            }),
-         );
-      });
+            Future->done;
+         }),
+      );
    };