images/directory listing script

[djr33]

Hmm..... fun.

I wonder if there's a way around it or something.

I guess just rendering the thumbnails isn't that big a deal.... it's a one time thing, after all.

After that, shouldn't be much to just load the pre-rendered thumbnails.

[ItsMeOnly]

Actually, there is: you can either use session-based "semaphores", or restrict http server to n instances per client (sane value is 5).

[blm126]

Wow, there is a lot of ugly coding in there. I'm attaching two versions of the thumbnail script. One is a commented version of yours, the other is how I would write it.

<?php
// The file
if ($_GET['image'] == "") die('Image Not Specified');
//**Don't do this. You should be using empty
$filename = urldecode($_GET['image']);
//**AHHHH!!!! This can cause horrible problems, this is done automatically
if (@getimagesize($filename)) {
//** I think you meant file_exists
    $imginfo = getimagesize($filename);
    }
if ($imginfo[2] != 1 && $imginfo[2] != 2 && $imginfo[2] != 3) {
    die('Image Filetype Incorrect');
    //**exit should be used
    }
//echo md5_file($filename)."jpg;".$filename."\n";
if (@getimagesize("_thumbs/".md5_file($filename).".jpg") !== FALSE) { 
//**Don't abuse functions, file_exists should be used.
    // Content type
    header('Content-type: image/jpeg');
    // Output
    $thumb = imagecreatefromjpeg("_thumbs/".md5_file($filename).".jpg");
    imagejpeg($thumb, null, 50);
    }
else {
    // Set a maximum height and width
    $width = 200;
    $height = 200;
    // Get new dimensions
    list($width_orig, $height_orig) = array($imginfo[0],$imginfo[1]);
    $ratio_orig = $width_orig/$height_orig;
    if ($width/$height > $ratio_orig) {
        $width = $height*$ratio_orig;
    } else {
        $height = $width/$ratio_orig;
    }
    // Make image
    if ($imginfo[2] == 1) {
        if (@imagecreatefromgif($filename)) $image = imagecreatefromgif($filename);
        //file_exists
        }
    else if ($imginfo[2] == 2) {
        if (@imagecreatefromjpeg($filename)) $image = imagecreatefromjpeg($filename);
        //file_exists
        }
    else if ($imginfo[2] == 3) {
        if (@imagecreatefrompng($filename)) $image = imagecreatefrompng($filename);
        //And file exists
        }
    if (!isset($image)) die('Image Filetype Incorrect.');
    //Else
    // Resample
    $thumb = imagecreatetruecolor($width, $height);
    imagecopyresampled($thumb, $image, 0, 0, 0, 0, $width, $height, $width_orig, $height_orig);
    // Content type
    header('Content-type: image/jpeg');
    // Output
    imagejpeg($thumb, "_thumbs/".md5_file($filename).".jpg", 50);
    }

?>

