CODEWITHSUNDEEP
php
andreicek
andreicek

Reputation: 53

Problems with reading and searching thru txt file with PHP

I am writing a small script which checks if a users has his username in file called whitelist.txt or not. If the username is not found its added. This is the script:

$fh = @fopen("whitelist.txt", 'a+');
$stringData = $_POST[usr]. "\n";
if ($fh)
    {
        while (!feof($fh))
        {
            $buffer = fgets($fh);
            if (strpos($buffer, $stringData) == TRUE)
                echo "This username is already whitelisted.";
            elseif (strpos($buffer, $stringData) == FALSE) {
                    fwrite($fh, $stringData);
                    echo "Username ". $stringData. " is now whitelisted.";
                    }
        }
        fclose($fh);
    }

What I get now if I enter a new username first time, all is ok. The second time I enter a new username it gets dubbled. Problems go on: if I enter an existing username it gets added twice and the message "This username is already whitelisted." is not shown. Every username is in a new row.

Thank you for your time and help!

Upvotes: 0

Views: 99

Answers (5)

Starx
Starx

Reputation: 79069

I don't like you are already opening the file for writing without finding out whether a match occurs or not

$username = "thegreatone";
$lines = file("yourfile.txt");
foreach($line as $no => $line) {
    if(strstr($line,$username)) {
       //Now start the hand
       $fp = fopen('whitelist.txt', 'a+');
       fwrite($fp, '\r\n'.$username);
       fclose($fp);

       //Since the search was true... no need to continue the loop
       break;    
    } 
}

Upvotes: 0

David Myers
David Myers

Reputation: 799

Without digging too much into the reasons behind the way this code is written, I'd say that @marramgrass has solved the initial problem. However, I'd suggest reading the file into an array using file(). Then you can just do a simple check like this:

if(strtolower($_POST[usr]) == $Array[index])
{
    echo 'Username is already whitelisted';
}

That way the logic is dead simple and you're not dealing with strpos.

Upvotes: 0

Mark Willis
Mark Willis

Reputation: 1741

Remove the \n from the string data variable. This is not going to help. Also check the result from strpos against true or false with ===:

strpos() === TRUE

As 0 and false are the same thing with ==.

Upvotes: 0

marramgrass
marramgrass

Reputation: 1411

EDIT TO ADD: The accepted answer is great. As a counterpoint, I've modified your code below in the event that you want to continue to read line by line instead of all it once - the file is unlikely to get so big that doing it in one chunk is ever a problem, but making that kind of assumption always makes me very slightly nervous.

I see a couple of issues, here:

Even when you find the username in the file, you carry on through the rest of the file, getting false negatives. Try:

if (strpos($buffer, $stringData) == TRUE) {
    echo "This username is already whitelisted.";
    break;
}

Your else if will trigger whenever the script finds a line that doesn't match the submitted username, rather than when it gets to the end of the file. You'll need to move that check outside of the loop so that the new username is only added once. Altogether, now:

$fh = @fopen("whitelist.txt", 'a+');
$stringData = $_POST[usr]. "\n";
if ($fh)
    {
        $found = false;
        while (!feof($fh))
        {
            $buffer = fgets($fh);
            if (strpos($buffer, $stringData) == TRUE) {
                echo "This username is already whitelisted.";
                $found = true;
                break;
        }
        if (!$found) {
            fwrite($fh, $stringData);
            echo "Username ". $stringData. " is now whitelisted.";
        }
        fclose($fh);
    }

Upvotes: 1

Dustin Graham
Dustin Graham

Reputation: 2096

What about something like this:

$file = 'whitelist.txt';
$contents = file_get_contents($file);
if (strpos($contents, $stringData) === FALSE)
{
    $contents .= "\r\n" . $stringData;
    file_put_contents($file, $contents);
    echo 'Added ' . $stringData . ' to whitelist.';
}
else
{
    echo 'Already whitelisted.';
}

And yes, note that it is three equal signs. The problem you probably encountered was that == FALSE was matching the case where the $stringData was the first result (and thus 0, and we all know FALSE == 0) :)

Upvotes: 1

Related Questions