Einfacher PHP Counter

[bischi]

Ich hab folgendes einfache PHP Script gefunden (ist ja nicht besonders schwer nachzuvollziehen, was das Script macht - selbst wenn man wie ich kein PHP kann :D).



<?php
if (file_exists('count_file.txt'))
{
$fil = fopen('count_file.txt', r);
$dat = fread($fil, filesize('count_file.txt'));
echo $dat+1;
fclose($fil);
$fil = fopen('count_file.txt', w);
fwrite($fil, $dat+1);
}
else
{
$fil = fopen('count_file.txt', w);
fwrite($fil, 1);
echo '1';
fclose($fil);
}
?>


Eingebunden werden soll das Script über ein iframe. Dazu zwei Fragen:
1) Wie sieht es mit der Sicherheit von solchen Scripten aus? Irgendwelche offensichtlichen Fehler/Sicherheitslücken?
2) Gibt es neben der Variante übers Einbinden per iframe noch andere Alternativen? Ich mag iframes nicht besonders gut ;)

MfG Bischi

PS: Falls irgendwer kreative Verbesserungsvorschläge hat - nur her damit :D
PS2: Falls jemand einen sicheren, zuverlässigen, open-source Page-Hit-Counter kennt, der ausserdem nur PHP und keine Datenbank braucht -> nur her damit ;)

[BLUESCREEN3D]

Irgendwelche offensichtlichen Fehler/Sicherheitslücken?
Ich sehe 3 Race Conditions. Das Script ist also vollkommen unbrauchbar.


kreative Verbesserungsvorschläge
Lass den Besucherzähler weg. Der Besucher hat nichts davon, diese unwichtige Zahl zu sehen.

[msi]

Ich sehe 3 Race Conditions. Das Script ist also vollkommen unbrauchbar.


Lass den Besucherzähler weg. Der Besucher hat nichts davon, diese unwichtige Zahl zu sehen.

das die race condition auftritt ist extrem unwahrscheinlich, davor geht dir deine festplatte 3mal kaputt. sicherheitslücken sind keine drinnen

[bischi]

Ich sehe 3 Race Conditions. Das Script ist also vollkommen unbrauchbar.

Und wenn du mir jetzt noch sagst, wo die sind ;) (also theoretisch könnte mehrmals gleichzeitig auf die Datei zugegriffen werden - aber dann geht - soweit ich das versteh - halt einfach ein Besucher "verloren"...)

Das einzige Problem, das ich noch sehe ist, dass ich Fehler wie "konnte Datei nicht öffnen" nicht abfangen...


Lass den Besucherzähler weg. Der Besucher hat nichts davon, diese unwichtige Zahl zu sehen.

Ich will die Zahl dem Benutzer auch nicht anzeigen ;) Was mich interessiert ist, welche Unterseiten wie oft aufgerufen werden :D

MfG Bischi

PS: Ich bin aktuell sowieso noch an einer Verbesserung am rumbasteln - ich melde mich wieder, sobald ich da ne einigermassen brauchbare Version hab :)

[BlueJay]

Und wenn du mir jetzt noch sagst, wo die sind ;) (also theoretisch könnte mehrmals gleichzeitig auf die Datei zugegriffen werden - aber dann geht - soweit ich das versteh - halt einfach ein Besucher "verloren"...)

Das einzige Problem, das ich noch sehe ist, dass ich Fehler wie "konnte Datei nicht öffnen" nicht abfangen...

... musst du halt ein flock drumherumbauen:
http://de3.php.net/flock




Ich will die Zahl dem Benutzer auch nicht anzeigen ;) Was mich interessiert ist, welche Unterseiten wie oft aufgerufen werden :D


Dann brauchste auch kein iframe, welches nur zum Anzeigen dient. Lass die echo-Zeilen eingfach weg.

Mal 'ne Frage: wieso wertest du dann nicht die logfiles aus?

Etwa so:

#!/bin/sh
# Logfileauswertung
echo "checke logs...\n"
grep "GET \/dingsbums\/index.htm" acc-3101 >abdiepost.log
echo "feddich!"

