Problem
The code it self is pretty self explanatory, however i do not think it is very efficient at all. Is there a better way to do this?
Its basically so i can track spam sites and save the sites into my honeypot db.
<?php
$site = $_GET['url'];
$con=mysqli_connect("localhost","_","","_");
// Check connection
if (mysqli_connect_errno()) {
echo "Failed to connect to MySQL: " . mysqli_connect_error();
}
//Gambling
if (strpos($site, "betting") ||strpos($site, "blackjack") ||strpos($site, "casino") ||strpos($site, "craps") ||strpos($site, "football") ||strpos($site, "gamble") ||strpos($site, "gambling") ||strpos($site, "game") ||strpos($site, "gaming") ||strpos($site, "greyhound") ||strpos($site, "highroller") ||strpos($site, "lottery") ||strpos($site, "lotto") ||strpos($site, "odds") ||strpos($site, "poker") ||strpos($site, "racehorse") ||strpos($site, "racing") ||strpos($site, "roulette") ||strpos($site, "royalflush") ||strpos($site, "slotmachine") ||strpos($site, "slots") ||strpos($site, "slotsmachine") ||strpos($site, "sport") ||strpos($site, "wager") ||strpos($site, "paigow") ||strpos($site, "bingo") ||strpos($site, "baccarat"))
{
//Insert into gambling
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`gambling` (`id`, `url`) VALUES (NULL, '$site');");
}
//Payday
if (strpos($site, "payday") ||strpos($site, "loan") ||strpos($site, "bank") ||strpos($site, "money") ||strpos($site, "wonga") ||strpos($site, "lender") ||strpos($site, "credit") ||strpos($site, "debt") ||strpos($site, "repayment") ||strpos($site, "mortgage"))
{
//Insert into payday
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`payday` (`id`, `url`) VALUES (NULL, '$site');");
}
//Lawyer
if (strpos($site, "law") ||strpos($site, "solicitor") ||strpos($site, "government") ||strpos($site, "injury") ||strpos($site, "personal") ||strpos($site, "accident") ||strpos($site, "advice") ||strpos($site, "traffic"))
{
//Insert into lawyer
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`lawyer` (`id`, `url`) VALUES (NULL, '$site');");
}
//Weight Loss
if (strpos($site, "weight") ||strpos($site, "loss") ||strpos($site, "pills") ||strpos($site, "diet") ||strpos($site, "garcinia") ||strpos($site, "cambogia") ||strpos($site, "acai") ||strpos($site, "berry") ||strpos($site, "raspberry") ||strpos($site, "ketone") ||strpos($site, "coffee") ||strpos($site, "tea") ||strpos($site, "health") ||strpos($site, "lose") ||strpos($site, "surgery") ||strpos($site, "fat") ||strpos($site, "dieting") ||strpos($site, "exercise") ||strpos($site, "workout") ||strpos($site, "gym") ||strpos($site, "hypnosis"))
{
//Insert into weight loss
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`weightloss` (`id`, `url`) VALUES (NULL, '$site');");
}
//Insurance
if (strpos($site, "ppi") ||strpos($site, "insurance") ||strpos($site, "payment") ||strpos($site, "claim") ||strpos($site, "calculator") ||strpos($site, "finance") ||strpos($site, "mis-sold"))
{
//Insert into insurance
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`insurance` (`id`, `url`) VALUES (NULL, '$site');");
}
mysqli_close($con);
exit();
?>
Solution
Why did you create multiple tables for each category?
Surely it would be better to have 1 single table with something like:
- id – int
- category – enum(lawyer, insurance, etc.)
- url – varchar
You might want to have a look at this answer from StackOverflow for a better strpos
alternative to your requirements: https://stackoverflow.com/a/9220624/367708
You have a large amount of duplication in your conditions. additionally you have helluva lot of hardcoded strings all over the place. I suggest you change your structure to be something like:
<?php
$gamblingStrings = ["betting", "blackjack", "casino", "football", /*... */];
//Same for your other categories and then
if (matchesCategory($site, $gamblingStrings))
{
//insert into gambling
}
//and the corresponding function
private function matchesCategory($site, $strings) {
foreach ($strings as $matcher)
{
if(strpos($site, $matcher) !== false)
{
return true;
}
}
return false;
}
Disclaimer: My PHP is extremely rusty, the provided samples could be quite borked. But I hope you get the gist of it 😉
I would create an array of the search values then implode them when you want to evaluate using preg_match – something like this should work:
$rocket_newsites = array('betting','blackjack','sports'); //etc.
if(preg_match('/'.implode('|', $rocket_newsites).'/', $url, $matches))
{
$result = mysqli_query($con,"INSERT INTO `rocket_newsites`.`gambling` (`id`, `url`) VALUES (NULL, '$site');");
}
This, to me, is far more readable and maintainable than having such long conditions, you just have to add or remove words from the array rather than touch the condition itself, and I would imagine that it offers better performance than all of those strpos, although I’m not too sure on that so don’t take my word for it!