Re: cleanzip

I think you'll want to be careful about strings that get passed to sub-shells, whether via system() calls or via back-ticks. At best, the job can just fail because of a file name that happens to contain a space or other shell meta-character (and at worst, you might be unpacking zip files whose contents include file names like foo; rm -rf /*).

You can easily change your one system call to the list style:

system( 'unzip', $ZIP, '-d', $TMP );  # instead of 'system "unzip $ZIP
+ -d $TMP"'
[download]

For the back-ticks, where you want to the command's stdout to be assigned to a variable, it might suffice to put backslash in front of any shell-magic characters:

$ZIP =~ s/([^\w.-])/\\$1/g;
for $BADFILE ( @BADFILES ) {
   if ($BADFILE=~s/:.*FOUND$//) {
      $BADFILE=substr($BADFILE,length($TMP)+1);
      $BADFILE =~ s/([^\w.-])/\\$1/g;
      $RESULT = `zip -d $ZIP $BADFILE`;
      print $RESULT;
   } ...
[download]

(not tested)

Bear in mind that in the OP code, the placement of double-quotes around "$BADFILE" in the back-tick command (running "zip -d ...") will do no good when the file name happens to contain one or more double-quote characters.

Comment on Re: cleanzip Select or Download Code

Replies are listed 'Best First'.
Re^2: cleanzip by sflitman (Hermit) on Oct 26, 2008 at 00:09 UTC
Agree fully. I've also posted execute() which uses an exec-like mechanism, bypassing shell issues for arguments. Here it is again: sub execute { # execute a command without shell but with timeout my ($cmd,@args)=@_; my $timeout=15; # seconds my ($result,$pid,$i,$time); if ($args[$#args]=~/^--?timeout=(\d+)$/i) { # or pass as last arg $timeout=$1; pop @args; } die "execute($cmd,".join(',',@args)."): null arguments" # SSF 08 +0708 eliminate null arguments, they are always erroneous if grep { length($_)==0 } @args; $i=index($cmd,' '); # args appended to command? if ($i>-1) { unshift @args,split(/\s+/,substr($cmd,$i+1)); # could be more +than one $cmd=substr($cmd,0,$i); } eval { local $SIG{ALRM} = sub { die "alarm\n" }; # NB: \n required local($/)=undef; alarm $timeout; $time=time if debug_has(EXEC); $pid=open(CMD,'-\|',$cmd,@args); # run without shell overhead if ($pid) { $result=<CMD>; close CMD; debug sprintf("execute(%s%s%s):\n%s%s [%.3f s]",$cmd,@args? +',':'',join(',',@args),substr($result,0,500),length($result)>500?'... +':'',time-$time) if debug_has(EXEC); } else { alarm 0; die "execute($cmd ".join(' ',@args)."): $!" unless $pid; } alarm 0; }; if ($@) { die $@ unless $@ eq "alarm\n"; # propagate unexpected errors } $result; } [download] SSF	[reply] [d/l]

Replies are listed 'Best First'.

Re^2: cleanzip
by sflitman (Hermit) on Oct 26, 2008 at 00:09 UTC

sub execute {       # execute a command without shell but with timeout
   my ($cmd,@args)=@_;
   my $timeout=15;  # seconds
   my ($result,$pid,$i,$time);
   if ($args[$#args]=~/^--?timeout=(\d+)$/i) {  # or pass as last arg
      $timeout=$1;
      pop @args;
   }
   die "execute($cmd,".join(',',@args)."): null arguments"    # SSF 08
+0708 eliminate null arguments, they are always erroneous
      if grep { length($_)==0 } @args;  
   $i=index($cmd,' ');   # args appended to command?
   if ($i>-1) {
      unshift @args,split(/\s+/,substr($cmd,$i+1));   # could be more 
+than one
      $cmd=substr($cmd,0,$i);
   }
   eval {
    local $SIG{ALRM} = sub { die "alarm\n" }; # NB: \n required
        local($/)=undef;
    alarm $timeout;
        $time=time if debug_has(EXEC);
        $pid=open(CMD,'-|',$cmd,@args);   # run without shell overhead
        if ($pid) {
           $result=<CMD>;
           close CMD;
           debug sprintf("execute(%s%s%s):\n%s%s [%.3f s]",$cmd,@args?
+',':'',join(',',@args),substr($result,0,500),length($result)>500?'...
+':'',time-$time) if debug_has(EXEC);
        } else {
           alarm 0;
           die "execute($cmd ".join(' ',@args)."): $!" unless $pid;
        }     
    alarm 0;
    };
    if ($@) {
    die $@ unless $@ eq "alarm\n";   # propagate unexpected errors
    }
    $result;
}
[download]

[reply]
[d/l]