Anonymous Monk has asked for the wisdom of the Perl Monks concerning the following question:

Hello, I was wondering if somebody would be able to help me debug this file upload script: What I'd like the script to do: Allow users to upload pictures to my (their)websites. Restrict to jpg,jpeg,gif only.Preserve their filename. At the moment it uploads the file and uses counter number as unique name and jpg as default extension, even so it might be a gif file!. I am not a programmer, and fiddled with this script for the last 3 days. Any help really appreciated!
#!/usr/local/bin/perl use CGI; use CGI::Carp qw/fatalsToBrowser/; print "Content-type:text/html\n\n"; $cgi = new CGI; $dir = "/usr/home/mywebsite/public_html/welcome/graphicsincoming"; $file = $cgi->param('file'); if ($file =~ /.jpeg/gi){ &start; } elsif ($file =~ /.gif/gi){ &start; } elsif ($file =~ /.jpg/gi){ &start; } else{ dienice("Sorry, but you have either not selected a file to upload +or you are trying to upload an invalid picture file. Only the follow +ing file extensions are supported: /.jpeg/.gif/.jpg. Please press th +e 'back' button below to return to the Upload form."); } sub start{ open(COUNT,"</usr/home/mywebsite/public_html/cgi-bin/counter/count.dat +"); @numbers = <COUNT>; chomp(@numbers); close(COUNT); $filename = @numbers[-1]; $filename++; $file_name = "$filename"; $file=~m/^.*(\\|\/)(.*)/; # strip the remote path and keep the filenam +e open(LOCAL, ">$dir/$file_name") or die $!; while(<$file>) { print LOCAL $_; }

Replies are listed 'Best First'.
Re: upload.cgi my script is bugged
by DamnDirtyApe (Curate) on Aug 11, 2002 at 06:31 UTC

    There are a number of changes/recommendation's I'd make here.

    1. Always, always, always run under use strict and use warnings.
    2. Use the CGI module's function for printing the HTML header, rather than writing it yourself.
    3. Define all your files & file paths right up front, rather than having them scattered through the file. I haven't done it here, but you might even consider making them constants.
    4. Simplify your regexp, and make sure it does what you mean; anchor it to the end of the string, so it won't match my_file.jpeg.txt.
    5. Use a better system for tracking the file number. The subroutine I've used here is taken from the Perl Cookbook, and uses file locking to prevent two programs (or the same program) from writing to the file at the same time. This is an important consideration anytime you're doing file I/O from the web.
    6. The CGI.pm docs recommend using $cgi->upload for upload fields.

    So, finally, here's my code:

    #!/usr/bin/perl use strict ; use warnings ; use CGI ; use CGI::Carp qw( fatalsToBrowser ) ; use Fcntl qw( :DEFAULT :flock ) ; my $cgi = new CGI ; print $cgi->header ; my $dir = "/www/test" ; my $count = "/www/test/counter.dat" ; my $file = $cgi->param( 'file' ) ; if ( $file =~ /.(jpe?g|gif)$/i ) { accept_file( $file, $dir, next_file_number( $count ), $1 ) ; } else { print <<"END_OF_MSG" ; <h1>$file</h1> Sorry, but you have either not selected a file to upload or you are trying to upload an invalid picture file. Only the following file extensions are supported: /.jpeg/.gif/.jpg. Please press the 'back' button below to return to the Upload form. END_OF_MSG } sub accept_file { my ( $file, $path, $file_num, $ext ) = @_ ; open FH, ">$path/$file_num.$ext" or die "Can't open $path/$file_num.$ext: $!" ; binmode FH ; while ( <$file> ) { print FH $_; } close FH ; } sub next_file_number { my $counter_file = shift ; sysopen FH, $counter_file, O_RDWR|O_CREAT or die "Can't open $counter_file: $!" ; flock( FH, LOCK_EX ) or die "Can't write-lock $counter_file: $! +" ; # Now we have acquired the lock, it's safe for I/O my $num = <FH> || 0 ; seek FH, 0, 0 or die "Can't rewind $counter_file: $!" ; truncate FH, 0 or die "Can't truncate $counter_file: $!" +; print FH ++$num, "\n" or die "Can't write $counter_file: $!" ; close FH or die "Can't close $counter_file: $!" ; return $num ; }

    _______________
    DamnDirtyApe
    Those who know that they are profound strive for clarity. Those who
    would like to seem profound to the crowd strive for obscurity.
                --Friedrich Nietzsche
      Thank's for your reply, will give it a go Pip
