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

fix tests for https://github.com/matrix-org/synapse/pull/2090 #350

Merged
merged 8 commits into from
Mar 11, 2019
Merged

fix tests for https://github.com/matrix-org/synapse/pull/2090 #350

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b5555cb
fix tests for https://github.com/matrix-org/synapse/pull/2090
ara4n Apr 1, 2017
fc53774
Merge branch 'develop' into matthew/fix-filename-escaping
richvdh Feb 27, 2019
3c28820
Merge remote-tracking branch 'origin/develop' into HEAD
richvdh Feb 27, 2019
7c78d24
revert earlier changes on this branch
richvdh Feb 27, 2019
9452b01
Factor out common code
richvdh Feb 27, 2019
5c1f1e7
Add tests for filenames with spaces and semicolons
richvdh Feb 27, 2019
f6de195
fix failing test
richvdh Feb 28, 2019
7c3710e
Improve Content-Disposition handling
richvdh Mar 7, 2019
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
104 changes: 74 additions & 30 deletions tests/51media/01unicode.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,36 +40,85 @@
);


test "Can upload with Unicode file name",
requires => [ $main::API_CLIENTS[0], local_user_fixture(),
qw( can_upload_media )],
=head2 upload_test_content

proves => [qw( can_upload_media_unicode )],
my ( $content_id, $content_uri ) = upload_test_content(
$user, filename => "filename",
) -> get;

