1
\$\begingroup\$

I have attempted to write a method that checks that images being uploaded are valid.

public function checkValidImages(){
            $errors             = array();
            $allowedImages      = array('image/jpg','image/jpeg','image/png','img/gif');
            $maxFileSize        = 2097152;
            $invalidSize        = 0;
            $invalidTypes       = 0;
            $invalidDimension   = 0;
            $minWidth           = 220;
            $minHeight          = 220;

            // Check atleast one image has been selected
            foreach($_FILES['event-images']['tmp_name'] as $tmp_name){ 

                if($tmp_name == ""){
                    $errors[] = 'Please select atleast one image.'; // This is okay because instance would only occur once
                }
            }

            // Check Images are valid image types
            foreach($_FILES['event-images']['type'] as $type){ 

                if(!in_array($type, $allowedImages)){
                    $invalidTypes++;
                }
            }

            // Check images are valid sizes
            foreach($_FILES['event-images']['size'] AS $size){
                if($size > $maxFileSize){
                    $invalidSize++;
                }
            }

            // Check images are atleast 220px high and 220px wide
            foreach($_FILES['event-images']['tmp_name'] AS $image){

                // Get image width and height
                $image_dimensions = getimagesize($image); // returns an array of image info [0] = width, [1] = height
                $image_width = $image_dimensions[0]; // Image width
                $image_height = $image_dimensions[1]; // Image height

                if(($image_width < $minWidth) || ($image_height < $minHeight)){
                    $invalidDimension++;
                }
            }

            // Add errors to return array
            if($invalidDimension > 0){
                $errors[] = 'One or more of your images has invalid dimensions. Images must be atleast 220px by 220px.';
            }
            if($invalidTypes > 0){
                $errors[] = 'One or more of your images has an invalid image type. Please only select jpg, png or gif images.';
            }
            if($invalidSize > 0){
                $errors[] = 'One or more of your images is an invalid size. Please only select images less than 2MB in size.';
            }

            // Return errors
            return $errors;
        }

I was wondering if this could be made cleaner or more efficient.

\$\endgroup\$
  • \$\begingroup\$ You're not doing any check for exploits nor managing duplicates \$\endgroup\$ – symcbean Nov 5 '13 at 21:46
1
\$\begingroup\$

There are 4 foreach loops for the same files, going for a little more complex but more organaized nested loop would be better, like this

foreach ($_FILES['event-images']['tmp_name'] as $tmp_name) {
    if (!empty($tmp_name)){
        $image = $_FILES["event-images"]["name"];
        $path_info = pathinfo($image);
        if ($path_info['extension'] == 'jpg' || $path_info['extension'] == 'jpeg' || $path_info['extension'] == 'png' || $path_info['extension'] == 'gif'){
            if ($_FILES["image"]["size"] <= $maxFileSize){
                $image_d = getimagesize($image);            
                if(($image_d[0] >= $minWidth) && ($image_d[1] >= $minHeight)){
                    $errors[]="valid image";
                }else{
                    $errors[]="invalid dimension";
                }
            }else{
                $errors[]="invalid file size";
            }
        }else{
            $errors[]="format not supported";
        }

    }else{
        $errors[] = 'Please select at least one image.';
    }
}
\$\endgroup\$
1
\$\begingroup\$

This builds on @Rajarshi answer, but simplifies the nesting (in my opinion)

foreach ($_FILES['event-images']['tmp_name'] as $tmp_name) {

    if (empty($tmp_name)){
        $errors[] = 'Please select at least one image.';
        continue;
    }

    $image = $_FILES["event-images"]["name"];
    $path_info = pathinfo($image);

    if (!is_valid_filetype($path_info['extension']) {
        $errors[]="format not supported";
        continue;
    }

    if ($_FILES["image"]["size"] > $maxFileSize){
        $errors[]="invalid file size";
        continue;
    }

    $image_d = getimagesize($image);            
    if(($image_d[0] < $minWidth) || ($image_d[1] < $minHeight)){
        $errors[]="invalid dimension";
        continue;
    }

    // why return an error for a valid image?
    //$errors[]="valid image";
}


function is_valid_filetype($ext) {
    $ext = strtolower($ext);
    return in_array($ext, array('jpg', 'jpeg', 'png', 'gif'));
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.