Re: upload.cgi my script is bugged
by grep (Monsignor) on Aug 11, 2002 at 07:08 UTC
    One other comment in addition to DamnDirtyApe's excellent post - you should set the POST_MAX to a reasonable number. Otherwise someone could upload a 20GB file filling your drive, effectively creating a DOS attack.

    $CGI::POST_MAX = 1024 * 1000 # 1MB

    You mentioned you are new to programming Perl, so for dealing with CGI applications I would highly recommend using Ovid's CGI::Safe module to help catch some security issues you may miss.



    grep
    Just me, the boy and these two monks, no questions asked.
      Thanks for pointing this out, grep Rainer
Re: upload.cgi my script is bugged
by adrianh (Chancellor) on Aug 11, 2002 at 10:53 UTC

    You might also want to take a look at the MIME type of the file upload, as well as the file extension using:

    $type = $query->uploadInfo($filename)->{'Content-Type'};

    Some OSs (pre-X Mac OS for example) rarely use file extensions to signify file types - so it's worth checking if you want to support those users.

    My usual method is to believe the file extension if it's there, then back down to the Content-Type, then give up.

      Thanks for your reply Rainer
Re: upload.cgi my script is bugged
by zentara (Cardinal) on Aug 11, 2002 at 19:29 UTC
    Here this upload script might suit your purpose. I'll leave it up
    to you to add the filtering for gif, and jpgs. It should be obvious
    where it goes.
    
    #!/usr/bin/perl -wT use strict; use CGI; my $q = new CGI; #The max size of the uploaded file: $CGI::POST_MAX = 1024 * 1024 * 30; my $maxfile = $CGI::POST_MAX; #The folder with the uploaded files: my $outfolder = "uploads"; &upload; ####################################################### sub upload { my ($filen, $ext); #Print the start of the page: $| = 1; print <<eof; Content-type: text/html Uploading the file... <body><html> eof my $file = $q->upload('file'); my $filename = $file; $filename =~ s/^.*\///g; $filename =~ s/^.*\\//g; $filename =~ s/\-/_/g; $filename =~ s/^\.*//g; $filename =~ s/ /_/g; my $allpath = $file; if ($filename =~ /\./) { $filen = $`; $ext = $'; } my $maxtries=4; for (my $i=1; $i < $maxtries; $i++) { if (-e "$outfolder/$filename") { $filename = $filen . "_" . $i . '.' . $ext; } } #Create the file on the server, and print the "." on the page: open(OUT, ">$outfolder/$filename") or die "Can't open $outfolder/$file for writing. - $!"; binmode OUT; while(read($file, my $buffer, 4096)){; print OUT $buffer; } close OUT; my $filesize = -s "$outfolder/$filename"; $filesize = (int(($filesize / 1024) * 10) / 10); $filesize = "$filesize KB"; #Print on the browser: my $script = 'http://zentara.zentara.net/~zentara/up2.html'; print <<eof; <br><br> The file $filename was successfully uploaded!<br> The file has $filesize.<br> <div class="center"> <a href="$script">Go back if you want to upload one more file.</a> &nbsp; &nbsp; <a href="http://zentara.zentara.net/~zentara">Go to home page!</a> </body></html> eof #End the subroutine }
      Thanks for this script zentera, appreciate it will give it a go Pip
      Hello Zentara, Sorry to bother you again, script is working fine, but..... I don't know where to put the exclusions for gif and jpg files, and don't know how to get the message back in there to notify user if they have entered the wrong file format. Appreciate the help very much as I'm not good at this, but willing to learn and try pip