Skip to content

Commit

Permalink
Always set member perms when unzipping
Browse files Browse the repository at this point in the history
Re-create `t/root/dist/pair/0.1.1/pair-0.1.1.zip` with no permissions
for groups or others to any file or directory and add tests to make sure
they are readable by all and that directories are executable by all.
This causes failures!

Change `unzip` to *always* set file permissions, with different values
for directories (which must be executable) and files (which need not be
executable for serving by the API). Fixes #15.

As a result of the re-organization of `pair-0.1.1.zip`, change the
`find_docs` tests to sort results before comparing them. Apparently
different zip files can index their members in different orders. So just
avoid the problem for the future.
  • Loading branch information
theory committed Feb 8, 2024
1 parent d69e0f4 commit 3e6d1c9
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 27 deletions.
3 changes: 3 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ Revision history for Perl extension PGXN::API
nor testing releases (#2).
- Updated the SemVer regex when parsing rsync output to the official
version published in https://regex101.com/r/vkijKf/ (#16).
- Fix unzipping of distributions to ensure that all directories are
readable and executable but not writeable by all, and that files are
only readable by all (#15).

0.16.5 2016-06-22T18:03:05Z
- Fixed a test failure on systems with a non-English locale, thanks to
Expand Down
2 changes: 1 addition & 1 deletion lib/PGXN/API/Sync.pm
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ sub unzip {

foreach my $member ($zip->members) {
# Make sure the file is readable by everyone
$member->unixFileAttributes($member->unixFileAttributes | 0444);
$member->unixFileAttributes( $member->isDirectory ? 0755 : 0444 );
my $fn = catfile $dist_dir, split m{/} => $member->fileName;
say " $fn\n" if $self->verbose > 2;
if ($member->extractToFileNamed($fn) != AZ_OK) {
Expand Down
38 changes: 19 additions & 19 deletions t/indexer.t
Original file line number Diff line number Diff line change
Expand Up @@ -1147,57 +1147,57 @@ is_deeply \@called, [qw(update_user_lists _commit)],
# Test find_docs().
touch(catfile $indexer->doc_root_file_for(source => $params->{meta}), qw(sql hi.mkdn));
$params->{meta}{provides}{pair}{docfile} = 'sql/hi.mkdn';
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'sql/hi.mkdn', extension => 'pair' },
{ filename => 'doc/pair.md' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md' },
{ filename => 'sql/hi.mkdn', extension => 'pair' },
], 'find_docs() should find specified and random doc files';

$params->{meta}{no_index} = { file => ['sql/hi.mkdn'] };
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'sql/hi.mkdn', extension => 'pair' },
{ filename => 'doc/pair.md' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md' },
{ filename => 'sql/hi.mkdn', extension => 'pair' },
], 'find_docs() no_index should be ignored for specified doc file';

$params->{meta}{no_index} = { file => ['doc/pair.md'] };
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'sql/hi.mkdn', extension => 'pair' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'sql/hi.mkdn', extension => 'pair' },
], 'find_docs() should respect no_index for found docs';

$params->{meta}{no_index} = { directory => ['sql'] };
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'sql/hi.mkdn', extension => 'pair' },
{ filename => 'doc/pair.md' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md' },
{ filename => 'sql/hi.mkdn', extension => 'pair' },
], 'find_docs() should ignore no_index directory for specified doc';

$params->{meta}{no_index} = { directory => ['doc'] };
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'sql/hi.mkdn', extension => 'pair' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'sql/hi.mkdn', extension => 'pair' },
], 'find_docs() should respect no_index directory for found docs';

delete $params->{meta}{no_index};
$params->{meta}{provides}{pair}{docfile} = 'foo/bar.txt';
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'doc/pair.md' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md' },
], 'find_docs() should ignore non-existent specified file';

$params->{meta}{provides}{pair}{docfile} = 'doc/pair.md';
is_deeply [ $indexer->find_docs($params)], [
{ filename => 'doc/pair.md', extension => 'pair' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md', extension => 'pair' },
], 'find_docs() should not return dupes';

$params->{meta}{provides}{pair}{docfile} = 'doc/pair.pdf';
touch(catfile $indexer->doc_root_file_for(source => $params->{meta}), qw(doc pair.pdf));

is_deeply [ $indexer->find_docs($params)], [
{ filename => 'doc/pair.md' },
is_deeply [ sort { $a->{filename} cmp $b->{filename} } $indexer->find_docs($params)], [
{ filename => 'README.md' },
{ filename => 'doc/pair.md' },
], 'find_docs() should ignore doc files it does not know how to parse';

sub touch {
Expand Down
2 changes: 1 addition & 1 deletion t/root/dist/pair/0.1.1/META.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"date": "2010-10-29T22:46:45Z",
"release_status": "testing",
"user": "theory",
"sha1": "703c0c485166636317c3d4bc8a1a36d512761af7",
"sha1": "443cbcf678a3c2f479c4c069bcb96054d9b25a32",
"license": "postgresql",
"provides": {
"pair": {
Expand Down
Binary file modified t/root/dist/pair/0.1.1/pair-0.1.1.zip
Binary file not shown.
28 changes: 22 additions & 6 deletions t/sync.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

use strict;
use warnings;
#use Test::More tests => 58;
use Test::More 'no_plan';
use Test::More tests => 65;
# use Test::More 'no_plan';
use File::Spec::Functions qw(catfile catdir tmpdir);
use Test::MockModule;
use Test::Output;
Expand Down Expand Up @@ -282,7 +282,7 @@ is_deeply \@users, [qw(
##############################################################################
# digest_for()
my $pgz = catfile qw(dist pair 0.1.1 pair-0.1.1.zip);
is $sync->digest_for($pgz), '703c0c485166636317c3d4bc8a1a36d512761af7',
is $sync->digest_for($pgz), '443cbcf678a3c2f479c4c069bcb96054d9b25a32',
'Should get expected digest from digest_for()';

##############################################################################
Expand Down Expand Up @@ -323,19 +323,35 @@ my @files = (qw(
catfile(qw(test expected base.out)),
);

my @dirs = (
'test',
catfile(qw(test expected)),
catfile(qw(test sql)),
'doc',
'sql',
);

my $src_dir = PGXN::API->instance->source_dir;
my $base = catdir $src_dir, 'pair', 'pair-0.1.1';

file_not_exists_ok catfile($base, $_), "$_ should not exist" for @files;
file_not_exists_ok $base, "pair-0.1.1 directory should not exist";

# Unzip it.
ok my $zip = $sync->unzip($pgz, {name => 'pair'}), "Unzip $pgz";
isa_ok $zip, 'Archive::Zip';
file_exists_ok catfile($base, $_), "$_ should now exist" for @files;
my $readall = S_IRUSR | S_IRGRP | S_IROTH;
my $dirall = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
ok(
((stat $base)[2] & $dirall) == $dirall,
'Root directory should be writeable by owner and readable and executable by all'
);
ok(
((stat catfile $base, $_)[2] & $dirall) == $dirall,
"Directory $_ should be writeable by owner and readable and executable by all"
) for @dirs;
ok(
((stat catfile $base, $_)[2] & $readall) == $readall,
"$_ should be readable by all",
"File $_ should be readable by all",
) for @files;

# Now try a brokenated zip file.
Expand Down

0 comments on commit 3e6d1c9

Please sign in to comment.