Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix push URLs to match the spec. #986

Merged
merged 2 commits into from
Dec 4, 2020
Merged

Fix push URLs to match the spec. #986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
541178c
Fix push URLs to match the spec.
clokep Dec 2, 2020
0ece6a6
Fix additional pusher URLs.
clokep Dec 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions tests/14account/01change-password.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ sub matrix_set_password
pushkey => "a_push_key",
lang => "en",
data => {
url => "https://dummy.url/is/dummy",
url => "https://dummy.url/_matrix/push/v1/notify",
},
},
);
Expand Down Expand Up @@ -197,7 +197,7 @@ sub matrix_set_password
pushkey => "a_push_key",
lang => "en",
data => {
url => "https://dummy.url/is/dummy",
url => "https://dummy.url/_matrix/push/v1/notify",
},
},
)->then( sub {
Expand Down
55 changes: 29 additions & 26 deletions tests/61push/01message-pushed.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Per the specification HTTP pushers must point to the following location.
my $PUSH_LOCATION = "/_matrix/push/v1/notify";

sub matrix_set_pusher {
my ( $user, $location ) = @_;

Expand Down Expand Up @@ -82,15 +85,15 @@ sub matrix_set_pusher {
# message received may be due to Bob joining rather than the
# message that Bob sent.
matrix_set_pusher(
$alice, $test_server_info->client_location . "/alice_push",
$alice, $test_server_info->client_location . $PUSH_LOCATION,
)->SyTest::pass_on_done( "Alice's pusher created" )
})->then( sub {
# Bob sends a message that should be pushed to Alice, since it is
# in a "1:1" room with Alice

Future->needs_all(
# TODO(check that the HTTP poke is actually the poke we wanted)
await_http_request( "/alice_push", sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand Down Expand Up @@ -132,7 +135,7 @@ sub matrix_set_pusher {
pass "Alice was pushed"; # Alice has gone down the stairs

Future->needs_all(
await_http_request( "/alice_push", sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand Down Expand Up @@ -177,14 +180,14 @@ sub matrix_set_pusher {
my $room_id;

matrix_set_pusher(
$alice, $test_server_info->client_location . "/alice_push",
$alice, $test_server_info->client_location . $PUSH_LOCATION,
)->then( sub {
matrix_create_room( $bob, visibility => "private" );
})->then( sub {
( $room_id ) = @_;

Future->needs_all(
await_http_request( "/alice_push", sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand Down Expand Up @@ -218,7 +221,7 @@ sub matrix_set_pusher {

=head2 setup_push
setup_push( $alice, $bob, $test_server_info, $loc )
setup_push( $alice, $bob, $test_server_info )
Sets up push for $alice and creates a room with $alice and $bob. Returns a
future with the room_id of the newly created room.
Expand All @@ -227,10 +230,10 @@ =head2 setup_push

sub setup_push
{
my ( $alice, $bob, $test_server_info, $loc ) = @_;
my ( $alice, $bob, $test_server_info ) = @_;
my $room_id;

my $target = $test_server_info->client_location . $loc;
my $target = $test_server_info->client_location . $PUSH_LOCATION;
matrix_set_pusher(
$alice, $target,
)->then( sub {
Expand All @@ -253,7 +256,7 @@ sub setup_push

log_if_fail "Joined room $room_id; waiting for push to start working";

wait_for_pusher_to_work( $bob, $room_id, $loc );
wait_for_pusher_to_work( $bob, $room_id );
})->then( sub {
Future->done( $room_id );
})
Expand All @@ -262,7 +265,7 @@ sub setup_push

=head2 wait_for_pusher_to_work
wait_for_pusher_to_work( $sending_user, $room_id, $push_location )
wait_for_pusher_to_work( $sending_user, $room_id )
This is mostly a helper function for setup_push, but it might also help in some
other situations when configuring a pusher.
Expand All @@ -275,10 +278,10 @@ =head2 wait_for_pusher_to_work

sub wait_for_pusher_to_work
{
my ( $sending_user, $room_id, $push_loc ) = @_;
my ( $sending_user, $room_id ) = @_;

# a future which waits for a push to arrive
my $push_future = await_http_request( $push_loc, sub {
my $push_future = await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand All @@ -305,10 +308,10 @@ sub wait_for_pusher_to_work

sub check_received_push_with_name
{
my ( $bob, $room_id, $loc, $room_name ) = @_;
my ( $bob, $room_id, $room_name ) = @_;

Future->needs_all(
await_http_request( $loc, sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand Down Expand Up @@ -352,7 +355,7 @@ sub check_received_push_with_name

my $name = "Test Name";

setup_push( $alice, $bob, $test_server_info, "/alice_push" )
setup_push( $alice, $bob, $test_server_info )
->then( sub {
( $room_id ) = @_;

Expand All @@ -361,7 +364,7 @@ sub check_received_push_with_name
content => { name => $name },
);
})->then( sub {
check_received_push_with_name( $bob, $room_id, "/alice_push", $name )
check_received_push_with_name( $bob, $room_id, $name )
});
};

Expand All @@ -375,7 +378,7 @@ sub check_received_push_with_name
my ( $alice, $bob, $room_alias, $test_server_info ) = @_;
my $room_id;

setup_push( $alice, $bob, $test_server_info, "/alice_push" )
setup_push( $alice, $bob, $test_server_info )
->then( sub {
( $room_id ) = @_;

Expand All @@ -391,7 +394,7 @@ sub check_received_push_with_name
content => { alias => $room_alias },
);
})->then( sub {
check_received_push_with_name( $bob, $room_id, "/alice_push", $room_alias )
check_received_push_with_name( $bob, $room_id, $room_alias )
});
};

Expand All @@ -407,7 +410,7 @@ sub check_received_push_with_name

my $name = "Test Name";

setup_push( $alice, $bob, $test_server_info, "/alice_push" )
setup_push( $alice, $bob, $test_server_info )
->then( sub {
($room_id) = @_;

Expand All @@ -425,7 +428,7 @@ sub check_received_push_with_name
content => { room_id => $room_id },
)
})->then( sub {
check_received_push_with_name( $bob, $room_id, "/alice_push", $name )
check_received_push_with_name( $bob, $room_id, $name )
});
};

Expand All @@ -443,14 +446,14 @@ sub check_received_push_with_name
# are received by push. This is because its "impossible" to test for the
# absence of second push without doing a third.

setup_push( $alice, $bob, $test_server_info, "/alice_push" )
setup_push( $alice, $bob, $test_server_info )
->then( sub {
( $room_id ) = @_;

log_if_fail "room_id", $room_id;

Future->needs_all(
await_http_request( "/alice_push", sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;

log_if_fail "Got /alice_push request";
Expand Down Expand Up @@ -501,7 +504,7 @@ sub check_received_push_with_name
my $push_count = 0; # Counts the number of pushes we've seen in this loop

Future->needs_all(
await_http_request( "/alice_push", sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

Expand Down Expand Up @@ -591,13 +594,13 @@ sub check_received_push_with_name
log_if_fail "Regular event " . $room->id_for_event( $regular_event );

matrix_set_pusher(
$alice, $test_server_info->client_location . "/alice_push",
$alice, $test_server_info->client_location . $PUSH_LOCATION,
)->then( sub {
# we need a second local user in the room, so that we can test if
# alice's pusher is active.
matrix_join_room( $bob, $room->room_id );
})->then( sub {
wait_for_pusher_to_work( $bob, $room->room_id, "/alice_push" );
wait_for_pusher_to_work( $bob, $room->room_id );
})->then( sub {
Future->needs_all(
# we send the rejected event first, and then the regular event, and
Expand All @@ -613,7 +616,7 @@ sub check_received_push_with_name
);
}),

await_http_request( "/alice_push" )->then( sub {
await_http_request( $PUSH_LOCATION )->then( sub {
my ( $request ) = @_;
my $body = $request->body_from_json;
$request->respond_json( {} );
Expand Down
32 changes: 20 additions & 12 deletions tests/61push/08_rejected_pushers.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Per the specification HTTP pushers must point to the following location.
my $PUSH_LOCATION = "/_matrix/push/v1/notify";

sub create_pusher
{
my ( $user, $app_id, $push_key, $url ) = @_;
Expand All @@ -20,17 +23,22 @@ sub create_pusher

sub wait_for_push
{
my ( $path, $response ) = @_;
my ( $pushkey, $response ) = @_;

await_http_request( $path, sub {
await_http_request( $PUSH_LOCATION, sub {
my ( $request ) = @_;
my $body = $request->body_from_json;

# Respond to all requests, even if we filter them out
$request->respond_json( $response // {} );

return unless $body->{notification}{type};
return unless $body->{notification}{type} eq "m.room.message";

# Ensure this is the expected pusher.
return unless $body->{notification}{devices};
return unless $body->{notification}{devices}[0]{pushkey} eq $pushkey;

# Respond to expected request.
$request->respond_json( $response // {} );

return 1;
});
Comment on lines +28 to 43
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is attempting to check one of the fields of the request instead of differentiating on URL.

}
Expand All @@ -46,7 +54,7 @@ sub wait_for_push

my $room_id;

my $url = $test_server_info->client_location . "/alice_push";
my $url = $test_server_info->client_location . $PUSH_LOCATION;

matrix_create_room( $alice, visibility => "private" )->then( sub {
( $room_id ) = @_;
Expand All @@ -66,10 +74,10 @@ sub wait_for_push
"m.read" => $event_id
);
})->then( sub {
create_pusher( $alice, "sytest", "key_1", "$url/1" )
create_pusher( $alice, "sytest", "key_1", "$url" )
->SyTest::pass_on_done( "Alice's pusher 1 created" );
})->then( sub {
create_pusher( $alice, "sytest", "key_2", "$url/2" )
create_pusher( $alice, "sytest", "key_2", "$url" )
->SyTest::pass_on_done( "Alice's pusher 2 created" );
})->then( sub {
retry_until_success {
Expand All @@ -89,16 +97,16 @@ sub wait_for_push
# It can take a while before we start receiving push on new pushers.
retry_until_success {
Future->needs_all(
wait_for_push( "/alice_push/1" ),
wait_for_push( "/alice_push/2" ),
wait_for_push( "key_1" ),
wait_for_push( "key_2" ),
matrix_send_room_text_message( $bob, $room_id, body => "message" )
)
}->SyTest::pass_on_done( "Message 1 Pushed" );
})->then( sub {
# Now we go and reject a push
Future->needs_all(
wait_for_push( "/alice_push/1", { rejected => [ "key_1" ] } ),
wait_for_push( "/alice_push/2" ),
wait_for_push( "key_1", { rejected => [ "key_1" ] } ),
wait_for_push( "key_2" ),
matrix_send_room_text_message( $bob, $room_id, body => "message" )
)->SyTest::pass_on_done( "Message 2 Pushed" );
})->then( sub {
Expand Down