A loop won't delete generated link written in a file as a string after expiry, only if clicked

Hello people,
as I am new to this forum my apology to the admin(s) and to all if I broke some of the rules here.

My actual problem is as the title itself says, below you will find a part of the code where foreach() loop is made through the strings written in a file called keys, where after the link is generated, it does not delete itself automatically after expiry, but only if clicked. I have tried with $one = “” and $one = ‘’, but it doesn’t work either.

Please help,
Xedeus

$fid = base64_decode(trim($_GET['fid']));
$key = trim($_GET['key']);

// Calculate link expiration time
$currentTime = time();
$keyTime = explode('-',$key);
$expTime = strtotime(EXPIRATION_TIME, $keyTime[0]);

// Retrieve the keys from the tokens file
$keys = file(TOKEN_DIR.'/keys');
$match = false;

// Loop through the keys to find a match
// When the match is found, remove it
foreach($keys as &$one){
    if(rtrim($one)==$key){
        $match = true;
        $one = "";
    }
}

// Put the remaining keys back into the tokens file
file_put_contents(TOKEN_DIR.'/keys',$keys);

// If match found and the link is not expired
if($match !== false && $currentTime <= $expTime){
    // If the file is found in the file's array
    if(!empty($files[$fid])){
        // Get the file data
        $contentType = $files[$fid]['content_type'];
        $fileName = $files[$fid]['suggested_name'];
        $filePath = $files[$fid]['file_path'];
        
        // Force the browser to download the file
        if($files[$fid]['type'] == 'remote_file'){
            $file = fopen($filePath, 'r');
            header("Content-Type:text/plain");
            header("Content-Disposition: attachment; filename=\"{$fileName}\"");
            fpassthru($file);
        }else{
            header("Content-Description: File Transfer");
            header("Content-type: {$contentType}");
            header("Content-Disposition: attachment; filename=\"{$fileName}\"");
            header("Content-Length: " . filesize($filePath));
            header('Pragma: public');
            header('Expires: '.gmdate('D, d M Y H:i:s \G\M\T', time() + (10)));
            readfile($filePath);
        }
        exit;
    }else{
        $response = 'Download link is not valid.';
    }
}else{
    // If the file has been downloaded already or time expired
    $response = 'Download link is expired.';
}

Hi @xede1 and welcome to the forums.

Try traversing the array in reverse, testing for a match and using unset($item);

I am using a tablet at the moment and unable to supply tested script. The free online PHP manual has some good examples.

Expiring keys are very nicely stored in Redis, where you can store a value and give it a Time To Live (TTL). After that time Redis will delete the key automatically.

With just a script that isn’t possible, and you need to live with the fact that you’ll have orphaned keys in your file.

One more I’d like to point out is that when you do using a file you may encounter race conditions.

Given two users A and B and given that key of A has value 1 and key of B has value 2:

A: Give me all keys (result: A=1, B=2)
B: Give me all keys (result: A=1, B=2)
A: Delete key for A (result: B=2)
B: Delete key for B (result: A=1)
A: Write remaining keys (result: B=2)
B: Write remaining keys (result: A=1) - still contains key from A, since B doesn’t know that A already removed their key, but the file should actually be empty now

That is why people use databases for this stuff, to avoid race conditions like this.

2 Likes

Isn’t better to execute a php script in a server’s cron job instead using Redis? As I am informed, not all servers support Redis… or I am totally wrong, maybe server’s cron job is done by Redis.
Please clarify this to me. Thank you

Well, Redis is a database server you need to install. Indeed on shared servers you don’t find it a lot, so if that’s what’s your using it might be hard.

Redis has nothing to do with cron by the way, they are separate things.

As for using cron, it still suffers from the race condition problems, so it’s up to you to decide if that’s worth it or not.

1 Like

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.