I-BOOK księga gości(ocena+audyt)

[gogulas]

" sie nie uzywa, $x=$_SERVER['x'] jest bez sensu a po header() wszsytko sie wykonuje jak leci.

Heh, racja, pierwsze 5 linijek kazdego pliku zdeczka traci aprofesjonalizmem:P otworzylem pierwszy lepszy plik i zobaczylem to j.w

$s=$_SERVER['PHP_SELF']; /* Hmm, czy kazdorazowy dostep do 
superglobalnej jest jakos bardziej czasochlonny ? :p
jesli nie to po co to? */

include("i-book.class.php"); /* powinno byc include('i-book.class.php');
szkoda pracy interpretatora na sprawdzanie czy miedzy komentarzami 
nie ma jakis specjalnych znakow... */

include("config.php"); /* j.w */

if($_SESSION['pass']!==$admin_pass)    header("Location: $s?file=login");
 /* skrypt nie konczy pracy po wyslaniu headera... normalnie 
przegladarka zostanie przekierowana, ale nienormalnie haxi0r wejdzie 
na prawach adma, zrob die(header());p
 (hmmm chyba ze serek wysyla jakis standardowy tekst przy 301
 i konczy zabawe, mozna to latwo sprawdzic)  */ 

Nie wiem po co zaczynacie flame, iron prosil o opinie i je dostaje w dodatku same sluszne uwagi :P

Sam jestem wysoce miekki w szukani bledow, przegladajac skrypt zapewne omijam mase logicznych pulapek...

    final public function is_connect()
    {
        if(@mysql_connect($this->adres_serwera,$this->user,$this->haslo))
        {
            $db=mysql_select_db($this->baza);
            if($db) return true; else return false;
        }
    } 

a czy tak nie bylo by lannndniej, sprytniej i fikusniej?

    final public function is_connect()
    {
        if(@mysql_connect($this->adres_serwera,$this->user,$this->haslo))
            if ( ($db=mysql_select_db($this->baza)) == false)
                return false; 
        return true;
    } 

Nasuwa sie pytanie, co po zakonczeniu dzialania funkcji is_connect() dzieje sie z tym uchwytem $db.... jakos tego nie lapie...

Pamietaj ze return konczy dzialanie funkcji, wiec dawanie else return czesto nie ma sensu.

Moja rada, posciagaj jakies szanujace sie CMSy i przeczytaj je od dechy do dechy, nabierzesz dobrych nawykow..

dalej...

    final public function usun_bany()
    {
        $tabela=$this->prefix."banicja";
        $zap=mysql_query("TRUNCATE TABLE $tabela");
        if(!$zap) $this->mysql_error();
    } 

wg mnie:

        final public function usun_bany() {
            $tabela=$this->prefix."banicja";
            if (mysql_query("TRUNCATE TABLE $tabela") == false) $this->mysql_error();
            /*return cokolwiek; funkcja powinna sie konczyc true, false, albo cos zwracac, znasz jakas funkcje z jadra php ktora nic nie zwraca? :D */
        } 

dalej...

            $a=0;
            while($wiersz=mysql_fetch_array($zap))
            {
                $this->bany_ip[$a]=$wiersz['ip'];
                $this->bany_id[$a]=$wiersz['id'];
                $a++;
            } 

Niby dziala, ale petla for jest po to coby jej uzywac!

                for ($a=0; $wiersz=mysql_fetch_array($zap); $a++) 
            {
                $this->bany_ip[$a]=$wiersz['ip'];
                $this->bany_id[$a]=$wiersz['id'];
            } 

dalej...

    final protected function is_banned()
    {
        $ip=real_ip();
        $this->pobierz_bany();
        if(in_array($ip,$this->bany_ip)) return true; else return false;
    } 

Ja bym to napisal tak:

    final protected function is_banned()
    {
        $this->pobierz_bany();
        if(in_array(real_ip(),$this->bany_ip))
            return true;
        return false;
    } 

Praktycznie kazda funkcja nadaje sie do poprawki wiec chyba nie ma sensu tego dalej ciagnac