<?php
// The file
if (empty($_GET['image']) exit('Image Not Specified');
$filename = $_GET['image'];
//echo md5_file($filename)."jpg;".$filename."\n";
if (file_exits("_thumbs/".md5_file($filename).".jpg")) { 
    // Content type
    header('Content-type: image/jpeg');
    // Output
    $thumb = imagecreatefromjpeg("_thumbs/".md5_file($filename).".jpg");
    imagejpeg($thumb, null, 50);
    }
else if(file_exists($filename)) {
     $imginfo = getimagesize($filename);
    // Set a maximum height and width
    $width = 200;
    $height = 200;
    // Get new dimensions
    list($width_orig, $height_orig) = array($imginfo[0],$imginfo[1]);
    $ratio_orig = $width_orig/$height_orig;
    if ($width/$height > $ratio_orig) {
        $width = $height*$ratio_orig;
    } else {
        $height = $width/$ratio_orig;
    }
    // Make image
    if ($imginfo[2] == 1) {
        $image = imagecreatefromgif($filename);
    }
    else if ($imginfo[2] == 2) {
        $image = imagecreatefromjpeg($filename);
    }
    else if ($imginfo[2] == 3) {
        $image = imagecreatefrompng($filename);
    }
    else{
        exit('Image Filetype Not Supported.')
    }
    // Resample
    $thumb = imagecreatetruecolor($width, $height);
    imagecopyresampled($thumb, $image, 0, 0, 0, 0, $width, $height, $width_orig, $height_orig);
    // Content type
    header('Content-type: image/jpeg');
    // Output
    imagejpeg($thumb,'_thumbs/'.md5_file($filename).'.jpg', 50);
}
else{
    exit('File does not exist');
}

?>

[djr33]

Thanks for the input... however, here's my reasoning for using the code I did. Please correct me if I'm wrong.

<?php
// The file
if ($_GET['image'] == "") die('Image Not Specified');
//**Don't do this. You should be using empty
////What if the url was ".php?image="?
$filename = urldecode($_GET['image']);
//**AHHHH!!!! This can cause horrible problems, this is done automatically
////No. In the original script, I use urlencode(), so this is reversing it.
if (@getimagesize($filename)) {
//** I think you meant file_exists
////No. Anything is a file. However, this checks if an IMAGE exists.
    $imginfo = getimagesize($filename);
    }
if ($imginfo[2] != 1 && $imginfo[2] != 2 && $imginfo[2] != 3) {
    die('Image Filetype Incorrect');
    //**exit should be used
   ////Why's that? die() works fine;
   ////According to php.net: "Note:  This language construct is equivalent to die()."
    }
//echo md5_file($filename)."jpg;".$filename."\n";
if (@getimagesize("_thumbs/".md5_file($filename).".jpg") !== FALSE) { 
//**Don't abuse functions, file_exists should be used.
////See above-- checking for IMAGES, not FILES
    // Content type
    header('Content-type: image/jpeg');
    // Output
    $thumb = imagecreatefromjpeg("_thumbs/".md5_file($filename).".jpg");
    imagejpeg($thumb, null, 50);
    }
else {
    // Set a maximum height and width
    $width = 200;
    $height = 200;
    // Get new dimensions
    list($width_orig, $height_orig) = array($imginfo[0],$imginfo[1]);
    $ratio_orig = $width_orig/$height_orig;
    if ($width/$height > $ratio_orig) {
        $width = $height*$ratio_orig;
    } else {
        $height = $width/$ratio_orig;
    }
    // Make image
    if ($imginfo[2] == 1) {
        if (@imagecreatefromgif($filename)) $image = imagecreatefromgif($filename);
        //file_exists
        }
    else if ($imginfo[2] == 2) {
        if (@imagecreatefromjpeg($filename)) $image = imagecreatefromjpeg($filename);
        //file_exists
        }
    else if ($imginfo[2] == 3) {
        if (@imagecreatefrompng($filename)) $image = imagecreatefrompng($filename);
        //And file exists
        }
    if (!isset($image)) die('Image Filetype Incorrect.');
    //Else
    // Resample
    $thumb = imagecreatetruecolor($width, $height);
    imagecopyresampled($thumb, $image, 0, 0, 0, 0, $width, $height, $width_orig, $height_orig);
    // Content type
    header('Content-type: image/jpeg');
    // Output
    imagejpeg($thumb, "_thumbs/".md5_file($filename).".jpg", 50);
    }

?>

Again, thanks for the input. Please let me know, once reading why I did that.

[blm126]

<?php
// The file
if ($_GET['image'] == "") die('Image Not Specified');
//**Don't do this. You should be using empty
////What if the url was ".php?image="?
//**read the manual entry on empty. It first checks if the variable is set, then it checks to make sure that it is not empty
$filename = urldecode($_GET['image']);
//**AHHHH!!!! This can cause horrible problems, this is done automatically
////No. In the original script, I use urlencode(), so this is reversing it.
//**PHP automatically calls urldecode on get variables. It ASSUMES they have been encoded already.Calling it twice can have weird results
if (@getimagesize($filename)) {
//** I think you meant file_exists
////No. Anything is a file. However, this checks if an IMAGE exists.
//**One thing at a time, You check if it is an image later on
    $imginfo = getimagesize($filename);
    }
if ($imginfo[2] != 1 && $imginfo[2] != 2 && $imginfo[2] != 3) {
    die('Image Filetype Incorrect');
    //**exit should be used
   ////Why's that? die() works fine;
   ////According to php.net: "Note:  This language construct is equivalent to die()."
//** yes, it is equivalent(I try not to argue with the manual :) ), bu check out http://www.php.net/manual/en/aliases.php You will see that die is an alias, and also that they don't encourage the use of aliases.
    }
//echo md5_file($filename)."jpg;".$filename."\n";
if (@getimagesize("_thumbs/".md5_file($filename).".jpg") !== FALSE) { 
//**Don't abuse functions, file_exists should be used.
////See above-- checking for IMAGES, not FILES
//**See above, really checking for a file
    // Content type
    header('Content-type: image/jpeg');
    // Output
    $thumb = imagecreatefromjpeg("_thumbs/".md5_file($filename).".jpg");
    imagejpeg($thumb, null, 50);
    }
else {
    // Set a maximum height and width
    $width = 200;
    $height = 200;
    // Get new dimensions
    list($width_orig, $height_orig) = array($imginfo[0],$imginfo[1]);
    $ratio_orig = $width_orig/$height_orig;
    if ($width/$height > $ratio_orig) {
        $width = $height*$ratio_orig;
    } else {
        $height = $width/$ratio_orig;
    }
    // Make image
    if ($imginfo[2] == 1) {
        if (@imagecreatefromgif($filename)) $image = imagecreatefromgif($filename);
        //file_exists
        }
    else if ($imginfo[2] == 2) {
        if (@imagecreatefromjpeg($filename)) $image = imagecreatefromjpeg($filename);
        //file_exists
        }
    else if ($imginfo[2] == 3) {
        if (@imagecreatefrompng($filename)) $image = imagecreatefrompng($filename);
        //And file exists
        }
    if (!isset($image)) die('Image Filetype Incorrect.');
    //Else
    // Resample
    $thumb = imagecreatetruecolor($width, $height);
    imagecopyresampled($thumb, $image, 0, 0, 0, 0, $width, $height, $width_orig, $height_orig);
    // Content type
    header('Content-type: image/jpeg');
    // Output
    imagejpeg($thumb, "_thumbs/".md5_file($filename).".jpg", 50);
    }

?>

As always I intend this to be nothing but constructive critism, so please don't take offense.

[djr33]

No, no offense taken. I really didn't understand the reasoning for some of that.
Ok, empty is fine... any reason not to do it like I did?
die()-- i've seen this and been using it from example. Either way....
ok, I thought that the urldecode was important for that since I did it manually in the original script.

Now.... the one thing I really don't agree with....
"//**One thing at a time, You check if it is an image later on "
What possible reason would I not use a function that checks both if it's an image and if it exists? I don't see the logic here.
In other circumstances, that would make sense, but any non-images aren't supposed to return true either, so seems like it just saves a step, and after that point, there's no need to escape the errors with @getimagesize() later.

[blm126]

In my mind file_exists should be better for performance. Why should you force PHP to get all that information for you when you are only interested in whether the file exists?

Now, about empty. The way you did it could cause PHP to throw a warning. Look at the following

if(isset($_GET['somevar'] && $_GET['somevar'] != '')) 

if(!empty($_GET['somevar'])) 

Those two are the same. I tend to use empty as it is shorter.However, the following is not the same

if($_GET['somevar'] != '') 

If $_GET['somevar'] does not exist PHP will throw a warning. You normally don't catch mistakes like this because many servers are set up to not display warnings.

Another small performace thing I noticed in your code is your use of quotes. Supposedly, a single quote( ' ) is faster for PHP to parse than a double quote ( " )

All of these things(except for the urldecode problem) are mostly small habits that will make your code more efficient. At least, I hope they will because that is the only reason I do them.

[djr33]

"In my mind file_exists should be better for performance. Why should you force PHP to get all that information for you when you are only interested in whether the file exists?"
Because I will be using that funtion later, and if it returns false, which it likely will if there are other files around, then it will save the processor from checking BOTH if it exists as a file then later as an image.

I'll agree that using double quotes is a habit, but it's nice when you want something inside to be parsed, like a variable, \n, etc.

[blm126]

The only way to really tell which is better will be to time it.

[Twey]

Supposedly, a single quote( ' ) is faster for PHP to parse than a double quote ( " )

The key word I used is "marginally" It's a good idea to use single quotes where possible, but if someone forgets, it's not exactly a big deal. They add perhaps twenty milliseconds onto their code execution time.