-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test for PDU limit on transactions API #555
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good start :)
tests/50federation/51transactions.pl
Outdated
@@ -0,0 +1,46 @@ | |||
test "Server correctly handles transactions that break edu limits", | |||
requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, $main::HOMESERVER_INFO[0], | |||
local_user_and_room_fixtures( user_opts => { with_events => 1 } ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the user_opts thing
tests/50federation/51transactions.pl
Outdated
test "Server correctly handles transactions that break edu limits", | ||
requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, $main::HOMESERVER_INFO[0], | ||
local_user_and_room_fixtures( user_opts => { with_events => 1 } ), | ||
federation_user_id_fixture(), room_alias_name_fixture() ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't think we need these either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bump
tests/50federation/51transactions.pl
Outdated
my $local_server_name = $info->server_name; | ||
|
||
my $remote_server_name = $inbound_server->server_name; | ||
my $datastore = $inbound_server->datastore; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unused
tests/50federation/51transactions.pl
Outdated
|
||
my $room_alias = "#$room_alias_name:$remote_server_name"; | ||
|
||
my $device_id = "random_device_id"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unused
tests/50federation/51transactions.pl
Outdated
}, | ||
); | ||
|
||
# Generate a messge with 51 PDUs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/messge/transaction/
tests/50federation/51transactions.pl
Outdated
|
||
# Generate a messge with 51 PDUs | ||
my @pdus = (); | ||
for my $i ( 0 .. 50 ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can probably put my @pdus = ( $new_event ) x 50;
here
tests/50federation/51transactions.pl
Outdated
$outbound_client->join_room( | ||
server_name => $local_server_name, | ||
room_id => $room_id, | ||
user_id => $creator_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use $creator here (and below) I think
tests/50federation/51transactions.pl
Outdated
test "Server correctly handles transactions that break edu limits", | ||
requires => [ $main::OUTBOUND_CLIENT, $main::INBOUND_SERVER, $main::HOMESERVER_INFO[0], | ||
local_user_and_room_fixtures( user_opts => { with_events => 1 } ), | ||
federation_user_id_fixture(), room_alias_name_fixture() ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bump
Huh, TIL git does not like |
…server_specific_tests * 'develop' of github.com:matrix-org/sytest: (44 commits) test endpoint for updating backup versions (#559) Add test for transferring bans on a room upgrade (#563) fix tests for matrix-org/synapse#2090 (#350) Fix typo Fixup Incorporate review Remove prev_state from federation API Make the userdir synced not rely on being able to search for yourself (#567) Add basic soft fail test Check that event_id is given over state fetching over federation (#566) Fix registration rate limiting settings Fix comments Make sytest support auth rate limiting Fixup to not send 100 messages Test history visibility works for backfill Check that Server ACLs are preserved on room upgrade Better diagnostics from synapse startup (#561) Don't set PYTHONPATH when running synapse (#560) Regression test for redactions in room v3 (#558) Add test for PDU limit on transactions API (#555) ...
Check that transaction events with > 50 PDUs are rejected as a possible DOS vector.
Synapse PR: matrix-org/synapse#4513