Coders Crap :D

bofh1337

bofh1337

Erfahrenes Mitglied
Ich habe jetzt 3 Tage nach einem Fehler gesucht, habe mir die unten stehende Funktion 100000 mal angesehen, aber keinen Fehler erkannt:

PHP: 
    public static function getDatabase ($params = array ()) {

        $signature = md5 (serialize ($params));

        if (empty (self::$database[$signature])) {
            self::$database[$signature][] = self::createDatabase ($params);
        }
        return self::$database[$signature];
    }
Richtig wäre:
PHP: 
    public static function getDatabase ($params = array ()) {

        $signature = md5 (serialize ($params));

        if (empty (self::$database[$signature])) {
             self::$database[$signature] = self::createDatabase ($params);
        }
        return self::$database[$signature];
    }

Ein kleiner aber feiner Fehler beim Serialisieren der Instance, das Database-Object "MySql" war zwar vorhanden, ich konnte aber weder aus der "Database"-Klasse, noch von auserhalb auf die Methoden zurückgreifen :D

MAAAAAAAAAAAAAAAAAAAAAAAAAN, 3 Tage meines Lebens für so sowas :D
 
Mitleid hab ich keins, weil ich da MD5 lese :rolleyes:
...bis die MD-Seuche ausgerottet ist vergehen wohl noch hundert Jahre...
 
@sheel: Naja, in diesem Fall ist md5 wohl vertretbar. Warum würdest du hier eine andere, vermutlich langsmere Hash-Funktion vorziehen? Es geht ja nicht um die Sicherung sensibler Daten, sondern um die erzeugung eines eindeutigen "fingerprints" (gleiche $params = gleiher Hash)

@bofh: Mir ist in dem Code ganz schön viel static ....
 
@alxy: Und für einen eindeutigen DB-Key ist ein Hash ohne weitere Prüfung generell,
abhängig von der Größe des serialisierten Arrays, sowieso fragwürdig.
Probleme werden "nicht oft" vorkommen, aber wenn...
 
Die Funktionen sind deshalb statisch, weil es ein Factory-Pattern ist, das "$params" ist ein Array, was die kompletten DB-Logindaten enthält, also Datenbank-Host, Datenbank-User, User-Password, Datenbank-Name, Driver (mysql, mysqli usw.) und Datenbank-Prefix...sind die beim 2. Aufrufen gleich, kann man wohl von ausgehen, das die gleiche Instance zurückgegeben werden kann ;)

Trotzdem werde ich die Prüfung auf Existenz der Keys und deren Inhalt verlegen und vor dem serialisieren machen, sollte "host" nämlich nicht angegeben werden, wird default "localhost" genommen, da könnten dann schon 2 Instanzen erstellt werden.

Mir ist schon klar, das die Zeiten von MD5 vorbei sind, besonders, wenn es um Passwörter usw. geht, aber bei mir werden die Passwörter ja auch als sha512 + Salt gehasht ;)
 
Mich stört eher das PHP keine typisierte Prüfung vorgenommen hat.
Sprich das mehrdimensionale Array nicht angemeckert hat, da scheinbar eindimensional.

In C# wurde sowas ähnliches mittels var/dynamic ja auch schon eingeführt und einige meiner Kollegen verwendens auch, find ich persönlich ***********.

Aber lass ma hatte letzten ein Stückcode geändert und gerade da es eine blöde Stelle war hab ich das vorher 10 mal angeschaut.
Was soll ich sagen its not a bug, its a feature ^^
Zum Glück wurds schnell bemerkt.
 
bofh1337 hat gesagt.:
wenn es um Passwörter usw. geht, aber bei mir werden die Passwörter ja auch als sha512 + Salt gehasht ;)
Zum Vergrößern anklicken....
Salt: gut (auf einer per-User Basis!)
SHA-512: nicht für Passwörter geeignet!*
Nutze eine langsamere Funktion, die auch für's Password-Hashen gedacht ist: bcrypt beispielsweise.
Weitere Informationen (u. a.): bcrypt vs PBKDF2

Aber den Fehler, dass ich [] vergessen habe, ist mir auch schon mal passiert! Was hilft? Unit Testing! Ein Unit Test, der die Rückgabe deiner Funktion testen würde, hätte sofort die Alarmglocken gemeldet, dass die Struktur nicht gleich der erwarteten Struktur ist.


* Ich würde deine DB nicht als völlig unsicher bezeichnen, aber du solltest schon zügig umsteigen. Am besten die Passwortfunktionen von PHP benutzen: [phpf]password_hash[/phpf] und Verwandte.
 
SHA512 ist nicht für Passwörter geeignet? Ich habe (bevor ich das alles eingebaut habe) natürlich einen "härtetest" gemacht und fast 2,5 Mio. Hashes am Stück in die Datenbank eintragen lassen. Irgendwie habe ich ein komisches Gefühl, wenn ich mir die Funktionsbeschreibung so ansehe, da wird auf das (eigentlich) schwache crypt() gesetzt. Im schlimmsten Fall wird daraus ein simpler MD5-Algo,- da bleibe ich doch lieber bei sha512 ;)
 
Um antworten zu können musst du eingeloggt sein.

Neue Beiträge

Zurück