Dann lässt du dir die Zeilennummern anzeigen, scrollst runter, feddich!

so long,
BlueJay

[bischi]

... musst du halt ein flock drumherumbauen:
http://de3.php.net/flock

Dachte mir noch fast, dass es so was geben sollte :D



Dann brauchste auch kein iframe, welches nur zum Anzeigen dient. Lass die echo-Zeilen eingfach weg.
Klar lass ich die echo-Zeilen weg ;) Aber wie muss ich das dann einbinden? Einfach ins HTML-File reinkopieren? Und dann noch nach .php umbenennen? (ich bastel gerade noch etwas an ner Version mit Cookies rum, die nach 24 Stunden verfallen...)



Mal 'ne Frage: wieso wertest du dann nicht die logfiles aus?

Weil ich auf die afaik keinen Zugriff hab ;) Ich hab gewissermassen nur ein wenig Webspace, auf dem zudem PHP läuft :D

MfG Bischi

[ContainerDriver]

Klar lass ich die echo-Zeilen weg ;) Aber wie muss ich das dann einbinden? Einfach ins HTML-File reinkopieren? Und dann noch nach .php umbenennen?


Genau.

Gruß, Florian

[bischi]

So - ich wollte mal ausprobieren, wie lange es braucht, um PHP zu "lernen" (bis jetzt gute 4 Stunden) :D Ich freu mich über sämtliche Rückmeldungen :)



<?

///////////////////////////////////////////////////////////////////////////////////////
// Copyright (C) 2009 Dominik Bischoff
//
// This program is free software; you can redistribute it and/or
// modify it under the terms of the GNU General Public License
// as published by the Free Software Foundation version 2 of the License.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program; if not, write to the Free Software
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
///////////////////////////////////////////////////////////////////////////////////////

// This script provides a counter for a webpage. Reloads from the same
// person are blocked for 24 hours by giving the person a cookie.


// counter save filename
$filename = "counter.txt";
// file structure
// ------------------------------
// mmddyyyy TotalToday TotalUntilThatDay
// mmddyyyy TotalToday Total
// ------------------------------


// actual date
$ddmmyy = date("dmY");

// show errors
error_reporting(E_ALL);
ini_set("display_errors", 1);

// read IP address
$ip = getenv("REMOTE_ADDR");


// set cookie with
// name : "beenhere"
// content : the ip address of the user
// expiration date : the actual time plus 24 hours
// !!! no 'echo' before setcookie() -> produces error !!!
setcookie("beenhere", $ip, time()+24*3600);



// test if cookie is already set
if(isset($_COOKIE['beenhere']))
{
// echo "already visited <br>";
}
// case: cookie not set => new visitor
else
{
// echo "new visitor <br>";

$lines = '';
// check if file existing
if(file_exists($filename))
{
// read whole file
// each array entry is a line
$lines = file($filename);
}

// check if file is 'not existing' OR 'available and writeable'
if(!file_exists($filename) || is_writeable($filename))
{
// create file handle and check if file can be opened
// 'w+' : read and write, delete actual content
if ($fp = fopen($filename, 'w+'))
{
// save actual time: Do not wait longer than a second
// to avoid having to many open scripts
$startTime = microtime();
do {
// check if this script can get a lock on the desired file
$lockObtained = flock($fp, LOCK_EX);

// if no lock obtained, wait random time between 0 and 50 milliseconds
if(!$lockObtained)
{
usleep(round(rand(0, 50)*1000));
}
// if no lock obtained, repeat until lock obtained a time passed
} while ( (!$lockObtained) and ((microtime()-$startTime) < 1000));

// check if the file is locked
if ($lockObtained)
{
// file is now locked!

// 1 for todays date already in file
$founddate = 0;
// actual total of visitors
$actualtotal = 0;

foreach ($lines as $line_num => $line)
{
// split stream by " "
$words = explode(" ",$line);

// check if enough values in array
// check if second and third value are numerics
if(count($words)>2 && is_numeric($words[1]) && is_numeric($words[2]))
{
// save latest total
$actualtotal = $words[2];

// check if read out date is equal to todays date
$same = strcmp($ddmmyy,$words[0]);

// date match
if($same == 0)
{
// increment both counters and then write into file
fwrite($fp, $words[0]." ".($words[1]+1)." ".($words[2]+1)." \n");
$founddate = 1;
}
else
{
// date does not match - just write original line back into file
fwrite($fp, $line);
}
}
// pattern does not match - write line back
else
{
fwrite($fp, $line);
}
}

// todays date not yet in file: write new line
if($founddate == 0)
{
fwrite($fp, $ddmmyy." 1 ".($actualtotal+1)." \n");
}
}
else
{
// echo "couldn't get a lock on file - $filename";
}
fclose($fp);
}
else
{
// echo "$filename - fopen=false <br>";
}
}
else
{
// echo "$filename - is_writeable=false <br>";
}
}

