PHP Session Destroy - wie eine Session beenden, die es nicht mehr gibt ?

[Blackbenji]

Hallo,

ich erhalte die Meldung "session_destroy(): Trying to destroy uninitialized session"

Mein Anwendungsbeispiel mag etwas komisch aussehen:

- auf localer website eingeloggt
- in phpmyadmin alle einträge gelöscht
- sql dump wieder eingespielt
- auf localer website neu geladen

obwohl username/passwort identisch sind werde ich ausgeloggt.
das ist soweit auch richtig, aber die meldung seitens php irritiert mich dann doch etwas.

session_start(); wird im __construct() aufgerufen, damit sollte der fall eigentlich ausgeschlossen sein, oder ?

 

[voodoo44]

... wird die Session denn vielleicht irgendwo anders zerstört ?!
Dass im Konstruktor irgendwo session_start() aufgerufen wird, ist ja toll - nützt dir aber nicht viel, wenn du sie wo anders wieder abschießt.

Von einem 'Senior Application Administrator' erwartet man doch eigentlich ein paar mehr Details oO

 

[Blackbenji]

ich kann es inzwischen eingränzen.
probleme macht die "checkUser" funktion.
diese wird ebenfalls im construct aufgerufen, da sie bei jedem reload, seitenwechsel, aktion den user überprüfen soll.

