allmail
allmail

Reputation: 133

Perl Archive::Zip unexpected behavior

I am wondering if this is a bug?

sub addFile {
    my $self = shift;

    my ($fileName, $newName, $compressionLevel);
    if (ref($_[0]) eq 'HASH') {
        $fileName         = $_[0]->{filename};
        $newName          = $_[0]->{zipName};
        $compressionLevel = $_[0]->{compressionLevel};
    } else {
        ($fileName, $newName, $compressionLevel) = @_;
    }

    if ($^O eq 'MSWin32' && $Archive::Zip::UNICODE) {
        $fileName = Win32::GetANSIPathName($fileName);
    }

    my $newMember = Archive::Zip::Member->newFromFile($fileName, $newName);
    $newMember->desiredCompressionLevel($compressionLevel);**
    if ($self->{'storeSymbolicLink'} && -l $fileName) {
        my $newMember =
          Archive::Zip::Member->newFromString(readlink $fileName, $newName);

        # For symbolic links, External File Attribute is set to 0xA1FF0000 by Info-ZIP
        $newMember->{'externalFileAttributes'} = 0xA1FF0000;
        $self->addMember($newMember);
    } else {
        $self->addMember($newMember);
    }

    return $newMember;
}

It is not checking if $newMember is defined before calling desiredCompressionLevel()

And in case I try to add a file that does not exist, I get this

Can't call method "desiredCompressionLevel" on an undefined value at C:/Perl5281/site/lib/Archive/Zip/Archive.pm line 256.

Shouldn't this addFile() check for this condition and return undef if unsuccessful in adding the file. The help seems to indicate that behavior

addFile( $fileName [, $newName, $compressionLevel ] )
addFile( { filename => $fileName [, zipName => $newName, compressionLevel => $compressionLevel } ] )
Append a member whose data comes from an external file, returning the member or undef.

Also in this function in the same module, it is checking the same condition and returning undef if unsuccessful (as expected)

sub updateMember {
    my $self = shift;

    my ($oldMember, $fileName);
    if (ref($_[0]) eq 'HASH') {
        $oldMember = $_[0]->{memberOrZipName};
        $fileName  = $_[0]->{name};
    } else {
        ($oldMember, $fileName) = @_;
    }

    if (!defined($fileName)) {
        _error("updateMember(): missing fileName argument");
        return undef;
    }

    my @newStat = stat($fileName);
    if (!@newStat) {
        _ioError("Can't stat $fileName");
        return undef;
    }

    my $isDir = -d _;

    my $memberName;

    if (ref($oldMember)) {
        $memberName = $oldMember->fileName();
    } else {
        $oldMember = $self->memberNamed($memberName = $oldMember)
          || $self->memberNamed($memberName =
              _asZipDirName($oldMember, $isDir));
    }

    unless (defined($oldMember)
        && $oldMember->lastModTime() == $newStat[9]
        && $oldMember->isDirectory() == $isDir
        && ($isDir || ($oldMember->uncompressedSize() == $newStat[7]))) {

        # create the new member
        my $newMember =
            $isDir
          ? Archive::Zip::Member->newDirectoryNamed($fileName, $memberName)
          : Archive::Zip::Member->newFromFile($fileName, $memberName);

        unless (defined($newMember)) {
            _error("creation of member $fileName failed in updateMember()");
            return undef;
        }**

        # replace old member or append new one
        if (defined($oldMember)) {
            $self->replaceMember($oldMember, $newMember);
        } else {
            $self->addMember($newMember);
        }

        return $newMember;
    }

    return $oldMember;
}

Upvotes: 1

Views: 567

Answers (1)

Steffen Ullrich
Steffen Ullrich

Reputation: 123270

You are right and such a bug was filed 9 years ago and again 6 years ago. Unfortunately it looks like nobody cared as is the case with lots of other bug reports for this module.

Upvotes: 2

Related Questions