From 79ac7862ed776f85545cebfecb024725b8b3a9aa Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 27 Apr 2020 10:53:40 -0400 Subject: [PATCH 01/34] Add a test-case for invalid JSON (integers out of range). --- tests/80torture/20json.pl | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/80torture/20json.pl diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl new file mode 100644 index 000000000..f3b56c07e --- /dev/null +++ b/tests/80torture/20json.pl @@ -0,0 +1,27 @@ +# Test integers that are outside of the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]. +test "Invalid JSON integers", + requires => [ local_user_and_room_fixtures() ], + + do => sub { + my ( $user, $room_id ) = @_; + + Future->needs_all( + do_request_json_for( $user, + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => 9007199254740992, # 2 ** 53 + }, + )->followed_by( \&main::expect_http_400 ), + + do_request_json_for( $user, + method => "PUT", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => -9007199254740992, # -2 ** 53 + }, + )->followed_by( \&main::expect_http_400 ), + ); + }; From 695dfcbb9966caeb9b2a57712db28a2725aec120 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 28 Apr 2020 14:17:06 -0400 Subject: [PATCH 02/34] Specify the room version. --- tests/80torture/20json.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index f3b56c07e..1a970e2d6 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -1,6 +1,8 @@ # Test integers that are outside of the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]. test "Invalid JSON integers", - requires => [ local_user_and_room_fixtures() ], + requires => [ local_user_and_room_fixtures( + room_opts => { room_version => "org.matrix.strict_canonicaljson" } + ), ], do => sub { my ( $user, $room_id ) = @_; From d80cb4be40a63d553e2628217e91b4118278c50f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 29 Apr 2020 08:25:23 -0400 Subject: [PATCH 03/34] Add tests for floats. --- tests/80torture/20json.pl | 48 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 1a970e2d6..59f5dd68b 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -27,3 +27,51 @@ )->followed_by( \&main::expect_http_400 ), ); }; + +# Floats (including NaN, Infinity, and -Infinity) should be rejected. +test "Invalid JSON floats", + requires => [ local_user_and_room_fixtures( + room_opts => { room_version => "org.matrix.strict_canonicaljson" } + ), ], + + do => sub { + my ( $user, $room_id ) = @_; + + Future->needs_all( + do_request_json_for( $user, + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => 1.1, + }, + )->followed_by( \&main::expect_http_400 ), + + do_request_json_for( $user, + method => "PUT", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => "NaN", + }, + )->followed_by( \&main::expect_http_400 ), + + do_request_json_for( $user, + method => "PUT", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => "inf", + }, + )->followed_by( \&main::expect_http_400 ), + + do_request_json_for( $user, + method => "PUT", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => "-inf", + }, + )->followed_by( \&main::expect_http_400 ), + ); + }; From 65855e06779bac0fe69d41ffe025b1d3eb72981a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 14 May 2020 13:41:14 -0400 Subject: [PATCH 04/34] Switch to room version 6 instead of experimental version. --- tests/80torture/20json.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 59f5dd68b..2352f855d 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -1,7 +1,7 @@ # Test integers that are outside of the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]. test "Invalid JSON integers", requires => [ local_user_and_room_fixtures( - room_opts => { room_version => "org.matrix.strict_canonicaljson" } + room_opts => { room_version => "6" } ), ], do => sub { @@ -31,7 +31,7 @@ # Floats (including NaN, Infinity, and -Infinity) should be rejected. test "Invalid JSON floats", requires => [ local_user_and_room_fixtures( - room_opts => { room_version => "org.matrix.strict_canonicaljson" } + room_opts => { room_version => "6" } ), ], do => sub { From d10ad89ed2c6c6156514a5d4ce31c2b34d626959 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 14 May 2020 15:33:35 -0400 Subject: [PATCH 05/34] Add version 6. --- lib/SyTest/Federation/Client.pm | 3 +-- tests/32room-versions.pl | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/SyTest/Federation/Client.pm b/lib/SyTest/Federation/Client.pm index 12be421ff..67bfa8689 100644 --- a/lib/SyTest/Federation/Client.pm +++ b/lib/SyTest/Federation/Client.pm @@ -16,8 +16,7 @@ use SyTest::Assertions qw( :all ); use URI::Escape qw( uri_escape ); use constant SUPPORTED_ROOM_VERSIONS => [qw( - 1 2 3 4 5 - org.matrix.msc2260 + 1 2 3 4 5 6 )]; sub configure diff --git a/tests/32room-versions.pl b/tests/32room-versions.pl index 5ea451f5e..a30964c65 100644 --- a/tests/32room-versions.pl +++ b/tests/32room-versions.pl @@ -1,7 +1,9 @@ use URI::Escape qw( uri_escape ); +use SyTest::Federation::Client; + # We test that some basic functionality works across all room versions -foreach my $version ( qw ( 1 2 3 4 5 ) ) { +foreach my $version ( qw ( 1 2 3 4 5 6 ) ) { multi_test "User can create and send/receive messages in a room with version $version", requires => [ local_user_fixture() ], From dbc682a166ddc6b4141b777ba1e9271f0d5d8a51 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 15 May 2020 14:39:56 -0400 Subject: [PATCH 06/34] Add a test for asserting the notifications power levels of room ver 6. --- tests/30rooms/08levels.pl | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/30rooms/08levels.pl b/tests/30rooms/08levels.pl index cc18ff15a..3ce77acba 100644 --- a/tests/30rooms/08levels.pl +++ b/tests/30rooms/08levels.pl @@ -7,6 +7,8 @@ sub lockeddown_room_fixture { + my ( %options ) = @_; + fixture( requires => [ $creator_fixture, $user_fixture, qw( can_change_power_levels ) ], @@ -14,7 +16,7 @@ sub lockeddown_room_fixture setup => sub { my ( $creator, $test_user ) = @_; - matrix_create_and_join_room( [ $creator, $test_user ] ) + matrix_create_and_join_room( [ $creator, $test_user ], %options ) ->then( sub { my ( $room_id ) = @_; @@ -157,3 +159,24 @@ sub test_powerlevel })->SyTest::pass_on_done( "Fails at setting 75" ); }; } + +multi_test "Users cannot set notifications powerlevel higher than their own", + requires => [ $creator_fixture, $user_fixture, lockeddown_room_fixture( room_version => "6" ), + qw( can_change_power_levels )], + + do => sub { + my ( $user, undef, $room_id ) = @_; + + matrix_change_room_power_levels( $user, $room_id, sub { + my ( $levels ) = @_; + + $levels->{notifications}{room} = 25; + })->SyTest::pass_on_done( "Succeeds at setting 25" ) + ->then( sub { + matrix_change_room_power_levels( $user, $room_id, sub { + my ( $levels ) = @_; + + $levels->{notifications}{room} = 10000000; + })->main::expect_http_403 + })->SyTest::pass_on_done( "Fails at setting 75" ); + }; From e75245cbc8c09bbadbf08e2b787d7243493db8b0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 18 May 2020 12:39:37 -0400 Subject: [PATCH 07/34] POST not PUT --- tests/80torture/20json.pl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 2352f855d..3acdcd417 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -18,7 +18,7 @@ )->followed_by( \&main::expect_http_400 ), do_request_json_for( $user, - method => "PUT", + method => "POST", uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", @@ -48,7 +48,7 @@ )->followed_by( \&main::expect_http_400 ), do_request_json_for( $user, - method => "PUT", + method => "POST", uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", @@ -57,7 +57,7 @@ )->followed_by( \&main::expect_http_400 ), do_request_json_for( $user, - method => "PUT", + method => "POST", uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", @@ -66,7 +66,7 @@ )->followed_by( \&main::expect_http_400 ), do_request_json_for( $user, - method => "PUT", + method => "POST", uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", From 972e60bac09c1fc4088d4c14d778f1e987aceaa7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 18 May 2020 12:47:12 -0400 Subject: [PATCH 08/34] Properly send infinity / NaN from Perl. --- tests/80torture/20json.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 3acdcd417..6d06a4663 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -52,7 +52,7 @@ uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", - body => "NaN", + body => "NaN" + 0, }, )->followed_by( \&main::expect_http_400 ), @@ -61,7 +61,7 @@ uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", - body => "inf", + body => "inf" + 0, }, )->followed_by( \&main::expect_http_400 ), @@ -70,7 +70,7 @@ uri => "/r0/rooms/$room_id/send/sytest.dummy", content => { msgtype => "sytest.dummy", - body => "-inf", + body => "-inf" + 0, }, )->followed_by( \&main::expect_http_400 ), ); From 727e23f11e37caad690126cbe4fbf3d886feb3e0 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 18 May 2020 12:49:10 -0400 Subject: [PATCH 09/34] Fix typo. --- tests/30rooms/08levels.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/30rooms/08levels.pl b/tests/30rooms/08levels.pl index 3ce77acba..9a14769ff 100644 --- a/tests/30rooms/08levels.pl +++ b/tests/30rooms/08levels.pl @@ -156,7 +156,7 @@ sub test_powerlevel $levels->{$levelname} = 10000000; })->main::expect_http_403 - })->SyTest::pass_on_done( "Fails at setting 75" ); + })->SyTest::pass_on_done( "Fails at setting 10000000" ); }; } @@ -178,5 +178,5 @@ sub test_powerlevel $levels->{notifications}{room} = 10000000; })->main::expect_http_403 - })->SyTest::pass_on_done( "Fails at setting 75" ); + })->SyTest::pass_on_done( "Fails at setting 10000000" ); }; From 82ae5cd2e3cd3123eada55e13c12f91170402ddb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 May 2020 15:07:33 -0400 Subject: [PATCH 10/34] Clarify that invalid values aren't even JSON. --- tests/80torture/20json.pl | 66 +++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 10 deletions(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 6d06a4663..c0d5992cc 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -28,7 +28,7 @@ ); }; -# Floats (including NaN, Infinity, and -Infinity) should be rejected. +# Floats should be rejected. test "Invalid JSON floats", requires => [ local_user_and_room_fixtures( room_opts => { room_version => "6" } @@ -37,16 +37,31 @@ do => sub { my ( $user, $room_id ) = @_; - Future->needs_all( - do_request_json_for( $user, - method => "POST", - uri => "/r0/rooms/$room_id/send/sytest.dummy", - content => { - msgtype => "sytest.dummy", - body => 1.1, - }, - )->followed_by( \&main::expect_http_400 ), + do_request_json_for( $user, + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + content => { + msgtype => "sytest.dummy", + body => 1.1, + }, + )->followed_by( \&main::expect_http_400 ); + }; +# Special values (like inf/nan) should be rejected. Note that these values are +# not technically valid JSON, but extensions that some programming languages +# support automatically. +test "Invalid JSON special values", + requires => [ local_user_and_room_fixtures( + room_opts => { room_version => "6" } + ), ], + + do => sub { + my ( $user, $room_id ) = @_; + + my $http = $user->http; + + Future->needs_all( + # Try some Perl magic values. do_request_json_for( $user, method => "POST", uri => "/r0/rooms/$room_id/send/sytest.dummy", @@ -73,5 +88,36 @@ body => "-inf" + 0, }, )->followed_by( \&main::expect_http_400 ), + + # Try some Python magic values. + $user->http->do_request( + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + params => { + access_token => $user->access_token, + }, + content => '{"msgtype": "sytest.dummy", "body": Infinity}', + content_type => "application/json", + )->followed_by( \&main::expect_http_400 ), + + $user->http->do_request( + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + params => { + access_token => $user->access_token, + }, + content => '{"msgtype": "sytest.dummy", "body": -Infinity}', + content_type => "application/json", + )->followed_by( \&main::expect_http_400 ), + + $user->http->do_request( + method => "POST", + uri => "/r0/rooms/$room_id/send/sytest.dummy", + params => { + access_token => $user->access_token, + }, + content => '{"msgtype": "sytest.dummy", "body": NaN}', + content_type => "application/json", + )->followed_by( \&main::expect_http_400 ), ); }; From 459d9e03b41954c5ec98443bba07d5b1e408d351 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 19 May 2020 15:09:05 -0400 Subject: [PATCH 11/34] Remove erronously added import. --- tests/32room-versions.pl | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/32room-versions.pl b/tests/32room-versions.pl index a30964c65..0aa66a914 100644 --- a/tests/32room-versions.pl +++ b/tests/32room-versions.pl @@ -1,7 +1,5 @@ use URI::Escape qw( uri_escape ); -use SyTest::Federation::Client; - # We test that some basic functionality works across all room versions foreach my $version ( qw ( 1 2 3 4 5 6 ) ) { multi_test "User can create and send/receive messages in a room with version $version", From 4e65103d261c1d85c6fe6473ddb7a7c0cb9a7df1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 21 May 2020 12:24:19 -0400 Subject: [PATCH 12/34] Send join tests. --- tests/50federation/30room-join.pl | 82 +++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index a6dab2d28..a2521c678 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1061,3 +1061,85 @@ sub assert_is_valid_pdu { }), ) }; + +test "Room version 6 rejects invalid JSON", + requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, + local_user_and_room_fixtures( room_opts => { room_version => "6" } ), + federation_user_id_fixture() ], + + do => sub { + my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id ) = @_; + my $first_home_server = $creator->server_name; + + my $local_server_name = $outbound_client->server_name; + my $datastore = $inbound_server->datastore; + + $outbound_client->do_request_json( + method => "GET", + hostname => $first_home_server, + uri => "/v1/make_join/$room_id/$user_id", + params => { + ver => ["6"], + }, + )->then( sub { + my ( $body ) = @_; + + 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( my $auth_events = $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'"; + + my %event = ( + ( map { $_ => $protoevent->{$_} } qw( + auth_events content depth prev_events room_id sender + state_key type ) ), + + origin => $local_server_name, + 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; + + $datastore->sign_event( \%event ); + + $outbound_client->do_request_json( + method => "PUT", + hostname => $first_home_server, + uri => "/v2/send_join/$room_id/xxx", + + content => \%event, + ) + })->main::expect_http_400 + ->then( sub { + my ( $response ) = @_; + my $body = decode_json( $response->content ); + log_if_fail "Join error response", $body; + + assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); + Future->done(1); + }); + }; From d8eaaf8a4de1b9c9701b4d8115989a2a7ac45acc Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 May 2020 10:38:46 -0400 Subject: [PATCH 13/34] Add a test for send invite. --- tests/50federation/30room-join.pl | 7 +++---- tests/50federation/35room-invite.pl | 32 ++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index a2521c678..dc7c26d47 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1062,7 +1062,7 @@ sub assert_is_valid_pdu { ) }; -test "Room version 6 rejects invalid JSON", +test "Inbound: room version 6 rejects invalid JSON", requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, local_user_and_room_fixtures( room_opts => { room_version => "6" } ), federation_user_id_fixture() ], @@ -1094,7 +1094,7 @@ sub assert_is_valid_pdu { auth_events content depth room_id sender state_key type )); - assert_json_nonempty_list( my $auth_events = $protoevent->{auth_events} ); + assert_json_nonempty_list( $protoevent->{auth_events} ); assert_json_nonempty_list( $protoevent->{prev_events} ); @@ -1130,14 +1130,13 @@ sub assert_is_valid_pdu { method => "PUT", hostname => $first_home_server, uri => "/v2/send_join/$room_id/xxx", - content => \%event, ) })->main::expect_http_400 ->then( sub { my ( $response ) = @_; my $body = decode_json( $response->content ); - log_if_fail "Join error response", $body; + log_if_fail "Send join error response", $body; assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); Future->done(1); diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index 175df965c..0491173ec 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -408,7 +408,6 @@ sub do_v2_invite_request my ( $outbound_client, $user, $sytest_user_id ) = @_; my $server_name = $user->server_name; - my $sytest_server_name = $outbound_client->server_name; my $datastore = $outbound_client->datastore; my $room = $datastore->create_room( @@ -755,3 +754,34 @@ sub do_v2_invite_request # this test is a bit slooow timeout => 20; + +test "Inbound federation rejects invites which are not signed by the sender", + requires => [ + $main::OUTBOUND_CLIENT, local_user_fixture(), federation_user_id_fixture(), + ], + + do => sub { + my ( $outbound_client, $user, $sytest_user_id ) = @_; + + my $server_name = $user->server_name; + my $datastore = $outbound_client->datastore; + + my $room = $datastore->create_room( + creator => $sytest_user_id, + room_version => "6", + ); + + my $invite = $room->create_event( + type => "m.room.member", + content => { + membership => "invite", + bad_val => 1.1, + }, + sender => $sytest_user_id, + state_key => $user->user_id, + ); + + # Note that only v2 supports providing different room versions. + do_v2_invite_request( $room, $server_name, $outbound_client, $invite ) + ->main::expect_http_400(); + }; From ee7c4572cc32610008d8630b0744bbfbc2878960 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 May 2020 14:28:04 -0400 Subject: [PATCH 14/34] Reject a v6 room invite successfully. --- tests/50federation/35room-invite.pl | 98 ++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index 0491173ec..204f004f9 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -755,7 +755,7 @@ sub do_v2_invite_request # this test is a bit slooow timeout => 20; -test "Inbound federation rejects invites which are not signed by the sender", +test "Inbound federation rejects invites which include invalid JSON for room version 6", requires => [ $main::OUTBOUND_CLIENT, local_user_fixture(), federation_user_id_fixture(), ], @@ -785,3 +785,99 @@ sub do_v2_invite_request do_v2_invite_request( $room, $server_name, $outbound_client, $invite ) ->main::expect_http_400(); }; + +test "Inbound federation rejects invite rejections which include invalid JSON for room version 6", + requires => [ + local_user_and_room_fixtures( room_opts => { room_version => "6" } ), + $main::INBOUND_SERVER, + $main::OUTBOUND_CLIENT, + federation_user_id_fixture(), + ], + + do => sub { + my ( $user, $room_id, $inbound_server, $outbound_client, $invitee_id ) = @_; + + Future->needs_all( + matrix_invite_user_to_room( $user, $invitee_id, $room_id ), + + $inbound_server->await_request_v2_invite( $room_id )->then( sub { + my ( $req, undef ) = @_; + + my $body = $req->body_from_json; + log_if_fail "Invitation", $body; + + my $invite = $body->{event}; + + # accept the invite event and send it back + $inbound_server->datastore->sign_event( $invite ); + + $req->respond_json( + { event => $invite } + ); + + Future->done; + }), + )->then( sub { + # now let's reject the event: start by asking the server to build us a + # leave event + $outbound_client->do_request_json( + method => "GET", + hostname => $user->server_name, + uri => "/v1/make_leave/$room_id/$invitee_id", + ); + })->then( sub { + my ( $resp ) = @_; + log_if_fail "/make_leave response", $resp; + + my $protoevent = $resp->{event}; + assert_json_keys( $protoevent, qw( + origin room_id sender type content state_key depth prev_events auth_events + )); + + assert_eq( $protoevent->{type}, "m.room.member", 'event type' ); + assert_eq( $protoevent->{room_id}, $room_id, 'event room_id' ); + assert_eq( $protoevent->{sender}, $invitee_id, 'event sender' ); + assert_eq( $protoevent->{content}{membership}, "leave", 'event content membership' ); + assert_eq( $protoevent->{state_key}, $invitee_id, 'event state_key' ); + + my ( $event, $event_id ) = $inbound_server->datastore->create_event( + map { $_ => $protoevent->{$_} } qw( + auth_events content depth prev_events room_id sender + state_key type + ), + ); + + my %event = ( + ( map { $_ => $protoevent->{$_} } qw( + auth_events content depth prev_events room_id sender + state_key type ) ), + + origin => $outbound_client->server_name, + origin_server_ts => $inbound_server->time_ms, + ); + + $inbound_server->datastore->sign_event( \%event ); + + $outbound_client->do_request_json( + method => "PUT", + hostname => $user->server_name, + uri => "/v2/send_leave/$room_id/xxx", + content => \%event, + ) + })->then( sub { + my ( $resp ) = @_; + log_if_fail "/send_leave response", $resp; + + assert_json_object( $resp ); + + # now wait for the leave event to come down /sync to $user + await_sync_timeline_contains( + $user, $room_id, check => sub { + my ( $event ) = @_; + return unless $event->{type} eq "m.room.member"; + return unless $event->{content}{membership} eq "leave"; + return 1; + } + ); + }); + }; From 3df4a7383c817404b3b4ded4da1db9a9f1da5f33 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 26 May 2020 16:24:34 -0400 Subject: [PATCH 15/34] Test send_leave with bad JSON. --- tests/50federation/35room-invite.pl | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index 204f004f9..afd57b406 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -820,6 +820,8 @@ sub do_v2_invite_request )->then( sub { # now let's reject the event: start by asking the server to build us a # leave event + # + # TODO Can we post a bad value here? $outbound_client->do_request_json( method => "GET", hostname => $user->server_name, @@ -848,13 +850,15 @@ sub do_v2_invite_request ); my %event = ( - ( map { $_ => $protoevent->{$_} } qw( + (map {$_ => $protoevent->{$_}} qw( auth_events content depth prev_events room_id sender - state_key type ) ), + state_key type)), origin => $outbound_client->server_name, origin_server_ts => $inbound_server->time_ms, ); + # Insert a "bad" value into the send leave, in this case a float. + ${event}{contents}{bad_val} = 1.1; $inbound_server->datastore->sign_event( \%event ); @@ -864,20 +868,13 @@ sub do_v2_invite_request uri => "/v2/send_leave/$room_id/xxx", content => \%event, ) - })->then( sub { - my ( $resp ) = @_; - log_if_fail "/send_leave response", $resp; - - assert_json_object( $resp ); + })->main::expect_http_400 + ->then( sub { + my ( $response ) = @_; + my $body = decode_json( $response->content ); + log_if_fail "Send join error response", $body; - # now wait for the leave event to come down /sync to $user - await_sync_timeline_contains( - $user, $room_id, check => sub { - my ( $event ) = @_; - return unless $event->{type} eq "m.room.member"; - return unless $event->{content}{membership} eq "leave"; - return 1; - } - ); + assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); + Future->done(1); }); }; From 505424020ab735d8773290aaa6c4080b96becc2f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 27 May 2020 13:08:34 -0400 Subject: [PATCH 16/34] Add a test for bad data in the return to make_invite. --- tests/50federation/35room-invite.pl | 49 ++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index afd57b406..b2a029e54 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -786,6 +786,52 @@ sub do_v2_invite_request ->main::expect_http_400(); }; +test "Outbound federation rejects invite response which include invalid JSON for room version 6", + requires => [ + local_user_and_room_fixtures( room_opts => { room_version => "6" } ), + $main::INBOUND_SERVER, + $main::OUTBOUND_CLIENT, + federation_user_id_fixture(), + ], + + do => sub { + my ($user, $room_id, $inbound_server, $outbound_client, $invitee_id) = @_; + + Future->needs_all( + matrix_invite_user_to_room($user, $invitee_id, $room_id), + + $inbound_server->await_request_v2_invite($room_id)->then(sub { + my ($req, undef) = @_; + + my $body = $req->body_from_json; + log_if_fail "Invitation", $body; + + my $invite = $body->{event}; + # Add a bad value into the response. + $invite->{bad_val} = 1.1; + + log_if_fail "Invitation 2", $invite; + + # accept the invite event and send it back + $inbound_server->datastore->sign_event($invite); + + $req->respond_json( + { event => $invite } + ); + + Future->done; + }), + )->main::expect_http_400 + ->then( sub { + my ( $response ) = @_; + my $body = decode_json( $response->content ); + log_if_fail "Send join error response", $body; + + assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); + Future->done(1); + }); + }; + test "Inbound federation rejects invite rejections which include invalid JSON for room version 6", requires => [ local_user_and_room_fixtures( room_opts => { room_version => "6" } ), @@ -821,7 +867,8 @@ sub do_v2_invite_request # now let's reject the event: start by asking the server to build us a # leave event # - # TODO Can we post a bad value here? + # Note that it doesn't make sense to try to use a bad JSON value here + # since the endpoint doesn't accept any JSON anyway. $outbound_client->do_request_json( method => "GET", hostname => $user->server_name, From 82907bc626009742cce2b52646dce7d3b2d7d4ae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 27 May 2020 14:17:52 -0400 Subject: [PATCH 17/34] Abstract the code that checks for a bad JSON return code. --- tests/50federation/30room-join.pl | 28 +++++++++++++++++++--------- tests/50federation/35room-invite.pl | 22 +++------------------- tests/80torture/20json.pl | 18 +++++++++--------- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index dc7c26d47..5322144d2 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1062,6 +1062,24 @@ 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", requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, local_user_and_room_fixtures( room_opts => { room_version => "6" } ), @@ -1132,13 +1150,5 @@ sub assert_is_valid_pdu { uri => "/v2/send_join/$room_id/xxx", content => \%event, ) - })->main::expect_http_400 - ->then( sub { - my ( $response ) = @_; - my $body = decode_json( $response->content ); - log_if_fail "Send join error response", $body; - - assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); - Future->done(1); - }); + })->main::expect_bad_json; }; diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index b2a029e54..2e264ecdf 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -783,7 +783,7 @@ sub do_v2_invite_request # Note that only v2 supports providing different room versions. do_v2_invite_request( $room, $server_name, $outbound_client, $invite ) - ->main::expect_http_400(); + ->main::expect_bad_json; }; test "Outbound federation rejects invite response which include invalid JSON for room version 6", @@ -821,15 +821,7 @@ sub do_v2_invite_request Future->done; }), - )->main::expect_http_400 - ->then( sub { - my ( $response ) = @_; - my $body = decode_json( $response->content ); - log_if_fail "Send join error response", $body; - - assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); - Future->done(1); - }); + )->main::expect_bad_json; }; test "Inbound federation rejects invite rejections which include invalid JSON for room version 6", @@ -915,13 +907,5 @@ sub do_v2_invite_request uri => "/v2/send_leave/$room_id/xxx", content => \%event, ) - })->main::expect_http_400 - ->then( sub { - my ( $response ) = @_; - my $body = decode_json( $response->content ); - log_if_fail "Send join error response", $body; - - assert_eq( $body->{errcode}, "M_BAD_JSON", 'responsecode' ); - Future->done(1); - }); + })->main::expect_bad_json; }; diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index c0d5992cc..1c614e4cd 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -15,7 +15,7 @@ msgtype => "sytest.dummy", body => 9007199254740992, # 2 ** 53 }, - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, do_request_json_for( $user, method => "POST", @@ -24,7 +24,7 @@ msgtype => "sytest.dummy", body => -9007199254740992, # -2 ** 53 }, - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, ); }; @@ -44,7 +44,7 @@ msgtype => "sytest.dummy", body => 1.1, }, - )->followed_by( \&main::expect_http_400 ); + )->main::expect_bad_json; }; # Special values (like inf/nan) should be rejected. Note that these values are @@ -69,7 +69,7 @@ msgtype => "sytest.dummy", body => "NaN" + 0, }, - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, do_request_json_for( $user, method => "POST", @@ -78,7 +78,7 @@ msgtype => "sytest.dummy", body => "inf" + 0, }, - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, do_request_json_for( $user, method => "POST", @@ -87,7 +87,7 @@ msgtype => "sytest.dummy", body => "-inf" + 0, }, - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, # Try some Python magic values. $user->http->do_request( @@ -98,7 +98,7 @@ }, content => '{"msgtype": "sytest.dummy", "body": Infinity}', content_type => "application/json", - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, $user->http->do_request( method => "POST", @@ -108,7 +108,7 @@ }, content => '{"msgtype": "sytest.dummy", "body": -Infinity}', content_type => "application/json", - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, $user->http->do_request( method => "POST", @@ -118,6 +118,6 @@ }, content => '{"msgtype": "sytest.dummy", "body": NaN}', content_type => "application/json", - )->followed_by( \&main::expect_http_400 ), + )->main::expect_bad_json, ); }; From 5bab21cf8aa7a0460c2bdab0f342f120e32b6b8b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 27 May 2020 14:34:46 -0400 Subject: [PATCH 18/34] Add tests for /send API with bad JSON. --- tests/50federation/51transactions.pl | 49 ++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/tests/50federation/51transactions.pl b/tests/50federation/51transactions.pl index 7ba252faa..a32508efa 100644 --- a/tests/50federation/51transactions.pl +++ b/tests/50federation/51transactions.pl @@ -1,14 +1,10 @@ test "Server correctly handles transactions that break edu limits", - requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, + requires => [ $main::OUTBOUND_CLIENT, local_user_and_room_fixtures(), - federation_user_id_fixture(), room_alias_name_fixture() ], + federation_user_id_fixture() ], do => sub { - my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id, $room_alias_name ) = @_; - - my $remote_server_name = $inbound_server->server_name; - - my $room_alias = "#$room_alias_name:$remote_server_name"; + my ( $outbound_client, $creator, $room_id, $user_id ) = @_; $outbound_client->join_room( server_name => $creator->server_name, @@ -50,3 +46,42 @@ ); }); }; + +# This currently tests that the entire transaction is rejected if a single bad +# PDU is sent in. It is unclear if this is the correct behavior or not. +# +# See https://github.com/matrix-org/synapse/issues/7543 +test "Server rejects invalid JSON in a version 6 room", + requires => [ $main::OUTBOUND_CLIENT, + local_user_and_room_fixtures( room_opts => { room_version => "6" } ), + federation_user_id_fixture() ], + + do => sub { + my ( $outbound_client, $creator, $room_id, $user_id ) = @_; + + $outbound_client->join_room( + server_name => $creator->server_name, + room_id => $room_id, + user_id => $user_id, + )->then( sub { + my ( $room ) = @_; + + my $new_event = $room->create_and_insert_event( + type => "m.room.message", + + sender => $user_id, + content => { + body => "Message 1", + bad_val => 1.1, + }, + ); + + my @pdus = ( $new_event ); + + # Send the transaction to the client and expect a fail + $outbound_client->send_transaction( + pdus => \@pdus, + destination => $creator->server_name, + )->main::expect_bad_json(); + }); + }; From c4aa0ee606eb2f74d4ba5f3157c55c18eeef8e54 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 27 May 2020 14:53:37 -0400 Subject: [PATCH 19/34] Partially roll-back the JSON checks. --- tests/80torture/20json.pl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 1c614e4cd..136248b68 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -50,6 +50,9 @@ # Special values (like inf/nan) should be rejected. Note that these values are # not technically valid JSON, but extensions that some programming languages # support automatically. +# +# Note that these tests don't explictely check for M_BAD_JSON since the +# homeserver's parser might not even be able to parse the string. test "Invalid JSON special values", requires => [ local_user_and_room_fixtures( room_opts => { room_version => "6" } @@ -69,7 +72,7 @@ msgtype => "sytest.dummy", body => "NaN" + 0, }, - )->main::expect_bad_json, + )->main::expect_http_400, do_request_json_for( $user, method => "POST", @@ -78,7 +81,7 @@ msgtype => "sytest.dummy", body => "inf" + 0, }, - )->main::expect_bad_json, + )->main::expect_http_400, do_request_json_for( $user, method => "POST", @@ -87,7 +90,7 @@ msgtype => "sytest.dummy", body => "-inf" + 0, }, - )->main::expect_bad_json, + )->main::expect_http_400, # Try some Python magic values. $user->http->do_request( @@ -98,7 +101,7 @@ }, content => '{"msgtype": "sytest.dummy", "body": Infinity}', content_type => "application/json", - )->main::expect_bad_json, + )->main::expect_http_400, $user->http->do_request( method => "POST", @@ -108,7 +111,7 @@ }, content => '{"msgtype": "sytest.dummy", "body": -Infinity}', content_type => "application/json", - )->main::expect_bad_json, + )->main::expect_http_400, $user->http->do_request( method => "POST", @@ -118,6 +121,6 @@ }, content => '{"msgtype": "sytest.dummy", "body": NaN}', content_type => "application/json", - )->main::expect_bad_json, + )->main::expect_http_400, ); }; From 9194474d2e2e926e351dfe2e6c15dc1f3239268f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 28 May 2020 14:04:23 -0400 Subject: [PATCH 20/34] Ensure a bad event doesn't come do backfill. --- lib/SyTest/Federation/Datastore.pm | 5 +- tests/50federation/34room-backfill.pl | 110 ++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/lib/SyTest/Federation/Datastore.pm b/lib/SyTest/Federation/Datastore.pm index 2a3d6bb93..aedd71a71 100644 --- a/lib/SyTest/Federation/Datastore.pm +++ b/lib/SyTest/Federation/Datastore.pm @@ -268,10 +268,13 @@ sub get_backfill_events my $event = eval { $self->get_event( $id ) } or next; + my $room = $self->get_room( $event->{room_id} ) or + croak "Unknown room $event->{room_id}"; + push @events, $event; push @event_ids, grep { !$exclude{$_} } - map { $_->[0] } @{ $event->{prev_events} }; + @{ $room->event_ids_from_refs( $event->{prev_events} ) }; # Don't include this event if we encounter it again $exclude{$id} = 1; diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index 1f08c1058..1cb1afa8d 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -443,3 +443,113 @@ }); }); }; + +test "Outbound federation rejects backfill containing invalid JSON for events in room version 6", + requires => [ local_user_fixture(), $main::INBOUND_SERVER, federation_user_id_fixture() ], + + do => sub { + my ( $user, $inbound_server, $creator_id ) = @_; + + my $local_server_name = $inbound_server->server_name; + my $datastore = $inbound_server->datastore; + + my $room_alias = "#50fed-31backfill:$local_server_name"; + + my $room = $datastore->create_room( + creator => $creator_id, + alias => $room_alias, + room_version => "6", + ); + + # Create some past messages to backfill from + $room->create_and_insert_event( + type => "m.room.message", + + sender => $creator_id, + content => { + msgtype => "m.text", + body => "Message here", + bad_val => 1.1, + }, + ); + + Future->needs_all( + $inbound_server->await_request_backfill( $room->room_id )->then( sub { + my ( $req ) = @_; + + # The helpfully-named 'v' parameter gives the "versions", i.e. the + # event IDs to start the backfill walk from. This can just be used + # in the 'start_at' list for $datastore->get_backfill_events. + # This would typically be an event ID the requesting server is + # aware exists but has not yet seen, such as one listed in a + # prev_events or auth_events list. + my $v = $req->query_param( 'v' ); + + my $limit = $req->query_param( 'limit' ); + + my @events = $datastore->get_backfill_events( + start_at => [ $v ], + limit => $limit, + ); + + log_if_fail "Responding with JSON", @events; + + $req->respond_json( { + origin => $inbound_server->server_name, + origin_server_ts => $inbound_server->time_ms, + pdus => \@events, + } ); + + Future->done; + }), + + do_request_json_for( $user, + method => "POST", + uri => "/r0/join/$room_alias", + + content => {}, + )->then( sub { + my ( $body ) = @_; + + my $room_id = $body->{room_id}; + + # It make take some time for the join to propagate, so we need to + # retry on failure. + retry_until_success { + matrix_get_room_messages( $user, $room_id, + limit => 10, # Something larger than 1. + )->then( sub { + my ( $body ) = @_; + my @events = $body->{chunk}; + + log_if_fail "Body", $body; + + # Theoretically this is 1 m.room.message events + our own + # m.room.member, but since the message event will be rejected + # only the member event will come back. + assert_eq( scalar @events , 1 ); + + my $member_event = $events[0]; + + assert_json_keys( $member_event, + qw( type event_id room_id sender state_key content )); + assert_eq( $member_event->{type}, "m.room.member", + 'events[0] type' ); + + assert_eq( $member_event->{type}, "m.room.member", + 'member event type' ); + assert_eq( $member_event->{room_id}, $room_id, + 'member event room_id' ); + assert_eq( $member_event->{sender}, $user->user_id, + 'member event sender' ); + assert_eq( $member_event->{state_key}, $user->user_id, + 'member event state_key' ); + assert_eq( $member_event->{content}{membership}, "join", + 'member event content.membership' ); + }); + }; + + Future->done(1); + }), + ) + }; From d4340b63dc33feb24f04501757ca7ad791c5674a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 29 May 2020 13:47:38 -0400 Subject: [PATCH 21/34] Add a passing test for get_missing_events. --- .../50federation/33room-get-missing-events.pl | 132 ++++++++++++++++-- 1 file changed, 124 insertions(+), 8 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index 589f2b8d1..e8ac96b71 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -13,7 +13,7 @@ my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; - my $missing_event; + my $missing_event_id; $outbound_client->join_room( server_name => $first_home_server, @@ -27,7 +27,7 @@ my $latest_event = $room->get_current_state_event( "m.room.member", $user_id ); # Generate but don't send an event - $missing_event = $room->create_and_insert_event( + my $missing_event = $room->create_and_insert_event( type => "m.room.message", sender => $user_id, @@ -35,6 +35,7 @@ body => "Message 1", }, ); + $missing_event_id = $room->id_for_event( $missing_event ); # Generate another one and do send it so it will refer to the # previous in its prev_events field @@ -43,9 +44,7 @@ # This would be done by $room->create_and_insert_event anyway but lets be # sure for this test - prev_events => [ - [ $missing_event->{event_id}, $missing_event->{hashes} ], - ], + prev_events => $room->make_event_refs( $missing_event ), sender => $user_id, content => { @@ -65,13 +64,13 @@ assert_json_list( my $earliest = $body->{earliest_events} ); @$earliest == 1 or die "Expected a single 'earliest_event' ID"; - assert_eq( $earliest->[0], $latest_event->{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->{event_id}, + assert_eq( $latest->[0], $room->id_for_event( $sent_event ), 'latest_events[0]' ); my @events = $datastore->get_backfill_events( @@ -97,7 +96,7 @@ await_event_for( $creator, filter => sub { my ( $event ) = @_; return $event->{type} eq "m.room.message" && - $event->{event_id} eq $missing_event->{event_id}; + $event->{event_id} eq $missing_event_id; }); }); }; @@ -410,3 +409,120 @@ sub sytest_user_and_room_fixture { Future->done; }); }; + +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() ], + + do => sub { + my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id ) = @_; + my $first_home_server = $creator->server_name; + + my $local_server_name = $inbound_server->server_name; + my $datastore = $inbound_server->datastore; + + my $missing_event_id; + + $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 ); + + 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", + + sender => $user_id, + content => { + body => "Message 1", + }, + ); + $missing_event_id = $room->id_for_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", + + # 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", + }, + ); + + 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; + + 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_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], $room->id_for_event( $sent_event ), + 'latest_events[0]' ); + + my @events = $datastore->get_backfill_events( + start_at => $latest, + stop_before => $earliest, + limit => $body->{limit}, + ); + + log_if_fail "Backfilling", @events; + + $req->respond_json( { + events => \@events, + } ); + + Future->done(1); + }), + + $outbound_client->send_event( + event => $sent_event, + destination => $first_home_server, + ), + ); + })->then( sub { + # creator user should eventually receive the missing event + await_event_for( $creator, filter => sub { + my ( $event ) = @_; + + log_if_fail "Event", $event; + + return $event->{type} eq "m.room.message" && + $event->{event_id} eq $missing_event_id; + }); + }); + }; From 2e6e10dea067799f1f5718bf4e1960772762264a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 10:13:39 -0400 Subject: [PATCH 22/34] Modify the test to have bad JSON. --- .../50federation/33room-get-missing-events.pl | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index e8ac96b71..b88126442 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -425,8 +425,6 @@ sub sytest_user_and_room_fixture { my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; - my $missing_event_id; - $outbound_client->join_room( server_name => $first_home_server, room_id => $room_id, @@ -447,10 +445,10 @@ sub sytest_user_and_room_fixture { sender => $user_id, content => { - body => "Message 1", + body => "Message 1", + bad_val => 1.1, }, ); - $missing_event_id = $room->id_for_event( $missing_event ); log_if_fail "Missing event", $missing_event; @@ -468,6 +466,7 @@ sub sytest_user_and_room_fixture { body => "Message 2", }, ); + my $sent_event_id = $room->id_for_event( $sent_event ); log_if_fail "Sent event", $sent_event; @@ -491,7 +490,7 @@ sub sytest_user_and_room_fixture { assert_json_list( my $latest = $body->{latest_events} ); @$latest == 1 or die "Expected a single 'latest_events' ID"; - assert_eq( $latest->[0], $room->id_for_event( $sent_event ), + assert_eq( $latest->[0], $sent_event_id, 'latest_events[0]' ); my @events = $datastore->get_backfill_events( @@ -506,23 +505,30 @@ sub sytest_user_and_room_fixture { events => \@events, } ); + log_if_fail "Done here"; + Future->done(1); }), - $outbound_client->send_event( - event => $sent_event, + # Can't use send_event here because that checks none were rejected. + $outbound_client->send_transaction( destination => $first_home_server, - ), - ); - })->then( sub { - # creator user should eventually receive the missing event - await_event_for( $creator, filter => sub { - my ( $event ) = @_; + pdus => [ $sent_event ], + )->then( sub { + my ( $body ) = @_; - log_if_fail "Event", $event; + log_if_fail "Send response", $body; - return $event->{type} eq "m.room.message" && - $event->{event_id} eq $missing_event_id; - }); + 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 ); + + Future->done; + }), + ); }); }; From 11f68ec120045d11be7d25ef0590e884fa88997c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 10:20:35 -0400 Subject: [PATCH 23/34] Remove unused variables. --- tests/50federation/33room-get-missing-events.pl | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index b88126442..e0ce05cd1 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -10,7 +10,6 @@ my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id ) = @_; my $first_home_server = $creator->server_name; - my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; my $missing_event_id; @@ -422,7 +421,6 @@ sub sytest_user_and_room_fixture { my ( $outbound_client, $inbound_server, $creator, $room_id, $user_id ) = @_; my $first_home_server = $creator->server_name; - my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; $outbound_client->join_room( From 6663e5dc669152ae62ac835a5718ab5ebd5809ae Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 14:46:02 -0400 Subject: [PATCH 24/34] Move helper for checking for a bad JSON value. --- tests/00expect_http_fail.pl | 10 ++++++++++ tests/50federation/30room-join.pl | 19 +------------------ tests/50federation/35room-invite.pl | 6 +++--- tests/50federation/51transactions.pl | 2 +- tests/80torture/20json.pl | 6 +++--- 5 files changed, 18 insertions(+), 25 deletions(-) diff --git a/tests/00expect_http_fail.pl b/tests/00expect_http_fail.pl index 2e7ccdf7c..fc3cda3e9 100644 --- a/tests/00expect_http_fail.pl +++ b/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 ); diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index 5322144d2..54299d0f9 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1062,23 +1062,6 @@ 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", requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, @@ -1150,5 +1133,5 @@ sub expect_bad_json { uri => "/v2/send_join/$room_id/xxx", content => \%event, ) - })->main::expect_bad_json; + })->main::expect_m_bad_json; }; diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index 2e264ecdf..7a5f01577 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -783,7 +783,7 @@ sub do_v2_invite_request # Note that only v2 supports providing different room versions. do_v2_invite_request( $room, $server_name, $outbound_client, $invite ) - ->main::expect_bad_json; + ->main::expect_m_bad_json; }; test "Outbound federation rejects invite response which include invalid JSON for room version 6", @@ -821,7 +821,7 @@ sub do_v2_invite_request Future->done; }), - )->main::expect_bad_json; + )->main::expect_m_bad_json; }; test "Inbound federation rejects invite rejections which include invalid JSON for room version 6", @@ -907,5 +907,5 @@ sub do_v2_invite_request uri => "/v2/send_leave/$room_id/xxx", content => \%event, ) - })->main::expect_bad_json; + })->main::expect_m_bad_json; }; diff --git a/tests/50federation/51transactions.pl b/tests/50federation/51transactions.pl index a32508efa..b62c00d98 100644 --- a/tests/50federation/51transactions.pl +++ b/tests/50federation/51transactions.pl @@ -82,6 +82,6 @@ $outbound_client->send_transaction( pdus => \@pdus, destination => $creator->server_name, - )->main::expect_bad_json(); + )->main::expect_m_bad_json(); }); }; diff --git a/tests/80torture/20json.pl b/tests/80torture/20json.pl index 136248b68..8e08f7ca7 100644 --- a/tests/80torture/20json.pl +++ b/tests/80torture/20json.pl @@ -15,7 +15,7 @@ msgtype => "sytest.dummy", body => 9007199254740992, # 2 ** 53 }, - )->main::expect_bad_json, + )->main::expect_m_bad_json, do_request_json_for( $user, method => "POST", @@ -24,7 +24,7 @@ msgtype => "sytest.dummy", body => -9007199254740992, # -2 ** 53 }, - )->main::expect_bad_json, + )->main::expect_m_bad_json, ); }; @@ -44,7 +44,7 @@ msgtype => "sytest.dummy", body => 1.1, }, - )->main::expect_bad_json; + )->main::expect_m_bad_json; }; # Special values (like inf/nan) should be rejected. Note that these values are From fa8ce9c0a6f207d8a07ad3713438743294a624a7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 14:51:19 -0400 Subject: [PATCH 25/34] Remove with_events option. --- tests/50federation/33room-get-missing-events.pl | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index c052400a6..46948fa76 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/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() ], @@ -415,7 +414,6 @@ sub sytest_user_and_room_fixture { 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() ], From 0dd8fa394294f114bc82624d31e9caf7180bd19d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 15:03:32 -0400 Subject: [PATCH 26/34] Clarify the get_missing_events test. --- tests/50federation/33room-get-missing-events.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index 46948fa76..282ea35eb 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -445,6 +445,7 @@ sub sytest_user_and_room_fixture { sender => $user_id, content => { body => "Message 1", + # Insert a bad value here so that this event cannot be fetched. bad_val => 1.1, }, ); @@ -504,8 +505,6 @@ sub sytest_user_and_room_fixture { events => \@events, } ); - log_if_fail "Done here"; - Future->done(1); }), @@ -523,8 +522,9 @@ sub sytest_user_and_room_fixture { my $pdus = $body->{pdus}; # Sending the event fails since fetching the event results in - # invalid JSON. + # 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; }), From ee89a40efaf1367b5ac1f6432e17519e13467847 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 15:04:59 -0400 Subject: [PATCH 27/34] Unique room alias. --- tests/50federation/34room-backfill.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index a3a5955ee..a8895ceea 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -11,7 +11,7 @@ my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; - my $room_alias = "#50fed-31backfill:$local_server_name"; + my $room_alias = "#50fed-34backfill:$local_server_name"; my $room = $datastore->create_room( creator => $creator_id, @@ -457,7 +457,7 @@ my $local_server_name = $inbound_server->server_name; my $datastore = $inbound_server->datastore; - my $room_alias = "#50fed-31backfill:$local_server_name"; + my $room_alias = "#50fed-34backfill-bad-json:$local_server_name"; my $room = $datastore->create_room( creator => $creator_id, From a17a67583d08fa37f6e6307b2249bac532a4e923 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 15:11:50 -0400 Subject: [PATCH 28/34] Consistently use Future->done. --- tests/50federation/33room-get-missing-events.pl | 2 +- tests/50federation/34room-backfill.pl | 2 +- tests/50federation/51transactions.pl | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index 282ea35eb..31164d800 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -505,7 +505,7 @@ sub sytest_user_and_room_fixture { events => \@events, } ); - Future->done(1); + Future->done; }), # Can't use send_event here because that checks none were rejected. diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index a8895ceea..883061e00 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -553,7 +553,7 @@ }); }; - Future->done(1); + Future->done; }), ) }; diff --git a/tests/50federation/51transactions.pl b/tests/50federation/51transactions.pl index b62c00d98..eee6b0f75 100644 --- a/tests/50federation/51transactions.pl +++ b/tests/50federation/51transactions.pl @@ -82,6 +82,6 @@ $outbound_client->send_transaction( pdus => \@pdus, destination => $creator->server_name, - )->main::expect_m_bad_json(); + )->main::expect_m_bad_json; }); }; From 4e582a8c21b33df850568938f136790f6f13b240 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 1 Jun 2020 15:21:15 -0400 Subject: [PATCH 29/34] Remove duplicate checks. --- tests/50federation/30room-join.pl | 30 +++--------------------------- 1 file changed, 3 insertions(+), 27 deletions(-) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index 54299d0f9..980731027 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1063,7 +1063,7 @@ sub assert_is_valid_pdu { }; -test "Inbound: room version 6 rejects invalid 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() ], @@ -1087,32 +1087,8 @@ sub assert_is_valid_pdu { 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( From 9ab48782c2c3fb69e7ea80790d8a0c73548fff60 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Jun 2020 07:59:02 -0400 Subject: [PATCH 30/34] Add comments. --- tests/50federation/30room-join.pl | 13 ++++++-- .../50federation/33room-get-missing-events.pl | 9 ++++++ tests/50federation/34room-backfill.pl | 23 +++++++------- tests/50federation/35room-invite.pl | 31 +++++++++---------- tests/50federation/51transactions.pl | 13 +++++--- 5 files changed, 55 insertions(+), 34 deletions(-) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index 980731027..e11019a1a 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1062,7 +1062,14 @@ sub assert_is_valid_pdu { ) }; - +# 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" } ), @@ -1087,6 +1094,8 @@ sub assert_is_valid_pdu { log_if_fail "make_join body", $body; + my $protoevent = $body->{event}; + # It is assumed that the make_join response is sane, other tests ensure # this behavior. @@ -1099,7 +1108,7 @@ sub assert_is_valid_pdu { 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 ); diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index 31164d800..2f4bbc088 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -411,6 +411,15 @@ 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: +# * Join a room. +# * 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( diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index 883061e00..cfde52f2e 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -448,6 +448,15 @@ }); }; +# A homeserver receiving a response from `backfill` 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: +# * Create a room. +# * Add "bad" data into the room history. +# * Join the room. +# * Attempt to backfill the room history. +# * Ensure that the "bad" event wil be discarded. test "Outbound federation rejects backfill containing invalid JSON for events in room version 6", requires => [ local_user_fixture(), $main::INBOUND_SERVER, federation_user_id_fixture() ], @@ -473,6 +482,7 @@ content => { msgtype => "m.text", body => "Message here", + # Insert a "bad" value into the event, in this case a float. bad_val => 1.1, }, ); @@ -535,21 +545,12 @@ my $member_event = $events[0]; + # Ensure the only event is the m.room.member event (not the + # m.room.message event). assert_json_keys( $member_event, qw( type event_id room_id sender state_key content )); assert_eq( $member_event->{type}, "m.room.member", 'events[0] type' ); - - assert_eq( $member_event->{type}, "m.room.member", - 'member event type' ); - assert_eq( $member_event->{room_id}, $room_id, - 'member event room_id' ); - assert_eq( $member_event->{sender}, $user->user_id, - 'member event sender' ); - assert_eq( $member_event->{state_key}, $user->user_id, - 'member event state_key' ); - assert_eq( $member_event->{content}{membership}, "join", - 'member event content.membership' ); }); }; diff --git a/tests/50federation/35room-invite.pl b/tests/50federation/35room-invite.pl index 7a5f01577..b88885dc1 100644 --- a/tests/50federation/35room-invite.pl +++ b/tests/50federation/35room-invite.pl @@ -824,6 +824,15 @@ sub do_v2_invite_request )->main::expect_m_bad_json; }; +# A homeserver should reject an invite rejection for a version 6 room if it +# contains bad JSON data. +# +# To test this we need to: +# * Send a successful invite to a room (via `invite`). +# * Send a successful `make_leave` for the room. +# * Add a "bad" value into the returned prototype event. +# * Make a request to `send_leave`. +# * Check that the response is M_BAD_JSON. test "Inbound federation rejects invite rejections which include invalid JSON for room version 6", requires => [ local_user_and_room_fixtures( room_opts => { room_version => "6" } ), @@ -856,8 +865,8 @@ sub do_v2_invite_request Future->done; }), )->then( sub { - # now let's reject the event: start by asking the server to build us a - # leave event + # Initiate a rejection of the invite: ask the server to build us a + # leave event. # # Note that it doesn't make sense to try to use a bad JSON value here # since the endpoint doesn't accept any JSON anyway. @@ -868,25 +877,13 @@ sub do_v2_invite_request ); })->then( sub { my ( $resp ) = @_; + log_if_fail "/make_leave response", $resp; my $protoevent = $resp->{event}; - assert_json_keys( $protoevent, qw( - origin room_id sender type content state_key depth prev_events auth_events - )); - assert_eq( $protoevent->{type}, "m.room.member", 'event type' ); - assert_eq( $protoevent->{room_id}, $room_id, 'event room_id' ); - assert_eq( $protoevent->{sender}, $invitee_id, 'event sender' ); - assert_eq( $protoevent->{content}{membership}, "leave", 'event content membership' ); - assert_eq( $protoevent->{state_key}, $invitee_id, 'event state_key' ); - - my ( $event, $event_id ) = $inbound_server->datastore->create_event( - map { $_ => $protoevent->{$_} } qw( - auth_events content depth prev_events room_id sender - state_key type - ), - ); + # It is assumed that the make_leave response is sane, other tests + # ensure this behavior. my %event = ( (map {$_ => $protoevent->{$_}} qw( diff --git a/tests/50federation/51transactions.pl b/tests/50federation/51transactions.pl index eee6b0f75..e3cdb0443 100644 --- a/tests/50federation/51transactions.pl +++ b/tests/50federation/51transactions.pl @@ -47,8 +47,12 @@ }); }; -# This currently tests that the entire transaction is rejected if a single bad -# PDU is sent in. It is unclear if this is the correct behavior or not. +# Room version 6 states that homeservers should strictly enforce canonical JSON +# on PDUs. Test that a transaction to `send` with a PDU that has bad data will +# be handled properly. +# +# This enforces that the entire transaction is rejected if a single bad PDU is +# sent. It is unclear if this is the correct behavior or not. # # See https://github.com/matrix-org/synapse/issues/7543 test "Server rejects invalid JSON in a version 6 room", @@ -66,17 +70,18 @@ )->then( sub { my ( $room ) = @_; - my $new_event = $room->create_and_insert_event( + my $bad_event = $room->create_and_insert_event( type => "m.room.message", sender => $user_id, content => { body => "Message 1", + # Insert a "bad" value into the PDU, in this case a float. bad_val => 1.1, }, ); - my @pdus = ( $new_event ); + my @pdus = ( $bad_event ); # Send the transaction to the client and expect a fail $outbound_client->send_transaction( From f3d5574f5ad129b24f6dc8f1992482d204e3d550 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Jun 2020 12:02:32 -0400 Subject: [PATCH 31/34] Use federated_rooms_fixture. --- .../50federation/33room-get-missing-events.pl | 170 +++++++++--------- tests/50federation/51transactions.pl | 43 ++--- 2 files changed, 98 insertions(+), 115 deletions(-) diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index 2f4bbc088..0d476c1fe 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -415,128 +415,118 @@ sub sytest_user_and_room_fixture { # room with a bad JSON value (e.g. a float) should discard the bad data. # # To test this we need to: -# * Join a room. # * 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( - 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", - # Insert a bad value here so that this event cannot be fetched. - 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]' ); + 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}, - ); + my @events = $datastore->get_backfill_events( + start_at => $latest, + stop_before => $earliest, + limit => $body->{limit}, + ); - log_if_fail "Backfilling", @events; + log_if_fail "Backfilling", @events; - $req->respond_json( { - events => \@events, - } ); + $req->respond_json( { + events => \@events, + } ); - Future->done; - }), + 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, 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 ) ); + # 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; + }), + ); }; diff --git a/tests/50federation/51transactions.pl b/tests/50federation/51transactions.pl index e3cdb0443..4723f25ea 100644 --- a/tests/50federation/51transactions.pl +++ b/tests/50federation/51transactions.pl @@ -57,36 +57,29 @@ # See https://github.com/matrix-org/synapse/issues/7543 test "Server rejects invalid JSON in a version 6 room", requires => [ $main::OUTBOUND_CLIENT, - local_user_and_room_fixtures( room_opts => { room_version => "6" } ), - federation_user_id_fixture() ], + federated_rooms_fixture( room_opts => { room_version => "6" } ) ], do => sub { - my ( $outbound_client, $creator, $room_id, $user_id ) = @_; + my ( $outbound_client, $creator, $user_id, @rooms ) = @_; - $outbound_client->join_room( - server_name => $creator->server_name, - room_id => $room_id, - user_id => $user_id, - )->then( sub { - my ( $room ) = @_; + my $room = $rooms[0]; - my $bad_event = $room->create_and_insert_event( - type => "m.room.message", + my $bad_event = $room->create_and_insert_event( + type => "m.room.message", - sender => $user_id, - content => { - body => "Message 1", - # Insert a "bad" value into the PDU, in this case a float. - bad_val => 1.1, - }, - ); + sender => $user_id, + content => { + body => "Message 1", + # Insert a "bad" value into the PDU, in this case a float. + bad_val => 1.1, + }, + ); - my @pdus = ( $bad_event ); + my @pdus = ( $bad_event ); - # Send the transaction to the client and expect a fail - $outbound_client->send_transaction( - pdus => \@pdus, - destination => $creator->server_name, - )->main::expect_m_bad_json; - }); + # Send the transaction to the client and expect a fail + $outbound_client->send_transaction( + pdus => \@pdus, + destination => $creator->server_name, + )->main::expect_m_bad_json; }; From 0aebc56c28854c5ab8ba6bb07a0f3d36838bc404 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Jun 2020 12:26:16 -0400 Subject: [PATCH 32/34] Use matrix_join_room. --- tests/50federation/34room-backfill.pl | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index cfde52f2e..0353097c1 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -517,15 +517,8 @@ Future->done; }), - do_request_json_for( $user, - method => "POST", - uri => "/r0/join/$room_alias", - - content => {}, - )->then( sub { - my ( $body ) = @_; - - my $room_id = $body->{room_id}; + matrix_join_room( $user, $room_alias )->then( sub { + my ( $room_id ) = @_; # It make take some time for the join to propagate, so we need to # retry on failure. From 1238be2615e907e6c4dfda1ef422eb14c4ac37e6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 2 Jun 2020 13:25:30 -0400 Subject: [PATCH 33/34] Switch to matrix_join_room_synced. --- tests/50federation/34room-backfill.pl | 54 +++++++++++++-------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index 0353097c1..c3bfbbdae 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -517,35 +517,31 @@ Future->done; }), - matrix_join_room( $user, $room_alias )->then( sub { - my ( $room_id ) = @_; - - # It make take some time for the join to propagate, so we need to - # retry on failure. - retry_until_success { - matrix_get_room_messages( $user, $room_id, - limit => 10, # Something larger than 1. - )->then( sub { - my ( $body ) = @_; - my @events = $body->{chunk}; - - log_if_fail "Body", $body; - - # Theoretically this is 1 m.room.message events + our own - # m.room.member, but since the message event will be rejected - # only the member event will come back. - assert_eq( scalar @events , 1 ); - - my $member_event = $events[0]; - - # Ensure the only event is the m.room.member event (not the - # m.room.message event). - assert_json_keys( $member_event, - qw( type event_id room_id sender state_key content )); - assert_eq( $member_event->{type}, "m.room.member", - 'events[0] type' ); - }); - }; + matrix_join_room_synced( $user, $room_alias )->then( sub { + my ($room_id) = @_; + + matrix_get_room_messages($user, $room_id, + limit => 10, # Something larger than 1. + ); + })->then( sub { + my ( $body ) = @_; + my @events = $body->{chunk}; + + log_if_fail "Body", $body; + + # Theoretically this is 1 m.room.message events + our own + # m.room.member, but since the message event will be rejected + # only the member event will come back. + assert_eq( scalar @events , 1 ); + + my $member_event = $body->{chunk}[0]; + + # Ensure the only event is the m.room.member event (not the + # m.room.message event). + assert_json_keys( $member_event, + qw( type event_id room_id sender state_key content )); + assert_eq( $member_event->{type}, "m.room.member", + 'events[0] type' ); Future->done; }), From a49a24023ec3db910e5a2261e91bd26180a26dfd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 3 Jun 2020 14:10:12 -0400 Subject: [PATCH 34/34] Fix typos Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- tests/50federation/30room-join.pl | 2 +- tests/50federation/34room-backfill.pl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index e11019a1a..b98461e99 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -1070,7 +1070,7 @@ sub assert_is_valid_pdu { # * 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", +test "Inbound: send_join rejects invalid JSON for room version 6", requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, local_user_and_room_fixtures( room_opts => { room_version => "6" } ), federation_user_id_fixture() ], diff --git a/tests/50federation/34room-backfill.pl b/tests/50federation/34room-backfill.pl index c3bfbbdae..fd3b24d99 100644 --- a/tests/50federation/34room-backfill.pl +++ b/tests/50federation/34room-backfill.pl @@ -456,7 +456,7 @@ # * Add "bad" data into the room history. # * Join the room. # * Attempt to backfill the room history. -# * Ensure that the "bad" event wil be discarded. +# * Ensure that the "bad" event will be discarded. test "Outbound federation rejects backfill containing invalid JSON for events in room version 6", requires => [ local_user_fixture(), $main::INBOUND_SERVER, federation_user_id_fixture() ],