das problem ist nur, dass die daten in der session noch aktiv sind, somit die if-abfrage immer true liefert.

    public function checkUser() {
        // Alte Session löschen und Sessiondaten in neue Session transferieren
        if (isset($_SESSION['loggedin']) == true) {
            session_regenerate_id( true );
            $result = $this->database->query("SELECT `ip`, `userinfo`, `login`, UNIX_TIMESTAMP(`lastlogin`) as lastlogin
			FROM ".GLOBAL_USER_DB." WHERE `email` = '" .mysql_real_escape_string( $_SESSION['email'] ). "' AND `activated` = 1");
            if (mysql_num_rows( $result ) == 1) {
                $benutzerdaten = mysql_fetch_array( $result );
                // Resourcen freigeben
                mysql_free_result( $result );
                // Daten aus der DB mit den Benutzerdaten vergleichen
                if ($benutzerdaten['ip'] != $_SERVER['REMOTE_ADDR']) { $this->resetUser(); }
                if ($benutzerdaten['userinfo'] != $_SERVER['HTTP_USER_AGENT']) { $this->resetUser(); }
                if ($benutzerdaten['login'] != $_SESSION['login']) { $this->resetUser(); }
                if (!(($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME'])) { $this->resetUser(); }
            } else {
                $this->resetUser();
            }
            $this->updateUser($_SESSION['email']);
            return true;
        } else {
            return false;
        }
    }

Ergänzung (9. Januar 2013)

ich habe das problem inzwischen gelöst. es macht natürlich keinen sinn session werte abzufragen die immer ein "true" zurück geben.
die lösung:
- erweiterung der sql tabelle um usersession
- beim loggin den session_id wert in die tabelle speichern
- $_SESSION['usersession'] = session_id();
- bei jedem checkUser die usersession gegen die sql-tabelle vergleichen.

problem gelöst

 

[voodoo44]

So sollte man PHP aber nicht benutzen ... :/

$benutzerdaten['login'] != $_SESSION['login'] => $benutzerdaten['login'] !== $_SESSION['login']
!(($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME']) => (($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME']) === false
GLOBAL_USER_DB => ... No-Go!
Direktzugriff auf $_SESSION/$_SERVER => ... No-Go!

1. Kapselung in Objekte ist toll
2. typensicherer Vergleich ist spitze!
3. implizites vergleichen stinkt, explizite Vergleiche sind deutlich übersichtlicher, besser lesbar und 1337
4. Verwendung von Konstanten und globalen Variablen sollte man tunlichst unterlassen
5. Denpendency-Injection macht Code qualitativ hochwertiger und sehr viel flexibler einsetzbar

 

[carom]

2. typensicherer Vergleich ist spitze!

An Stellen, an denen es Sinn macht. Wo ist der Sinn hierbei?

($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME']) === false


Mich würde noch interessieren, was du mit 3 meinst. Was ist bei 4 dein Problem mit Konstanten?

 

[voodoo44]

Wo der Sinn bei 2. ist? Na dabei, dass man einen String mit einem String und nicht mit 'irgendwas' vergleichen sollte? Dass man keinen int auf nen String typsicher vergleicht, ist klar - würde ja auch keinen Sinn machen

Wo ist der Sinn hierbei?
($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME']) === false

öhm ... man kann es einfacher und schneller lesen, als dieses Ausrufezeichen davor, welches einen impliziten Vergleich darstellt und auch schnell mal übersehen wird, wenn man viel Quelltext überfliegt. Das ist im übrigen mit 3. gemeint.

Bei 4. - nun, mein 'Problem' ist, dass er dort eine Konstante verwendet, die den Namen der Datenbank beinhaltet. Das gehört sich nicht und widerspricht dem Gedanken von OOP. Sauberer könnte man das Lösen, indem man das über eine Instanzvariable löst.
Macht man das bei etwas umfangreicheren Projekten, wird's quasi untestbar. Als Klassenproperty könnte ich es z.B. direkt über den Konstruktor 'reinstopfen' und verwenden, was meine (einmal geschriebene) Klasse auch leichter verwend- und ersetzbar macht.
Dazu geht auch noch schlichtweg die Übersicht verloren, wenn ich an tausend Stellen irgendwo im Quelltext irgendwelche Konstanten verwende. Ist schlichtweg unsauberer Code, den man so nicht schreiben sollte - nur weil man's kann (PHP eben), muss man's nicht machen.

Und auf sowas sollte ein 'Senior' eben schlichtweg achten - darum geht's mir. Das springt mir beim ersten mal überfliegen in's Auge und schmerzt da einfach nur. Das ist in etwa so wie: 'Erzähl mal ein Witz' ... da geht mir die Hutschnur hoch ^^

 

[carom]

Wo der Sinn bei 2. ist? Na dabei, dass man einen String mit einem String und nicht mit 'irgendwas' vergleichen sollte? Dass man keinen int auf nen String typsicher vergleicht, ist klar - würde ja auch keinen Sinn machen

Ich beschäftige mich zugegebenermaßen selten mit PHP, aber das von mir zitierte macht imho keinen Sinn und ist auch nicht das, was du willst. Der Ausdruck in der Klammer wird zuerst evaluiert und gibt aufgrund der Definition des größer-gleich Operators immer einen boolschen Wert zurück. Willst du den ernsthaft nochmal typsicher auf bool prüfen? Mit den Werten in der Klammer hat das jedenfalls nichts mehr zu tun.

öhm ... man kann es einfacher und schneller lesen, als dieses Ausrufezeichen davor, welches einen impliziten Vergleich darstellt und auch schnell mal übersehen wird, wenn man viel Quelltext überfliegt. Das ist im übrigen mit 3. gemeint.

Ah, verstehe. Das ist aber eigentlich eher verpöhnt und überhaupt nicht "1337", wie du es genannt hast, eher anfängermäßig. Mag sein, dass das bei PHP etwas lockerer ist aufgrund des expliziten Vergleichens zur Sicherstellung der Typsicherheit, aber ansonsten findest du sowas nirgends, kannst dir ja mal ein größeres Open-Source Projekt deiner Wahl anschauen. Du schreibst ja auch nicht if i == 7 == false.

http://www.mycsharp.de/wbb2/thread.php?threadid=17536

Bei 4. - nun, mein 'Problem' ist, dass er dort eine Konstante verwendet, die den Namen der Datenbank beinhaltet. Das gehört sich nicht und widerspricht dem Gedanken von OOP.

Stimmt, so wie er das gemacht hat ist das natürlich nicht schön. Ansonsten beißen sich Konstanten und OOP aber keinesfalls. Eine Konstante ist zunächst mal nichts anderes als ein nicht veränderbarer Wert, das war's.
Du gehst imho davon aus, dass Konstanten stets global verfügbar sind, das ist aber nicht der Sinn einer Konstante, auch wenn Konstanten gerne hierfür benutzt werden. Und wenn schon, dann kommt es immer noch auf den Kontext an. Enumerationen in neueren Programmiersprachen sind so gesehen auch nichts anderes, als eine Sammlung global verfügbarer Konstanten.


edit: nochmal zum Vergleich oben:

Von allem mal abgesehen ist dein (a >= b) == false (wie das aussieht, das tut doch weh ...) nichts anderes als (a < b), da brauchst du nirgens einen Negationsoperator.

 

[Yuuri]

öhm ... man kann es einfacher und schneller lesen, als dieses Ausrufezeichen davor, welches einen impliziten Vergleich darstellt und auch schnell mal übersehen wird, wenn man viel Quelltext überfliegt.

nicht( a >= b ) ist schlechter lesbar als (a >= b) = falsch? Sorry, beim Zweiten müsste ich zwei mal drüber gucken um den Sinn zu verstehen, beim Ersten seh ich sofort was der Ausdruck bedeutet.

 

[Blackbenji]

@voodoo44: ich bin jederzeit dankbar für hilfreiche antworten und nehme auch gerne verbesserungsvorschläge war. doch möchte ich dich bitten meine fragen bezüglich php nicht mit meiner berufsberzeichnung in verbindung zu stellen. als application administrator betreue ich anwendungen, ich programmiere sie nicht. die fragen die ich hier stelle in bezug auf php sind lediglich meine privaten projekte, aus denen ich lernen möchte.

in bezug auf das nutzen meiner "GLOBALS" hätte ich gerne eine alternative lösung - wenn es, wie du sagst, zu vermeiden gilt.

danke!

 

[voodoo44]

Du gehst imho davon aus, dass Konstanten stets global verfügbar sind, das ist aber nicht der Sinn einer Konstante, auch wenn Konstanten gerne hierfür benutzt werden.

Naja, so lange sie 'GLOBAL_' heißen und tatsächlich IRGENDWO definiert werden, gehe ich davon aus. Natürlich benutze ich auch Konstanten - die aber Klassenintern. Mir ging es jetzt um die Art und Weise, wie er sie verwendet hat. Dass man Konstanten innerhalb von Klassen verwenden kann, ist klar - aber dann arbeitet man meist auch mit einem Scope Resolution Operator.

Von allem mal abgesehen ist dein (a >= b) == false (wie das aussieht, das tut doch weh ...) nichts anderes als (a < b), da brauchst du nirgens einen Negationsoperator.

Mir ging es jetzt primär um das 'außenrum' - nicht um das, was IN der Klammer steht. Das hätte man (wie du es bereits getan hast) auch 'andersrum' schreiben können, indem man ein < setzt. War mein Fehler, muss ich zugeben. Da muss ich mich demnächst etwas drum kümmern, die 'unschärfe' aus meinen Beiträgen zu entfernen. Noch ein Vorsatz für 2013.....

Noch kurz zum "i == 7 == false" - nein, das schreibe ich in der Tat nicht. Bei mir wäre das ein i !== 7

@Blackbenji
Hab ich doch: setz es im Konstruktor der Klasse, setz es über einen Setter, mach's als Klassenproperty, schieb ein fertiges Datenzugriffsobjekt rein - Hauptsache keine globale Konstanten.

 

[Blackbenji]

@voodoo44:

wäre es so besser?

class Test {
    
    const db_name = GLOBAL_LOGGING_DB;

    public function db_test() {
        return self::db_name;
    }

}
$test = new Test();
echo $test->db_test();

 

[voodoo44]

Besser als vorher.

Noch besser wäre aber sowas:

class Test {
    
    protected $_dbName;
    protected $_db;

    public function __construct($dbName) {

        $this->_dbName = $dbName;
        $this->_setupDb();
    }

    protected function setupDb() {
        $this->_db = new mysqli('localhost', 'root', '', $this->_dbName);
    }
 
    public function getDb() {
        return $this->_db;
    }
 
}

$test = new Test(GLOBAL_LOGGING_DB);
$db = $test->getDb();
echo $db->query('SELECT * FROM HUSARE');

=> Namen von AUSSEN reingeben, nicht innendrin irgendwo hart setzen. Wenn du in der index.php irgendwelche Konstanten setzt mag das gehen, wenn du diese in den Rest der Klassen irgendwo reingibst. Wenn du sie IN den Klassen hart ausliest, hast du ein Problem - weil du uU irgendwann nicht mehr nachvollziehen kannst, welche Konstanten du brauchst, wo sie herkommen, wie sie sich zusammensetzen etc.

 

[Blackbenji]

danke für die info.
ich finde es nur ziemlich unübersichtlich in der form die klassen anzumelden, wenn ich dort auch noch die gloabals mit ranhänge - gerade wenn es eine klasse ist die mehreren sq-tabellen nutzt, und so schon das ein oder andere übergeben bekommt.

 

[voodoo44]

Dann bau einen Setter für die Tabellen und mach sowas wie

$test->addTable('bla1')->addTable('bla2')->addTable('bla3');

Zumindest, wenn ich deine Anforderung gerade richtig interpretiere.

Ansonsten seh ich gerade das:

if (mysql_num_rows( $result ) == 1) {
    $benutzerdaten = mysql_fetch_array( $result );
    // Resourcen freigeben
    mysql_free_result( $result );
    // Daten aus der DB mit den Benutzerdaten vergleichen
    if ($benutzerdaten['ip'] != $_SERVER['REMOTE_ADDR']) { $this->resetUser(); }
    if ($benutzerdaten['userinfo'] != $_SERVER['HTTP_USER_AGENT']) { $this->resetUser(); }
    if ($benutzerdaten['login'] != $_SESSION['login']) { $this->resetUser(); }
    if (!(($benutzerdaten['lastlogin']+900) >= $_SERVER['REQUEST_TIME'])) { $this->resetUser(); }
} else {
    $this->resetUser();
}

würde ich so machen:

public function compareUserdata(array $benutzerdaten) {

    $bIsSame = true;

    if ($benutzerdaten['ip'] != $_SERVER['REMOTE_ADDR']
        OR $benutzerdaten['userinfo'] != $_SERVER['HTTP_USER_AGENT']
        OR $benutzerdaten['login'] != $_SESSION['login']
        OR ($benutzerdaten['lastlogin'] + 900) < $_SERVER['REQUEST_TIME']
    ) {
        $bIsSame = false;
    }

    return $bIsSame;
}

[...]

if (mysql_num_rows($result) == 1) {
    $benutzerdaten = mysql_fetch_array($result);
    // Resourcen freigeben
    mysql_free_result($result);

    // Daten aus der DB mit den Benutzerdaten vergleichen
    if ($this->compareUserdata($benutzerdaten) === false) {
        $this->resetUser();
    }
} else {
    $this->resetUser();
}

Kurzum: kleine, sinnvolle 'Arbeitsgrüppchen' bilden => fördert Übersicht und ggf. Wiederverwendbarkeit.
Abhängigkeiten nach außen sind immer schlecht. Auch wenn wir extrem OT sind mittlerweile