?>


Lg Bischi

PS: Funktionierendes Beispiel findet man aktuell:
Die aktuelle Zählerstand (in counter.txt): http://people.ee.ethz.ch/~dominikb/counter.txt
Der Counter: http://people.ee.ethz.ch/~dominikb/counter2.php

[ContainerDriver]

Naja, zumindest eine Race-Condition ist noch drinnen, es könnten mehrere Benutzer gleichzeitig die Datei auslesen und dann nacheinander den ausgelesenen Wert + 1 zurückschreiben. Ich würde mit einer Lock-Datei arbeiten (um das ganze Konstrukt außenrum).

Gruß, Florian

[bischi]

Hmm - fürs lesen brauch ich doch kein Lock... Und fürs Schreiben hab ich eins... Ok - könnte sein, dass ich damit dann gewisse Besucher verlier, weil ich die alte Datei gelesen hab... Hmmm - werd ich ändern :D

MfG Bischi

[bischi]

Ok - neue Version:



<?php

///////////////////////////////////////////////////////////////////////////////////////
// Copyright (C) 2009 Dominik Bischoff
//
// This program is free software; you can redistribute it and/or
// modify it under the terms of the GNU General Public License
// as published by the Free Software Foundation version 2 of the License.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program; if not, write to the Free Software
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
///////////////////////////////////////////////////////////////////////////////////////

// This script provides a counter for a webpage. Reloads from the same
// person are blocked for 24 hours by giving the person a cookie.


// counter save filename
$filename = "counter.txt";
// file structure
// ------------------------------
// mmddyyyy TotalToday TotalUntilThatDay
// mmddyyyy TotalToday Total
// ------------------------------


// actual date
$ddmmyy = date("dmY");

// show errors
error_reporting(E_ALL);
ini_set("display_errors", 1);

// read IP address
$ip = getenv("REMOTE_ADDR");


// set cookie with
// name : "beenhere"
// content : the ip address of the user
// expiration date : the actual time plus 24 hours
// !!! no 'echo' before setcookie() -> produces error !!!
setcookie("beenhere", $ip, time()+24*3600);



