diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index ca8a4d3c9..e6e4dbb8e 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -83,7 +83,7 @@ jobs: if [[ -z "$BRANCH_NAME" || $BRANCH_NAME =~ ^refs/pull/.* ]]; then continue fi - (wget -O - "https://github.com/element-hq/synapse/archive/$BRANCH_NAME.tar.gz" \ + (wget -O - "https://github.com/famedly/synapse-upstreaming/archive/tt/room-v12.tar.gz" \ | tar -xz --strip-components=1 -C /src/) \ && echo "Successfully downloaded and extracted $BRANCH_NAME.tar.gz" \ && break diff --git a/lib/Protocol/Matrix.pm b/lib/Protocol/Matrix.pm index eb0316a02..30a050edf 100644 --- a/lib/Protocol/Matrix.pm +++ b/lib/Protocol/Matrix.pm @@ -31,6 +31,9 @@ our @EXPORT_OK = qw( sign_event_json signed_event_json verify_event_json_signature + + room_version_is_11_plus + room_version_is_12_plus ); my $json_canon = JSON->new @@ -54,6 +57,57 @@ with signing and verifying signatures on federation-level events. =cut +=head2 room_version_is_11_plus + + $bool = room_version_is_11_plus( $room_version ) + +Returns true if the given room version uses the room v11 (or later) event +format and redaction rules (most notably, C has no C +field and its entire content is preserved on redaction). Non-numeric +(unstable) room versions are treated as 11+. + +=cut + +sub room_version_is_11_plus +{ + my ( $room_version ) = @_; + $room_version //= 1; + return ( $room_version !~ /\A[0-9]+\z/ || $room_version >= 11 ); +} + +=head2 room_version_is_12_plus + + $bool = room_version_is_12_plus( $room_version ) + +Returns true if the given room version uses the room v12 (or later) semantics +introduced by MSC4291 (hash-based room IDs) and MSC4289 (privileged room +creators): + +=over 4 + +=item * the room ID is a hash of the C event (sigil C) [MSC4291]; + +=item * the C event carries no C field [MSC4291]; + +=item * C is never referenced in any event's C [MSC4291]; and + +=item * room creators have an infinitely high power level [MSC4289]. + +=back + +Only numeric versions >= 12 qualify; intentionally-unknown placeholder versions +(e.g. C<"sytest-room-ver">) are treated as pre-v12, as they are used in tests +that deliberately mimic the older event format. + +=cut + +sub room_version_is_12_plus +{ + my ( $room_version ) = @_; + $room_version //= 1; + return ( $room_version =~ /\A[0-9]+\z/ && $room_version >= 12 ); +} + =head2 encode_json_for_signing $json = encode_json_for_signing( $data ) @@ -289,7 +343,7 @@ sub redact_event # - m.room.member: 'third_party_invite.signed' is also preserved # - m.room.redaction: 'redacts' is also preserved # Non-numeric (unstable) room versions are assumed to be 11+ - if( $room_version !~ /\A[0-9]+\z/ or $room_version >= 11 ) { + if( room_version_is_11_plus( $room_version ) ) { if( $type eq 'm.room.create' ) { %$new_content = %$old_content; $event->{unsigned}{age_ts} = $old_unsigned->{age_ts} if exists $old_unsigned->{age_ts}; diff --git a/lib/SyTest/Federation/Client.pm b/lib/SyTest/Federation/Client.pm index 028c5228e..23bd415a3 100644 --- a/lib/SyTest/Federation/Client.pm +++ b/lib/SyTest/Federation/Client.pm @@ -16,7 +16,7 @@ use SyTest::Assertions qw( :all ); use URI::Escape qw( uri_escape ); use constant SUPPORTED_ROOM_VERSIONS => [qw( - 1 2 3 4 5 6 7 8 9 10 11 + 1 2 3 4 5 6 7 8 9 10 11 12 )]; sub configure diff --git a/lib/SyTest/Federation/Datastore.pm b/lib/SyTest/Federation/Datastore.pm index 8cee0162f..bdcd70672 100644 --- a/lib/SyTest/Federation/Datastore.pm +++ b/lib/SyTest/Federation/Datastore.pm @@ -5,7 +5,7 @@ use warnings; use Carp; -use Protocol::Matrix qw( sign_event_json ); +use Protocol::Matrix qw( sign_event_json room_version_is_12_plus ); use List::Util 1.45 qw( uniq ); use Time::HiRes qw( time ); @@ -187,12 +187,20 @@ sub create_event my $self = shift; my %fields = @_; - defined $fields{$_} or croak "Every event needs a '$_' field" - for qw( type auth_events content depth prev_events room_id sender ); - my $room_version = delete $fields{room_version} // 1; my $event_id_suffix = delete $fields{event_id_suffix}; + # In room v12+, the m.room.create event carries no 'room_id' field: the room + # ID is instead derived from the create event's own reference hash. + my $omit_room_id = + defined $fields{type} && $fields{type} eq "m.room.create" + && room_version_is_12_plus( $room_version ); + + my @required_fields = qw( type auth_events content depth prev_events sender ); + push @required_fields, "room_id" unless $omit_room_id; + defined $fields{$_} or croak "Every event needs a '$_' field" + for @required_fields; + my $event = { %fields, origin_server_ts => JSON::number( int( time() * 1000 )), @@ -220,6 +228,34 @@ sub create_event return ( $event, $event_id ); } +=head2 _event_ref_ids + + $event_ids = $store->_event_ref_ids( $event, $refs ) + +Decodes a C or C reference list belonging to C<$event> +into a plain ARRAY ref of event IDs, by looking up the event's room (the +decoding differs between room versions). + +In room v12+, the C event has no C (and carries no +C or C), so its room cannot be looked up from the +event alone. As such events have no references to decode, an empty list is +returned without requiring a room lookup. + +=cut + +sub _event_ref_ids +{ + my $self = shift; + my ( $event, $refs ) = @_; + + return [] unless $refs && @$refs; + + my $room = $self->get_room( $event->{room_id} ) or + croak "Unknown room " . ( $event->{room_id} // "(undef)" ); + + return $room->event_ids_from_refs( $refs ); +} + =head2 get_backfill_events @events = $store->get_backfill_events( start_at => \@ids, ... ) @@ -272,13 +308,10 @@ 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{$_} } - @{ $room->event_ids_from_refs( $event->{prev_events} ) }; + @{ $self->_event_ref_ids( $event, $event->{prev_events} ) }; # Don't include this event if we encounter it again $exclude{$id} = 1; @@ -308,10 +341,7 @@ sub get_auth_chain_events while( @event_ids ) { my $event = $events_by_id{shift @event_ids}; - my $room = $self->get_room( $event->{room_id} ) or - croak "Unknown room $event->{room_id}"; - - my @auth_ids = @{ $room->event_ids_from_refs( $event->{auth_events} ) }; + my @auth_ids = @{ $self->_event_ref_ids( $event, $event->{auth_events} ) }; foreach my $id ( @auth_ids ) { next if $events_by_id{$id}; diff --git a/lib/SyTest/Federation/Room.pm b/lib/SyTest/Federation/Room.pm index 064ccdc4d..aba15ce5c 100644 --- a/lib/SyTest/Federation/Room.pm +++ b/lib/SyTest/Federation/Room.pm @@ -8,6 +8,7 @@ use Carp; use List::Util qw( max ); use List::UtilsBy qw( extract_by ); +use Protocol::Matrix qw( room_version_is_11_plus room_version_is_12_plus ); use SyTest::Federation::Protocol; =head1 NAME @@ -84,6 +85,32 @@ sub make_event_refs } } +=head2 auth_event_refs + + $refs = $room->auth_event_refs( $event1, $event2, ... ); + +As C, but intended for building an C list: in +room v12+, the C event must not be referenced in C, +so any create event in the given list is filtered out. For earlier room +versions this is equivalent to C. + +This lets callers pass the create event unconditionally (as they would for +pre-v12 rooms) without producing invalid v12 events. + +=cut + +sub auth_event_refs +{ + my $self = shift; + my @events = @_; + + if( room_version_is_12_plus( $self->room_version ) ) { + @events = grep { defined $_ && $_->{type} ne "m.room.create" } @events; + } + + return $self->make_event_refs( @events ); +} + =head2 event_ids_from_refs $event_ids = $room->event_ids_from_refs( [ $ref1, $ref2 ] ); @@ -186,7 +213,7 @@ sub create_initial_events # Default to old 'creator' field if no room version is specified, or room version is # a numeric value <11. Non-numeric (unstable) versions are treated as 11+. $create_content->{creator} = $creator - unless defined( $room_version ) && ( $room_version !~ /\A[0-9]+\z/ || $room_version >= 11 ); + unless defined( $room_version ) && room_version_is_11_plus( $room_version ); $self->create_and_insert_event( type => "m.room.create", @@ -237,9 +264,14 @@ sub create_event defined $fields{$_} or croak "Every event needs a '$_' field" for qw( type content sender ); + my $is_v12 = room_version_is_12_plus( $self->room_version ); + # pick auth events, per https://spec.matrix.org/v1.2/server-server-api/#auth-events-selection + # + # In room v12+, the m.room.create event MUST NOT be referenced in auth_events; + # the room_id (being the create event's ID) implies it instead. my @auth_events = grep { defined } ( - $self->get_current_state_event( "m.room.create" ), + ( $is_v12 ? () : $self->get_current_state_event( "m.room.create" ) ), $self->get_current_state_event( "m.room.power_levels" ), $self->get_current_state_event( "m.room.member", $fields{sender} ), ); @@ -258,6 +290,24 @@ sub create_event $fields{prev_events} //= $self->make_event_refs( @{ $self->{prev_events} } ); + # In room v12+, the m.room.create event has no room_id; instead the room ID + # is derived from the create event's reference hash (its event ID with the + # sigil '!' in place of '$'). We must therefore create the event without a + # room_id and adopt the derived room ID for the room. + if( $is_v12 && $fields{type} eq "m.room.create" ) { + my ( $event, $event_id ) = $self->{datastore}->create_event( + room_version => $self->room_version, + %fields, + ); + + ( my $room_id = $event_id ) =~ s/\A\$/!/ + or croak "Unexpected create event id '$event_id'"; + $self->{room_id} = $room_id; + + return $event unless wantarray; + return ( $event, $event_id ); + } + return $self->{datastore}->create_event( room_version => $self->room_version, room_id => $self->room_id, @@ -402,8 +452,10 @@ sub make_join_protoevent my $user_id = $args{user_id}; + # In room v12+, the m.room.create event MUST NOT be referenced in auth_events. my @auth_events = grep { defined } ( - $self->get_current_state_event( "m.room.create" ), + ( room_version_is_12_plus( $self->room_version ) + ? () : $self->get_current_state_event( "m.room.create" ) ), $self->get_current_state_event( "m.room.join_rules" ), ); my $auth_events = $self->make_event_refs( @auth_events ); diff --git a/run-tests.pl b/run-tests.pl index dc205f3aa..c0b9cb7da 100755 --- a/run-tests.pl +++ b/run-tests.pl @@ -68,7 +68,7 @@ # the room version that we use for the majority of our tests (those which do # not requires a specific room version). 'undef' means 'use the default from # the server under test'. -our $TEST_ROOM_VERSION; +our $TEST_ROOM_VERSION = '12'; # should we include tests that claim to use deprecated endpoints? our $INCLUDE_DEPRECATED_ENDPOINTS = 1; diff --git a/tests/30rooms/01state.pl b/tests/30rooms/01state.pl index cc17d1b0c..6c6478b79 100644 --- a/tests/30rooms/01state.pl +++ b/tests/30rooms/01state.pl @@ -1,4 +1,5 @@ use List::UtilsBy qw( partition_by ); +use Protocol::Matrix qw( room_version_is_12_plus ); my $user_fixture = local_user_fixture( presence => "online", @@ -141,10 +142,20 @@ assert_json_keys( my $power_level_state = $state_by_type{"m.room.power_levels"}[0], qw( content )); assert_json_keys( my $levels = $power_level_state->{content}, qw( users )); my $user_levels = $levels->{users}; - assert_ok( exists $user_levels->{ $user->user_id }, - "user level exists for room creator" ); - assert_ok( $user_levels->{ $user->user_id } > 0, - "room creator has nonzero power level" ); + + my $room_version = $state_by_type{"m.room.create"}[0]{content}{room_version} // "1"; + if( room_version_is_12_plus( $room_version ) ) { + # Room v12+ (MSC4289): room creators have an implicit, infinitely + # high power level and cannot be listed in m.room.power_levels. + assert_ok( !exists $user_levels->{ $user->user_id }, + "room creator is not listed in power_levels for room version $room_version" ); + } + else { + assert_ok( exists $user_levels->{ $user->user_id }, + "user level exists for room creator" ); + assert_ok( $user_levels->{ $user->user_id } > 0, + "room creator has nonzero power level" ); + } my $messages = $room->{messages}; assert_json_keys( $messages, qw( start end chunk )); diff --git a/tests/30rooms/60version_upgrade.pl b/tests/30rooms/60version_upgrade.pl index 74bbde890..f0cf257c9 100644 --- a/tests/30rooms/60version_upgrade.pl +++ b/tests/30rooms/60version_upgrade.pl @@ -291,6 +291,12 @@ sub upgrade_room_synced { matrix_create_room_synced( $creator, + # Room v12+ (MSC4289) gives creators an implicit, infinite power level + # and forbids listing them in m.room.power_levels, so a creator cannot + # hold an explicit >100 power level. This test exercises the pre-v12 + # behaviour of copying such power levels on upgrade, so pin the source + # room to a version that still supports it. + room_version => "11", power_level_content_override => { users => $user_power_levels }, )->then( sub() { ( $room_id ) = @_; @@ -738,7 +744,12 @@ sub upgrade_room_synced { test "/upgrade moves remote aliases to the new room", requires => [ - local_user_and_room_fixtures(), + # Pin the source room to a pre-v12 version. In room v12+ (MSC4289) the + # creator has an implicit, infinite power level and is not listed in + # m.room.power_levels; when upgrading such a room to a pre-v12 version the + # creator's power level is not carried over, leaving them unable to invite + # users to (and thus exercise alias migration on) the replacement room. + local_user_and_room_fixtures( room_opts => { room_version => "11" } ), remote_user_fixture(), remote_room_alias_fixture(), qw( can_upgrade_room_version ), diff --git a/tests/31sync/09archived.pl b/tests/31sync/09archived.pl index d1b34db6e..75d187065 100644 --- a/tests/31sync/09archived.pl +++ b/tests/31sync/09archived.pl @@ -188,10 +188,14 @@ matrix_create_filter( $user, {} )->then( sub { ( $filter_id ) = @_; - Future->needs_all( - matrix_create_room_synced( $user )->on_done( sub { ( $room_id_1 ) = @_; } ), - matrix_create_room_synced( $user )->on_done( sub { ( $room_id_2 ) = @_; } ), - ); + # Create the two rooms sequentially rather than concurrently. In room + # v12+ the room ID is the hash of the m.room.create event, so two rooms + # created by the same user with identical content in the same + # millisecond would hash to the same room ID and collide. Serialising + # the creates ensures their origin_server_ts (and thus room IDs) differ. + matrix_create_room_synced( $user )->on_done( sub { ( $room_id_1 ) = @_; } ); + })->then( sub { + matrix_create_room_synced( $user )->on_done( sub { ( $room_id_2 ) = @_; } ); })->then( sub { matrix_sync( $user, filter => $filter_id ); })->then( sub { diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl index 0b86f3380..06a9b924e 100644 --- a/tests/50federation/30room-join.pl +++ b/tests/50federation/30room-join.pl @@ -18,7 +18,7 @@ sub assert_is_valid_pdu { assert_json_keys( $event, qw( auth_events content depth hashes origin_server_ts - prev_events room_id sender signatures type + prev_events sender signatures type )); assert_json_list( $event->{auth_events} ); @@ -28,7 +28,11 @@ sub assert_is_valid_pdu { assert_json_number( $event->{origin_server_ts} ); assert_json_list( $event->{prev_events} ); - assert_json_string( $event->{room_id} ); + # In room v12+, the m.room.create event has no room_id (the room ID is + # derived from the create event itself); every other event still carries one. + if ( $event->{type} ne "m.room.create" or defined $event->{room_id} ) { + assert_json_string( $event->{room_id} ); + } assert_json_string( $event->{sender} ); assert_json_object( $event->{signatures} ); assert_json_string( $event->{type} ); diff --git a/tests/50federation/31room-send.pl b/tests/50federation/31room-send.pl index 4b4437627..79a1ab928 100644 --- a/tests/50federation/31room-send.pl +++ b/tests/50federation/31room-send.pl @@ -250,7 +250,7 @@ type => "m.room.message", sender => $sytest_user_id, content => { body => "event P" }, - auth_events => $room2->make_event_refs( @auth_events ), + auth_events => $room2->auth_event_refs( @auth_events ), ); log_if_fail "sending dodgy event $event_id_P in ".$room2->room_id, $event_P; diff --git a/tests/50federation/33room-get-missing-events.pl b/tests/50federation/33room-get-missing-events.pl index bbb097809..a1b0d4e21 100644 --- a/tests/50federation/33room-get-missing-events.pl +++ b/tests/50federation/33room-get-missing-events.pl @@ -200,7 +200,7 @@ sub sytest_user_and_room_fixture { sender => $sytest_user_1, state_key => $sytest_user_1, content => { membership => 'join', }, - auth_events => $room2->make_event_refs( + auth_events => $room2->auth_event_refs( $room2->get_current_state_event( "m.room.create" ), $room2->get_current_state_event( "m.room.power_levels" ), $room1->get_current_state_event( "m.room.member", $sytest_user_1 ), diff --git a/tests/50federation/36state.pl b/tests/50federation/36state.pl index 5f66ed695..ce0822c6f 100644 --- a/tests/50federation/36state.pl +++ b/tests/50federation/36state.pl @@ -292,7 +292,7 @@ sub get_state_ids_from_server { sender => '@fake_sender:' . $outbound_client->server_name, content => { body => "Rejected" }, - auth_events => $room->make_event_refs( + auth_events => $room->auth_event_refs( $room->get_current_state_event( "m.room.create" ), $room->get_current_state_event( "m.room.power_levels" ), ), @@ -345,7 +345,7 @@ sub get_state_ids_from_server { content => { body => "Rejected", }, - auth_events => $room->make_event_refs( + auth_events => $room->auth_event_refs( $room->get_current_state_event( "m.room.create" ), $room->get_current_state_event( "m.room.power_levels" ), ), diff --git a/tests/50federation/52soft-fail.pl b/tests/50federation/52soft-fail.pl index 45604cf49..4acb65502 100644 --- a/tests/50federation/52soft-fail.pl +++ b/tests/50federation/52soft-fail.pl @@ -99,7 +99,7 @@ $denied_event = $room->create_and_insert_event( type => "m.room.message", - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), prev_events => $room->make_event_refs( $join_event ), sender => $user_id, @@ -273,7 +273,7 @@ # send a regular message (event m1), which should be accepted $event_m1 = $room->create_and_insert_event( event_id_suffix => "m1", - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), prev_events => $room->make_event_refs( $join_event ), sender => $remote_user_id, type => "m.room.message", @@ -291,7 +291,7 @@ # send an event which will be soft-failed (sf1) $event_sf1 = $room->create_and_insert_event( event_id_suffix => "sf1", - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), prev_events => $room->make_event_refs( $event_m1 ), sender => $remote_user_id, type => "test.sf", @@ -309,7 +309,7 @@ # send a second soft-fail event $event_sf2 = $room->create_and_insert_event( event_id_suffix => "sf2", - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), prev_events => $room->make_event_refs( $event_m1 ), sender => $remote_user_id, type => "test.sf", @@ -475,7 +475,7 @@ $event_m1 = $room->create_and_insert_event( event_id_suffix => "m1", prev_events => $room->make_event_refs( $join_event ), - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), sender => $remote_user_id, type => "m.room.message", content => { body => "M1" }, @@ -493,7 +493,7 @@ $event_sf1 = $room->create_and_insert_event( event_id_suffix => "sf1", prev_events => $room->make_event_refs( $event_m1 ), - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), sender => $remote_user_id, type => "test.sf", content => { body => "SF1" }, @@ -511,7 +511,7 @@ $event_sf2 = $room->create_and_insert_event( event_id_suffix => "sf2", prev_events => $room->make_event_refs( $event_sf1 ), - auth_events => $room->make_event_refs( @remote_auth_events ), + auth_events => $room->auth_event_refs( @remote_auth_events ), sender => $remote_user_id, type => "test.sf", content => { body => "SF2" },