Checking File Types

[andrew]

@alltypes = qw(.gif .jpg .jpeg .jpe .jfif);
if(param('im1') || param('im2') || param('im3') || param('im4') || par
+am('im5')) {
    for ($i=1; $i<=5; $i++) {
         if (param("im$i")) {
              $file = param("im$i");
              $filename = param("im$i");
              $filename =~ s/.*[\/\\]//;
              $ext = param("im$i");
            $ext =~ s/^[^.]*(\.[\w]+$)/$1/;
          foreach $line (@alltypes) {
            if(($ext !~ /$line/i)) {
              error("Your image needs to be in GIF or JPEG format.");
            }
          }
       }
    }
} else {
    error("You need to upload at least one image!");
}

When it comes to the point of checking what kind of extention it has, even if it is a gif I get a error "Your image needs to be in GIF or JPEG format." Anyone know why??

[Chmrr]

Be the perl interpreter for a second, and run though the for loop in your mind:

1. OK, so @alltypes is (".gif", ".jpg", ".jpeg", ".jpe", ".jfif")
2. For each element, print an error unless the string matches the extension.
3. So it doen't print for ".gif", but then it continues on to check if it ends in ".jpg" and falls over!

So what you're looking at is an and instead of an or. The simple fix in this case is to make use of grep:

unless (grep {$ext eq $_} @alltypes) {
  error("Your image needs to be a GIF of JPEG file!");
}

..thus, if the grep returns a false value, (the empty list) it means that none of the matches worked, so we need to print an error.

As a side note, proper indenting will make it easier for yourself and others to see what's going on at a glance.

Update: See rob_au's answer for how not to reinvent the wheel.

perl -pe '"I lo*`+$^X$\"$]!$/"=~m%(.*)%s;$_=$1;y^`+*^e v^#$&V"+@( NO CARRIER'

[rob_au]

It seems that you are making more work for yourself than you rightly need to. It is for just this type of scenario that I wrote the CGI::Upload module - This module allows you to check file upload type by both contents (using File::MMagic and MIME magic numbers ) and file extension.

For example, using the CGI::Upload module, this code could be reduced to the following (comments included for explanation) :

use CGI;
use CGI::Upload;

my $cgi = CGI->new;
my $upload = CGI::Upload->new;

#   The array @types contains a list of allowed file
#   extensions - Note that testing a file by MIME type is 
#   more secure than depending upon the file extension 
#   alone.

my @types = qw/ gif jpg jpeg jpe jfif /;

#   Loop through each of the image upload fields.  The 
#   image field names are being used here in place of the 
#   loop index - This has been written thus so that 
#   additional image upload fields, which may not 
#   necessarily be named 'im<num>' can be added.

foreach ( qw/ im1 im2 im3 im4 im5 / ) {

    #   Ensure that the CGI parameter has been defined

    if ( $cgi->param($_) ) {

        #   Check the file type of the uploaded file with the list
        #   of allowed file types in @types (grep works nicely
        #   for this) - If the image file extension is not within
        #   the array @types then return an error to the user.

        unless ( grep /\Q$upload->file_type($_)\E/ @types ) {

            error('Your image needs to be in GIF or JPEG format');

        }
    }
}

 

perl -e 'print+unpack("N",pack("B32","00000000000000000000000111001011")),"\n"'

[Revelation]

In addition to what the previous person said, some shortening of your code:

@alltypes = qw(.gif .jpg .jpeg .jpe .jfif);
for (1..5) {  # No need for a c-style loop.
    if param("im$_") {
       ($filename = $ext = $file = param("im$_")) =~ s/.*[\/\\]//;  # 
+Simultaneous copy/substitute.
       $ext =~ s/^[^.]*(\.[\w]+$)/$1/; # There are better ways to chec
+k for a file, like grepping to make sure the file *ends* in the right
+ extension, and getting rid of $ext, IMHO, unless you're using the ex
+tension somewhere...?
       unless (grep {$ext eq $_} @alltypes) {
       error("Your image needs to be a GIF of JPEG file!");
       }      
    } 
}

---

Gyan Kapur
[e-mail]

[l2kashe]

or making use of a regex like

# note not sure where the file extension is within param,
# I am assuming at end of string, anchoring the regex
# to the end will speed it up slightly
$regex = '\.(gif|jpg|jpe[g]?|jfif)(\s+|)$';

if ($filename !~ /$regex/o) {
   error("Your image needs to be in GIF or JPEG format.");
}


/*
* And the Creator, against his better judgement, wrote man.c
*/

[andrew]

Thanks for that but now this wont work

       opendir(DIR, "/var/www/virtual/merchandisemarket.net/m_images/"
+);
       my @userfiles = sort grep !/^\.\.?\z/, readdir DIR;
       close DIR;

       unless (grep {$filename eq $_} @userfiles) {
       error("Your already uploaded a file named \"$_\"!");
     }

[Chmrr]

Th phrase "won't work" or any varient there of conveys no information -- see on asking for help for more information on how to, well, ask for help. As it is, we can only guess at what "wont work."

The only guess I can come up with is that you're using unless where you want to use if. That is, if the filename they just uploaded is already in the list of user files, then we should give an error.

However, you're doing more work than you need to. It's a simple matter to ask if a file exists in a certain directory; just use the -e file test, as so:

if (-e "/var/www/virtual/merchandisemarket.net/m_images/$filename") {
  error(qq/You already uploaded a file named "$_!"/);
}

..in place of the whole block of code you wrote above. Also note the use of the qq/.../ operator, so that one doens't need to have all that ugly escaping to put quotes in interpolated text.

perl -pe '"I lo*`+$^X$\"$]!$/"=~m%(.*)%s;$_=$1;y^`+*^e v^#$&V"+@( NO CARRIER'

[Mr. Muskrat]

How do you know that the user is not going to rename whatever file they want to use to have an acceptable file extension? Why not use image::info? It can tell you the mime type or suggested extension for an image.

Untested code snippet:

use Image::Info qw(image_info);
my $filename = "file5.jpg"; # actually a PNG
my $info = image_info($filename);
if (my $error = $info->{error}) {
    die "Can't parse image info: $error\n";
}
my $mime_type = $info->{file_media_type};
my $suggested_ext = $info->{file_ext};
print "mime type: $mime_type\n";
print "suggested extension: $suggested_ext\n";

Output produced:
mime type: image/png
suggested extension: png

Then you can check against your list of good extensions.

Update: Code updated and tested!