Local image cache for external images

Question

I have written a simple image cache script, the purpose is to load the image locally, so i can control the cache times of the images, and i have better load times than the host server, (battle.net serves 350 ms, i serve 65ish ms) this times 20 images is a pretty good performance step.

But being the performance freak i am. I was wondering if i can make it run faster!

Please also include the following in the review:

• Better suggestions for function/variable names.
• Better writing of code.
• Alternate formatting.
• Something that could be changed for something better / more correct.
• Suggestions about how to make this generic / reusable.
• An explanation about why you are right.
• An explanation why you down vote this post.

So we all can learn from this.

Comments are only for Code Review and not actually in the file.

<?php
// Checks if the img parameter is set. 
if(!isset($_GET['img'])){
    die;
}
// Saves it as a variable
$url = $_GET['img'];
// Compiles the local file string
$img = dirname(__FILE__) . '/media/cache/' . get_image_from_url($url);

// Check if the file exists, if not it dowloads a version of the image.
if(!file_exists($img)){
    // log_message('info', 'file does not exists');
   download_image($img, $url);
}

// Checks if the image existing is more than 24 hours old if it is gets a new image.
if(time()-filemtime($img) > 24 * 3600){
  //  log_message('info', 'file is too old');
    download_image($img, $url);
}

header('Content-Type: image/jpeg');
$handle = imagecreatefromjpeg($img);
imagejpeg($handle);

/*
 * FUNCTIONS
 */

function get_image_from_url($url){
    $url = str_replace('http://eu.battle.net/static-render/','', $url);
    $url = str_replace('/','-',$url);
    return $url;   
}

function download_image($img, $url){
    $headers = get_headers($url);
    $response_code = substr($headers[0], 9, 3);

    if($response_code == '404'){

        // Pleas also advice here to make the file more generic.
        $url = 'http://localhost/warlords' . '/media/images/anonymous.jpg';
    }

    $thisurl = file_get_contents($url);    
    file_put_contents($img, $thisurl);   
}

Answer 1

When I want to cache some file, I'm typically doing it similar to your solution.

However, you're able to do most of this with built-in functionality, making the code a lot shorter:

<?php
// Some URL for testing
$url = 'https://www.google.de/favicon.ico';

// Time to cache the files (here: 10 minutes)
define('time_to_cache', 600);

// Create a local file representation
$local = './cache/' . urlencode($url);

// Determine whether the local file is too old
if (@filemtime($local) + time_to_cache < time()) {
    // Download a fresh copy
    copy ($url, $local);

    // Store headers in case we need them (see alternative below)
    file_put_contents($local . '.hdr', join($http_response_header, "\n"));
}

// Solution 1: Redirect to the local cache file
header('Location: ' . urlencode($local));
exit();

// Alternative: Send headers and the actual file
// Note that this might cause problems, e.g. due
// to cache fields and the like.

// Read and send headers
foreach(file($local . '.hdr') as $line)
    header($line);

// Read and send the actual file
readfile($local);

This doesn't include any error handling though.

For error handling, you might just want to check the results of the copy() operation. There's no need to request headers in advance or doing anything like that, since that will essentially delay you twice.

Regarding your implementation above:

• Don't check the headers first. Just try downloading the file. This will take the same time. If you request headers first, you'll do two request, each of which might delay you by the mentioned 200-300 ms.

• I've used urlencode() to create a working local filename. This might not be perfect depending on your actual file system though.

• Don't check whether your local file exists first; you don't have to. You can just prepend filemtime() by an @, which will keep it from outputting any error messages and still return 0 in case the file doesn't exist (which will simply be interpretes as the file being ancient).

• Why do you read the JPEG from your local disk and then reencoding it as a new image to be sent to the browser? Just send the actual file contents (or redirect the browser). The following will do the same without reencoding the image:

  header('Content-type: image/jpeg');
  readfile($local);