Script that retrieves trending topics from Twitter The following code fetches trending topics from Twitter's API using a WOEID. It caches the response, loading from this cache the next time if its relatively fresh (generated within the last 15 minutes) to avoid returning a "Rate limit exceeded" message from Twitter.

A 'force-refresh' flag can be passed to skip the cache.

Please could someone have a look at the code and suggest ways it can be improved?

&lt;?php   
use Abraham\TwitterOAuth\TwitterOAuth;
$cache_file = '/home/domstapleton/public_html/home/profiles/default/cache/twitter-trends-' . $_GET['location-code'] . '.data';
if (file_exists($cache_file) &amp;&amp; !$_GET['force-refresh']) {
    $data = unserialize(file_get_contents($cache_file));
    if (($data['timestamp'] &gt; time() - 15 * 60)) {
        $trends = $data['trends'];
        echo $trends;
        exit;
    }
}
if (!$trends) {
    require '/home/domstapleton/lib/twitteroauth-master/autoload.php';
    $consumer_key = 'REDACTED';
    $consumer_secret = 'REDACTED';
    $access_token = 'REDACTED';
    $access_token_secret = 'REDACTED';
    $connection = new TwitterOAuth($consumer_key, $consumer_secret, $access_token, $access_token_secret);
    $trends = $connection-&gt;get('trends/place', array('id' =&gt; $_GET['location-code']));
    if (strpos($trends, 'Rate limit exceeded') !== false) {
        if (file_exists($cache_file)) {
            $data = unserialize(file_get_contents($cache_file));
            $trends = $data['trends'];
            echo $trends;
        }
    } else {
        $data = array('trends' =&gt; json_encode($trends), 'timestamp' =&gt; time());
        file_put_contents($cache_file, serialize($data));
        echo json_encode($trends);
    }
}
Structure: Cache Class

You already noted it yourself, you have some duplication. You could get rid of it by introducing some loadTweetFromCache function, but ideally, you would extract the whole cache handling code to its own class (and maybe have an additional twitter specific cache function or class).

Right now, if you want to change how the cache works, you would need to do that all over the place. And if you want to cache other things, you would again duplicate code. 

So instead, create a Cache class that can load and store cache entries.

Structure: Logic


  Wouldn't it be better if the code tried to get the trending topics directly from Twitter, resorting to the cache if it can't right off the bat?


It really depends. You would still spam twitter with requests if you handled it like that, so it's probably not the best approach.

Still, it takes a while to see when what is happening in your code, because of the nested ifs, the exit, and the check for the trends variable.

Because of this nesting, it's also easy to miss errors. For example: If your cache file doesn't exit, and the rate limit is exceeded, nothing happens. There is no feedback for this at all.

I would probably go with a structure like this:

// the used variables are either class variables or just passed as arguments
function getTwitterTrends() { // throws some kind of exception if trends could not be loaded from cache
    if (!$twitterCache-&gt;isStale()) {
        return $twitterCache-&gt;getTrends();
    }
    try {
        $trends = $twitterAPI-&gt;getTrends();
    } catch (RateLimitException $rle) {
        return $twitterCache-&gt;getTrends(); // couldn't retrieve new trends, use stale cache
    }
    $twitterCache-&gt;storeTrends($trends);
}


Now it's pretty clear when what is happening (of course, you have a lot more function calls, but the code clarity should be worth it).

Security

You are open to directory traversal via location-code. 

In your case, that isn't all that bad, but it should still be fixed. What can happen is: 


An attacker can read parts of files that have a .data extension and properly deserialize (not likely).
An attacker can check for the existence of any .data files (which may gain them some system knowledge or may aid in further attacks, but which isn't that serious).
If you have a file upload that allows the uploading of files with a .data extension an attacker can gain object injection (which may be serious, depending on the rest of your code).


Note that the .data extension limitation does not apply to old PHP versions that are vulnerable to null byte poisoning or path truncation.

Also, your echo is vulnerable to XSS.