// test if cookie is already set
if(isset($_COOKIE['beenhere']))
{
// echo "already visited <br>";
}
// case: cookie not set => new visitor
else
{
// echo "new visitor <br>";

// check if file is 'not existing' OR 'available and writeable'
if(!file_exists($filename) || is_writeable($filename))
{
// create file handle and check if file can be opened
// 'r+' : read and write, set file pointer to begin of file
if ($fp = fopen($filename, 'r+'))
{
// save actual time: Do not wait longer than a second
// to avoid having to many open scripts
$startTime = microtime();
do {
// check if this script can get a lock on the desired file
$lockObtained = flock($fp, LOCK_EX);

// if no lock obtained, wait random time between 0 and 50 milliseconds
if(!$lockObtained)
{
usleep(round(rand(0, 50)*1000));
}
// if no lock obtained, repeat until lock obtained a time passed
} while ( (!$lockObtained) and ((microtime()-$startTime) < 1000));

// check if the file is locked
if ($lockObtained)
{
$lines = '';
// read whole file
// each array entry is a line
$lines = file($filename);

// file is now locked!

// 1 for todays date already in file
$founddate = 0;
// actual total of visitors
$actualtotal = 0;

foreach ($lines as $line_num => $line)
{
// split stream by " "
$words = explode(" ",$line);

// check if enough values in array
// check if second and third value are numerics
if(count($words)>2 && is_numeric($words[1]) && is_numeric($words[2]))
{
// save latest total
$actualtotal = $words[2];

// check if read out date is equal to todays date
$same = strcmp($ddmmyy,$words[0]);

// date match
if($same == 0)
{
// increment both counters and then write into file
fwrite($fp, $words[0]." ".($words[1]+1)." ".($words[2]+1)." \n");
$founddate = 1;
}
else
{
// date does not match - just write original line back into file
fwrite($fp, $line);
}
}
// pattern does not match - write line back
else
{
fwrite($fp, $line);
}
}

// todays date not yet in file: write new line
if($founddate == 0)
{
fwrite($fp, $ddmmyy." 1 ".($actualtotal+1)." \n");
}
}
else
{
// echo "couldn't get a lock on file - $filename";
}
fclose($fp);
}
else
{
// echo "$filename - fopen=false <br>";
}
}
else
{
// echo "$filename - is_writeable=false <br>";
}
}

?>




MfG Bischi

[BLUESCREEN3D]

if(!file_exists($filename) || is_writeable($filename))

Weglassen. Versuch einfach, die Datei zum Schreiben zu öffnen.
Und falls die Datei nicht existiert wird sie von dem Script auch nicht angelegt werden.



do {
// check if this script can get a lock on the desired file
$lockObtained = flock($fp, LOCK_EX);

// if no lock obtained, wait random time between 0 and 50 milliseconds
if(!$lockObtained)
{
usleep(round(rand(0, 50)*1000));
}
// if no lock obtained, repeat until lock obtained a time passed
} while ( (!$lockObtained) and ((microtime()-$startTime) < 1000));

Das ist unnötig. flock() wartet, bis es den Lock kriegt. Falls flock() aus irgendwelchen Gründen gehlschlägt, wird es wohl kaum ein paar Millisekunden später funktionieren.
Außerdem: Benutz immer mt_rand() statt rand().



$lines = file($filename);

Nur zur Information: Die Datei wird hier ein zweites Mal geöffnet. Das funktioniert nur, weil file() den Lock ignoriert.



$lines = '';

Unnötig.



// file is now locked!

Das ist schon ein paar Zeilen zuvor so gewesen.



strcmp($ddmmyy,$words[0])

Hier reicht ein ($ddmmyy == $words[0]).



" \n"

Das Leerzeichen brauchst du nicht.

[bischi]

Erstmal: Merci für die vielen Kommentare!



if(!file_exists($filename) || is_writeable($filename))

Weglassen. Versuch einfach, die Datei zum Schreiben zu öffnen.
Und falls die Datei nicht existiert wird sie von dem Script auch nicht angelegt werden.

Doch - das Script erstellt die Datei sonst - probiers aus ;)




do {
// check if this script can get a lock on the desired file
$lockObtained = flock($fp, LOCK_EX);

// if no lock obtained, wait random time between 0 and 50 milliseconds
if(!$lockObtained)
{
usleep(round(rand(0, 50)*1000));
}
// if no lock obtained, repeat until lock obtained a time passed
} while ( (!$lockObtained) and ((microtime()-$startTime) < 1000));

Das ist unnötig. flock() wartet, bis es den Lock kriegt. Falls flock() aus irgendwelchen Gründen gehlschlägt, wird es wohl kaum ein paar Millisekunden später funktionieren.

Ich hab leider nichts weiter darüber gefunden. Warum ich das so gemacht habe: Ich hab angenommen, dass zu exakt dem Zeitpunkt, an dem das Script den flock-Aufruf erreicht, geschaut wird, ob ein Lock erhalten wird. Aber es könnte ja sein, dass das gleiche Script schon einmal läuft und gerade gelockt hat... In dem Fall hätte ein Lock ein paar ms später sehr wohl eine Chance...

