Hi,
war eine interessante Disskusion, die ich bei euch mitlesen konnte. Man denkt einfach mal wieder über das eine oder andere nach, wenn manche über Optimierungen sprechen.
Einen hätte ich auch dazu, obwohl das hier wohl eher in den Bereich der Theorie geht, da der zu erzielende Gewinn nicht mehr wirklich spürbar ist. PHP hat einen grösseren Aufwand beim konkatenieren von Strings, wenn diese in doppelten Anführungsstrichen stehen als wenn sie in einfachen enthalten sind.
Also bringt
PHP:
if(file_exists('./' .$dir. '/' .$page. '.php')) { include './' .$dir. '/' .$page. '.php'; }
}else{
if(file_exists('./inhalt/fehler.php')) { include './inhalt/fehler.php'; }
}
noch einen kleinen Vorteil.
Nun aber zur Frage, wie man mit den verschiedenen Verzeichnissen, in denen sich die Dateien befinden können, umgehen kann.
Hier gibt es sicher auch wieder verschiedenen Möglichkeiten. Eine davon wäre natürlich mit der Switch-Anweisung zu arbeiten und dort in jedem Zweig den passenden Pfad einzusetzen. Bei den vorangegangen Überlegungen zur Optimierung ist es aber wohl eine gute Idee gewesen, sie wegzulassen und die Array-Idee beizubehalten. Eine Switch-Anweisung ist ja für PHP auch eine Belastung, die mit der in_array() - Funktion nicht vergleichbar ist. Sie könnte sich allerdings wieder lohnen, wenn in einigen Fällen noch mehr Verarbeitung nötig ist, die z.B. in der inkludierten Datei nicht stattfinden kann, vielleicht um weitere Dateien zu inkludieren oder so.
Die Idee, die ich dir für den bisher fertigen Code empfehlen würde, wäre das Verzeichnis ebenfalls in deinem Array aufzunehmen und dann wieder daraus zu entnehmen. Dabei können auch Unterverzeichnisse beachtet werden. Du müsstest genau drei Änderungen vornehmen: (1) Das Array in ein mehrdimensionales Array ändern, (2) die Prüfung, ob ein Element im Array enthalten ist anpassen und (3) die Information nutzen.
PHP:
$whitelist = array("news"=>"dir1", "archiv"=>"dir2/subdir", "search"=>"dir1", "guestbook"=>"dir3", "inhalt"=>"dir2");
if (!empty($page) && isset(array($whitelist[$page]))) {
$dir = $whitelist[$page];
if(file_exists("./" .$dir. "/" .$page. ".php")) { include "./" .$dir. "/" .$page. ".php"; }
}else{
if(file_exists("./inhalt/fehler.php")) { include "./inhalt/fehler.php"; }
}
}
Das optimiere ich nochmal ein wenig:
PHP:
$whitelist = array("news"=>"dir1", "archiv"=>"dir2/subdir", "search"=>"dir1", "guestbook"=>"dir3", "inhalt"=>"dir2");
if (!empty($page) && isset(array($whitelist[$page]))) {
$file = '/'.$whitelist[$page].'/'.$page.'.php';
if(file_exists($file)) { include $file; }
}else{
if(file_exists("./inhalt/fehler.php")) { include "./inhalt/fehler.php"; }
}
}
Der Vorteil dieser Änderung ist nun noch, dass die Variable $dir entfällt. Somit sparen wir uns und vor allem dem Interpreter die Zuordnung des Verzeichnisses zur Variablen, da wir den Wert schon haben. Allerdings ist der Zugriff auf ein Array-Element wieder aufwendiger als auf einen einzelnen String-Wert. Das ist aber hier in Ordnung, da wir über die neue Variable $file das Problem sofort wieder beseitigen und gleich nochmal optimieren. Durch die Zuweisung an die Variable $file kann ab sofort $file verwendet werden. So findet der Zugriff auf das Array-Element in diesem Fall nur einmal statt und nicht, wie in meinem ersten Beispiel zweimal. Und da wir schon einmal dabei sind, stellen wir auch gleich fest, dass der Pfad zur zu inkludierenden Datei nur einmal zusammengestellt werden muss und dann aber zweimal verwendet werden kann. Auch das Zusammenstellen eines Strings bedeutet einen Aufwand, den wir so reduzieren.
Nun vielleicht noch meine Meinung zur Thematik Sicherheit der Parameter. Ich denke ebenfalls, dass über den inzwischen zusammengetragenen Code eine ziemliche Sicherheit entstanden ist. Hier kann sicher nicht mehr viel schiefgehen. Die Frage ist eher, ob die Parameter-Übergabe für dich noch ein Sicherheitsproblem darstellt. Ein Benutzer deiner Seite könnte nachdem er die URL in der Adresszeile gesehen hat, den Parameter manuell bearbeiten und damit eine Datei anfordern, z.B. impressum, die du eventuell an dieser Stelle gar nicht verlinkt hast und vielleicht zur Zeit nicht anbieten willst. Natürlich muss er entweder dafür wissen, welche Seiten du noch hast oder sie erraten. Das kann manchmal recht einfach sein, aber meistens ist es doch aufwendiger für den Benutzer, da er viel herumprobieren müsste. Das könntest du natürlich erschweren, in dem du nicht die Namen deiner Dateien als Parameter nimmst, sondern diese etwas verschlüsselst.
Da könntest du zum Beispiel page=hdnehsfhooz statt page=news verwenden. Diese Buchstabenkombination ist zufällig gewählt, aber erraten werden können diese nur sehr schwer. Ärgerlich ist dann allerdings, dass du selbst eigentlich nicht mehr weist, was in der Datei enthalten ist, wenn sie nicht mehr news.php sondern hdnehsfhooz.php heist.
Dann wäre die andere Idee, dass du die Parameter mit base64 kodierst. Ein häufiges Mittel übrigens. Das kannst du sogar auch mit den Namen der Parameter tun. Die PHP-Funktionen base64_encode() und base64_decode() helfen dabei.
Es gibt noch eine Variante, die ich sehr witzig finde und auf die man mittels raten auch nicht so leicht kommt:
PHP:
$mapping_keys = array("katze"=>"news", "haus"=>"archiv", "eierkuchen"=>"search", "motorblock"=>"guestbook", "news"=>"inhalt");
if(isset($_GET["page"])) {
$page = $mapping_keys[$_GET["page"]]
...
Hier werden als Parameter Werte übertragen, die offenbar nicht sinnvoll sind und aus denen man keinen Rückschluss auf vorhandene Dateien ziehen kann. Diese Begriffe werden anschliessend in die passenden echten Werte mittels des Hilfsarrays mapping_keys gemappt.
Ideallerweise kann man auch mehrere Vorgehensweisen kombinieren.
Die Frage dabei ist natürlich, wieviel Sicherheit du tatsächlich benötigst und ob du dir damit tatsächlich weiterhilfst oder dich doch eher nur selbst beschäftigst, da die Manipulation der Parameter für dich nicht so entscheidend ist. Du bietest ja eh' schon nur Seiten an, die in deiner Whitelist enthalten sind. Bei vorübergehend nicht angebotenen Seiten müsstest du eben deine Whitelist entsprechend anpassen oder wenn diese Seiten "geheim" sein sollen, dann lohnt sich eine Berechtigungsprüfung für den Aufruf der Seite durch den Benutzer durch eine zuvor erfolgte Authentifizierung, die in der inkludierten Datei verwendet wird. Aber prinzipiell können solche Mechanismen natürlich für den Schutz deiner Parameter eingesetzt werden, auch wenn es sich um andere Zwecke als die Auswahl einer Datei handelt.
Besser noch ist es natürlich, wenn du die Parameter gar nicht als GET-Parameter über die URL überträgst, sondern lieber Formulare statt Links einsetzt und die Parameter dann als POST-Parameter empfängst ($_POST).
Jetzt bin ich gleich fertig. (Ja, ihr habt es gleich geschafft!)
Mir ist noch eine Kleinigkeit aufgefallen, die sich auf einen leicht überflüssigen Codeteil bezieht.
PHP:
if(!isset($_GET["page"])) {
$page = "news";
} else {
if(isset($_GET["page"])) {
$page = preg_replace( "/[^a-z0-9]/im", "", $_GET["page"]);
}
}
Was passiert hier? Erst wird geprüft, ob der Parameter existiert. Ist das nicht der Fall gibt es eine Verarbeitung. Im anderen Fall, also wenn es den Parameter doch gibt, wird die Verarbeitung im else-Block durchgeführt. Aber worin besteht die? Nun zuerst wird geprüft, ob der Parameter existiert. Schon wieder? Wir wissen doch schon, dass er existiert. Deswegen sind wir doch überhaupt erst in den else-Zweig geraten. Also können wir diese Abfrage ohne Bedenken streichen. So geht's auch:
PHP:
if(!isset($_GET["page"])) {
$page = "news";
} else {
$page = preg_replace( "/[^a-z0-9]/im", "", $_GET["page"]);
}
Ich hoffe meine Anmerkungen waren interessant, verständlich und am Ende auch noch etwas hilfreich.
Mit Grüssen
Mitreder.