do => sub {
my ( $http, $user ) = @_;
Uploads some test content with the given filename.

$http->do_request(
method => "POST",
full_uri => "/_matrix/media/r0/upload",
content => "Test media file",
content_type => "text/plain",

params => {
access_token => $user->access_token,
filename => $FILENAME,
}
)->then( sub {
my ( $body ) = @_;
Returns the content id of the uploaded content.

=cut
sub upload_test_content
{
my ( $user, %params ) = @_;

$user->http->do_request(
method => "POST",
full_uri => "/_matrix/media/r0/upload",
content => "Test media file",
content_type => "text/plain",

params => {
access_token => $user->access_token,
%params,
},
)->then( sub {
my ( $body ) = @_;

assert_json_keys( $body, qw( content_uri ));

my $content_uri = $body->{content_uri};

my $parsed_uri = URI->new( $body->{content_uri} );
my $server = $parsed_uri->authority;
my $path = $parsed_uri->path;

my $content_id = "$server$path";

Future->done( $content_id, $content_uri );
});
}
push our @EXPORT, qw( upload_test_content );


=head2 get_media

my ( $content_dispostion, $content ) = get_media( $http, $content_id ) -> get;

Fetches a piece of media from the server.

=cut
sub get_media
{
my ( $http, $content_id ) = @_;

$http->do_request(
method => "GET",
full_uri => "/_matrix/media/r0/download/$content_id",
)->then( sub {
my ( $body, $response ) = @_;

my $disposition = $response->header( "Content-Disposition" );
Future->done( $disposition, $body );
});
}
push @EXPORT, qw( get_media );

assert_json_keys( $body, qw( content_uri ));

my $content_uri = URI->new( $body->{content_uri} );
my $server = $content_uri->authority;
my $path = $content_uri->path;
test "Can upload with Unicode file name",
requires => [ local_user_fixture(),
qw( can_upload_media )],

$content_id = "$server$path";
proves => [qw( can_upload_media_unicode )],

do => sub {
my ( $user ) = @_;

upload_test_content( $user, filename=>$FILENAME )->then( sub {
( $content_id ) = @_;
Future->done(1)
});
};
Expand All @@ -85,13 +134,8 @@ sub test_using_client
$content = $content_id;
}

$client->do_request(
method => "GET",
full_uri => "/_matrix/media/r0/download/$content",
)->then( sub {
my ( $body, $response ) = @_;

my $disposition = $response->header( "Content-Disposition" );
get_media( $client, $content )->then( sub {
my ( $disposition ) = @_;
uc $disposition eq uc "inline; filename*=utf-8''$FILENAME_ENCODED" or
die "Expected a UTF-8 filename parameter";

Expand Down
34 changes: 6 additions & 28 deletions tests/51media/02nofilename.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,31 +1,13 @@
my $content_id;

test "Can upload without a file name",
requires => [ $main::API_CLIENTS[0], local_user_fixture() ],
requires => [ local_user_fixture() ],

do => sub {
my ( $http, $user ) = @_;

$http->do_request(
method => "POST",
full_uri => "/_matrix/media/r0/upload",
content => "Test media file",
content_type => "text/plain",

params => {
access_token => $user->access_token,
}
)->then(sub {
my ( $body ) = @_;

assert_json_keys( $body, qw( content_uri ));

my $content_uri = URI->new( $body->{content_uri} );
my $server = $content_uri->authority;
my $path = $content_uri->path;

$content_id = "$server$path";
my ( $user ) = @_;

upload_test_content( $user, )->then( sub {
( $content_id ) = @_;
Future->done(1)
});
};
Expand All @@ -37,13 +19,9 @@ sub test_using_client
{
my ( $client ) = @_;

$client->do_request(
method => "GET",
full_uri => "/_matrix/media/r0/download/$content_id",
)->then( sub {
my ( $body, $response ) = @_;
get_media( $client, $content_id )->then( sub {
my ( $disposition ) = @_;

my $disposition = $response->header( "Content-Disposition" );
defined $disposition and
die "Unexpected Content-Disposition header";

Expand Down
116 changes: 44 additions & 72 deletions tests/51media/03ascii.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,80 +2,58 @@
my $content_uri;

test "Can upload with ASCII file name",
requires => [ $main::API_CLIENTS[0], local_user_fixture() ],
requires => [ local_user_fixture() ],

do => sub {
my ( $http, $user ) = @_;

$http->do_request(
method => "POST",
full_uri => "/_matrix/media/r0/upload",
content => "Test media file",
content_type => "text/plain",

params => {
access_token => $user->access_token,
filename => "ascii",
}
)->then( sub {
my ( $body ) = @_;
my ( $user ) = @_;
upload_test_content( $user, filename=>"ascii" )->then( sub {
( $content_id, $content_uri ) = @_;
Future->done(1);
});
};

assert_json_keys( $body, qw( content_uri ));
# we only need one user for these tests
my $user_fixture = local_user_fixture();

$content_uri = $body->{content_uri};
my %TEST_CASES = (
"ascii" => "inline; filename=ascii",

my $parsed_uri = URI->new( $body->{content_uri} );
my $server = $parsed_uri->authority;
my $path = $parsed_uri->path;
# synapse uses filename* encoding for this, though regular filename encoding
# with a quoted string would be valid too
"name with spaces" => "inline; filename*=utf-8''name%20with%20spaces",
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if this was a map of file names to a list of valid content dispositions, so we don't force synapse's choice on other projects

Copy link
Member

Choose a reason for hiding this comment

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

that's not quite as easy as it sounds. have made it parse the header.


$content_id = "$server$path";
"name;with;semicolons" => "inline; filename*=utf-8''name%3Bwith%3Bsemicolons",
);

Future->done(1)
});
};
while ( my ( $filename, $expected_disposition ) = each %TEST_CASES ) {
test "Can download file '$filename'",
requires => [
$user_fixture, $main::API_CLIENTS[1]
],

# These next two tests do the same thing with two different HTTP clients, to
# test locally and via federation
check => sub {
my ( $user, $federation_client ) = @_;

sub test_using_client
{
my ( $client ) = @_;
my $content_id;

$client->do_request(
method => "GET",
full_uri => "/_matrix/media/r0/download/$content_id",
)->then( sub {
my ( $body, $response ) = @_;
# first upload the content with the given filename
upload_test_content( $user, filename=>$filename )->then( sub {
( $content_id ) = @_;

my $disposition = $response->header( "Content-Disposition" );
$disposition eq "inline; filename=ascii" or
die "Expected an ASCII filename parameter";
# try and fetch it as a local user
get_media( $user->http, $content_id );
})->then( sub {
assert_eq( $_[0], $expected_disposition );

Future->done(1);
});
# do the same over federation
get_media( $federation_client, $content_id );
})->then( sub {
assert_eq( $_[0], $expected_disposition );
Future->done(1);
});
};
}

test "Can download with ASCII file name locally",
requires => [ $main::API_CLIENTS[0] ],

check => sub {
my ( $http ) = @_;
test_using_client( $http )
->then( sub {
test_using_client( $http )
});
};

test "Can download with ASCII file name over federation",
requires => [ $main::API_CLIENTS[1] ],

check => sub {
my ( $http ) = @_;
test_using_client( $http )
->then( sub {
test_using_client( $http )
});
};

test "Can download specifying a different ASCII file name",
requires => [ $main::API_CLIENTS[0] ],
Expand All @@ -102,25 +80,19 @@ sub test_using_client

check => sub {
my ( $http, $user, $room_id ) = @_;
test_using_client( $http )
->then( sub {
matrix_send_room_message( $user, $room_id,
content => { msgtype => "m.file", body => "test.txt", url => $content_uri }
)
});
matrix_send_room_message( $user, $room_id,
content => { msgtype => "m.file", body => "test.txt", url => $content_uri }
);
};

test "Can fetch images in room",
requires => [ $main::API_CLIENTS[0], local_user_and_room_fixtures() ],

check => sub {
my ( $http, $user, $room_id ) = @_;
test_using_client( $http )
->then( sub {
matrix_send_room_message_synced( $user, $room_id,
content => { msgtype => "m.text", body => "test" }
)
})->then( sub {
matrix_send_room_message_synced( $user, $room_id,
content => { msgtype => "m.text", body => "test" }
)->then( sub {
matrix_send_room_message_synced( $user, $room_id,
content => { msgtype => "m.file", body => "test.txt", url => $content_uri }
)
Expand Down