Was mir gerade noch aufgefallen ist: Ich hab ein

flock($fp, LOCK_UN);

vergessen...



Außerdem: Benutz immer mt_rand() statt rand().
Jup - werd ich ändern! Gut zu wissen (auch wenn hier weder die Geschwindigkeit noch eine gute Zufallszahl wirklich von Nöten ist :D).




$lines = file($filename);

Nur zur Information: Die Datei wird hier ein zweites Mal geöffnet. Das funktioniert nur, weil file() den Lock ignoriert.


Jup - das ist zumindest teilweise Absicht. Da ich ja ein Lock hab, kann nichts passieren ;) Und PHP wird ja sequentiell abgearbeitet :D




$lines = '';

Unnötig.

Jup - hab ich vergessen wieder rauszulöschen :D




// file is now locked!

Das ist schon ein paar Zeilen zuvor so gewesen.


Aber erst nach dieser Zeile weiss ich auch, ob ich ein Lock hab oder nicht :D Oben dran könnte es ja sein, dass flock fehlgeschlagen hat...




strcmp($ddmmyy,$words[0])

Hier reicht ein ($ddmmyy == $words[0]).

Hmm - Angewohnheit aus Java :) Was ist denn genau der Unterschied in PHP? (In Java ist ein String ein Objekt... Zwei identische Strings in zwei Objekten sind aber nicht gleich...)




" \n"

Das Leerzeichen brauchst du nicht.

Wieso?

MfG Bischi

[BLUESCREEN3D]

Doch - das Script erstellt die Datei sonst - probiers aus ;)
Eig. sollte fopen() fehlschlagen anstatt die Datei zu erstellen.


Was mir gerade noch aufgefallen ist: Ich hab ein
flock($fp, LOCK_UN);
vergessen...
Das passiert bei fclose() automatisch.


Hmm - Angewohnheit aus Java :) Was ist denn genau der Unterschied in PHP? (In Java ist ein String ein Objekt... Zwei identische Strings in zwei Objekten sind aber nicht gleich...)
Strings sind in PHP ein primitiver Datentyp und keine Klasse. Deshalb sind die Stringvariablen auch keine Referenzen, sondern enthalten direkt das Objekt.


Wieso?
explode() trennt den String an den Leerzeichen und wenn am Ende ein Leerzeichen steht, wird daran auch getrennt und du hast in der Rückgabe von explode() noch einen Arrayeintrag, der einen leeren String enthält.

[BlueJay]

explode() trennt den String an den Leerzeichen

Man lernt doch immer noch dazu! Bisher dachte ich immer, man muss den Trenner angeben.

@Bischi:
war meine Einstiegslektüre in php:
http://de2.php.net/manual/de/

Obwohl ich schon fit in Javascript bin, kann ich auch nach über 5 Jahren php nicht besonders gut und muss oft dort nachschauen, wenn es sich nicht gerade um Datenschubsereien handelt.

Es scheint, als wolltest du die Zugriffe doch ausgeben, statt via ftp in den counter-Dateien nachzusehen.
Du kannst dir auch ein php-Progrämmchen basteln, was hübsch sortiert den Kram entweder selbst anzeigt oder in ein bestehendes HTML-Dokument rüber(a)jaxt.
Du wirst es noch lieben, dass php den String als primitiven Datentyp ansieht, die Möglichkeiten sind schier endlos!

so long,
BlueJay

[BLUESCREEN3D]

explode() trennt den String an den LeerzeichenMan lernt doch immer noch dazu! Bisher dachte ich immer, man muss den Trenner angeben.
Muss man auch, aber er hat ja ein Leerzeichen als Trenner angegeben und es ging in meinem Kommentar nur darum, dass er am Ende des Strings nicht noch ein Leerzeichen braucht.

[BlueJay]

oha, man sollte 1. seine Computerbrille aufsetzen und 2. auch mal etwas scrollen :o