5
0
mirror of git://git.proxmox.com/git/pve-storage.git synced 2025-02-25 17:57:31 +03:00

api: upload: correctly test for result of unlink

It's not enough to check whether $! is set. From "perldoc perlvar":

> Many system or library calls set "errno" if they fail, to
> indicate the cause of failure. They usually do not set "errno"
> to zero if they succeed and may set "errno" to a non-zero value
> on success. This means "errno", hence $!, is meaningful only
> *immediately* after a failure:

To protect against potential issues, check the return value of unlink
and only check $! if it failed.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This commit is contained in:
Fiona Ebner 2024-07-29 16:29:56 +02:00 committed by Thomas Lamprecht
parent 851cc07a2a
commit 1fd1b53018

View File

@ -461,7 +461,7 @@ __PACKAGE__->register_method ({
# best effort to match apl_download behaviour
chmod 0644, $tmpfilename;
my $err_cleanup = sub { unlink $dest; die "cleanup failed: $!\n" if $! && $! != ENOENT };
my $err_cleanup = sub { unlink $dest or $! == ENOENT or die "cleanup failed: $!\n" };
my $cmd;
if ($node ne 'localhost' && $node ne PVE::INotify::nodename()) {
@ -513,9 +513,8 @@ __PACKAGE__->register_method ({
};
if (my $err = $@) {
# unlinks only the temporary file from the http server
unlink $tmpfilename;
warn "unable to clean up temporory file '$tmpfilename' - $!\n"
if $! && $! != ENOENT;
unlink $tmpfilename or $! == ENOENT
or warn "unable to clean up temporory file '$tmpfilename' - $!\n";
die $err;
}
@ -526,8 +525,9 @@ __PACKAGE__->register_method ({
eval { run_command($cmd, errmsg => 'import failed'); };
unlink $tmpfilename; # the temporary file got only uploaded locally, no need to rm remote
warn "unable to clean up temporary file '$tmpfilename' - $!\n" if $! && $! != ENOENT;
# the temporary file got only uploaded locally, no need to rm remote
unlink $tmpfilename or $! == ENOENT
or warn "unable to clean up temporary file '$tmpfilename' - $!\n";
if (my $err = $@) {
eval { $err_cleanup->() };