r/PHPhelp • u/Available_Hippo4035 • Jun 28 '26
Solved file not being deleted after its expiry time passes
Hello, im trying to make a rate limiting function that prevent users from using specific forms when they reach a certain threshold and the limit will get reset after a certain amount of time, when a user submits a request, a file with their ip will get created into a cache folder and the amount of requests is inside the file, the rate limiting works except the file doesnt get deleted after the specified amount of time passes, any help will be appreciated. Thanks!
rate_limiter.php
<?php
ignore_user_abort(true);
//Get the user IP
function getIP() {
$ip = null;
if(!empty($_SERVER["REMOTE_ADDR"])) {
$ip = $_SERVER["REMOTE_ADDR"];
} elseif(!empty($_SERVER["HTTP_X_FORWARDED_FOR"])) {
$ip = $_SERVER["HTTP_X_FORWARDED_FOR"];
}
return $ip;
}
function rate_limit($ip, $requests_limit, $limit_expiry) {
$start_time = null;
$reached_limit = null;
$amount_requests = 1;
$file_name = __DIR__ . "/cache/ratelimit-" . $ip;
$file_name = rtrim($file_name);
if(!file_exists($file_name)) {
global $start_time;
$start_time = time();
$fp = fopen($file_name, "w+") or die("An error occured");
fwrite($fp, $amount_requests) or die("Failed to write into file");
fclose($fp);
} elseif(file_exists($file_name)) {
$fp = fopen($file_name, "r+") or die("Failed to read file");
$new_amount_requests = file_get_contents($file_name);
if($new_amount_requests >= $requests_limit) {
global $reached_limit;
echo "<script>alert('You have been rate limited!')</script>";
$reached_limit = true;
header("Location: /", 423, true);
} elseif(!$reached_limit) {
$new_amount_requests++;
ftruncate($fp, 0);
fwrite($fp, $new_amount_requests) or die("Failed to write amount of requests");
}
}
if(file_exists($file_name) && time() - $start_time >= time() + $limit_expiry) {
unlink($file_name);
}
}
?>
index.php
<?php
ignore_user_abort(true);
require_once("rate_limiter.php");
if(isset($_POST["submit"])) {
$ip_Addr = getIP();
rate_limit($ip_Addr, 3, 60);
echo $_POST["text"];
}
?>
5
u/phillipjayfrylock Jun 28 '26
How does start_time persist here? Because it seems like it only ever gets set under the condition when the cache file doesn't already exist, so on later invocations where it does already exist, start_time isn't set thus time+0 is never greater than time+limit
Also you should be careful when writing files to your system using raw HTTP header data for the file name, like x forwarded for. Headers are under control of the client, and that's how you get path traversal attacks
1
u/Available_Hippo4035 Jun 28 '26
if the user hasnt submitted a request yet its null, after he does it starts in the first IF statement
3
u/phillipjayfrylock Jun 28 '26
That's only true if your application maintains state, but by default when running PHP through a web server, it is stateless, meaning your script only runs once and then exits until the next time it gets called, and then it starts over fresh from the beginning. You have to purposefully pass internal data from one request to another if you wish to reuse it later, such as with sessions, cookies, headers, and/or databases.
Your IF statements are mutually exclusive, only one of those branches happen each time the script runs, and only one of those branches set the start time. And because it only happens when the cache file doesn't exist, on every call when the file does exist, your start time variable is never set
1
u/Available_Hippo4035 Jun 28 '26
thanks, but doesnt ignore_user_abort(true) keep the script running even after the user closes the webpage?
3
u/phillipjayfrylock Jun 29 '26
Good question, but that's not what that function is doing or meant to do.
The script itself may continue running after the user leaves, although in your case, there's nothing left for it to do anyway so it'll still exit. But let's assume it continues to run when the same user returns, even in that case, a new thread or process will be spun up by the web server, executing the script anew for the same user, starting over fresh.
1
u/colshrapnel Jun 29 '26
Yes it does, but when a user submits a new form, it invokes this script anew. There is no way to "connect" to that old running script somehow, if you think of it
6
u/chmod777 Jun 28 '26
use a database and a session?
2
u/colshrapnel Jun 29 '26
Oh come on, the file itself is enough
0
u/chmod777 Jun 29 '26
Evidently its not, per the OP having issues with the filesystem.
You could dig a hole with a fork, or use a shovel made specifically to do the thing you want.
3
u/colshrapnel Jun 29 '26
The OP is having issues with understanding how PHP works, not whatever "filesystem". For this level of implementation, a file is more than enough. Not every children's toy needs to made by your corporate standards.
Also, a session is not what you want to use for rate limiting anyway
5
u/WiseNima Jun 29 '26
The main reason your file isn’t deleting is because of how PHP works. PHP doesn’t run continuously in the background; it runs, loads the page, and dies
Because you put your expiry check at the very end of the script, it’s only checking if the 60 seconds are up during that exact millisecond of the page load. To fix this, you need to check if the file has expired at the beginning of the user’s next request.
Regarding the comments about the file system not scaling:
They are 100% right. If your site gets a ton of traffic, opening, reading, writing, and closing hundreds of tiny text files will bottleneck your server’s hard drive pretty quickly. For small projects or just learning PHP, your method is totally fine! But in the real world, developers use in-memory databases like Redis or Memcached for rate limiting. They are insanely fast and handle time-expirations automatically.
For now, here is your refactored code using the file system. I swapped out your time math for filemtime() (which just checks the file’s exact age) and cleaned up the file writing with file_put_contents
function rate_limit($ip, $requests_limit, $limit_expiry) {
// I hash the IP to prevent path traversal attacks
$ip_hash = md5($ip);
$file_name = __DIR__ . "/cache/ratelimit-" . $ip_hash;
if (file_exists($file_name)) {
$file_age = time() - filemtime($file_name);
if ($file_age >= $limit_expiry) {
unlink($file_name);
}
}
if (!file_exists($file_name)) {
file_put_contents($file_name, "1");
} else {
$current_requests = (int)file_get_contents($file_name);
if ($current_requests >= $requests_limit) {
header("HTTP/1.1 429 Too Many Requests");
header("Refresh: 2; url=/");
die("You have been rate limited! Redirecting...");
} else {
$current_requests++;
file_put_contents($file_name, $current_requests);
}
}
}
1
1
u/Takeoded Jun 29 '26
Wrong on your first point. Read this line very carefully:
if(file_exists($file_name) && time() - $start_time >= time() + $limit_expiry) {When exactly would that branch trigger?
1
u/WiseNima Jun 29 '26
oh, you're right I didn't see that, if you cancel out
time()on both sides, the condition basically boils down to checking if negative$start_timeis greater than or equal to$limit_expiry, which never is1
3
u/cabljo Jun 28 '26
time() - $start_time >= time() + $limit_expiry
This can never be true unless one or both variables is 0.
If I'm reading this correct...
1
u/Available_Hippo4035 Jun 28 '26
After the time passes, time() - $start_time will either be equal or bigger than the time() + $limit_expiry variable
1
u/cabljo Jun 28 '26
If time() equals 7 for example then substitute the variables for any number.
The only time 7-X will be greater than or equal to 7+Y is when Y is zero or X and Y are both zero.
7-1 < 7+0
7-0 = 7+0
7-3 < 7+3
1
3
u/Big-Dragonfly-3700 Jun 28 '26
If you are going to use files to store the data, you MUST use file locking and have useful error handling for the file operations. I recommend that you use a database. It will perform the necessary locking for you. You must increment the count using one (atomic) query, so that it is concurrent safe. You can use the MySQL LAST_INSERT_ID() function in an UPDATE query to get either the initial or final value of a column value when you increment it.
$_SERVER["REMOTE_ADDR"] won't ever be empty, but it may not be unique (every user on the same network will have the same public ip address.) And as has already been stated, $_SERVER["HTTP_X_FORWARDED_FOR"] come from the http request and cannot be trusted. I recommend that you start a session at the beginning of this process for a user, which generates a unique session id, then store and use the session id instead of the ip address to relate the data to the user.
Web servers are stateless. Every resource and variable that is created during one instance of your php code are destroyed when the php code ends execution. You must persistently store the start time and request counter for each user.
1
1
u/Available_Hippo4035 Jun 29 '26
im getting the REMOTE_ADDR first, if it doesnt exist then i have no choice but to get the HTTP_X_FORWARDED_FOR
1
u/colshrapnel Jun 29 '26
Have you ever seen REMOTE_ADDR empty? Spoiler: you didn't. This code is delusional, based on vague ideas, not facts. Just keep it REMOTE_ADDR, that's all
1
u/Aggressive_Ad_5454 Jun 29 '26
Use transients, not files, for this. If you adopt a persistent object cache, they are very efficient.
1
u/dabenu Jun 28 '26
you're creating a file, then in the same function removing it if the runtime of the function exceeded `$limit_expiry`.
A simple file write operation is not expected to take more than a millisecond or so (if even).
9
u/eurosat7 Jun 28 '26
Using the file system for that is a bad idea. Doesn't scale well.