Das Skript zum Montag - Blocklist.de befragen

[Jörg Reinholz]

Moin!

Ich kenne den Dienst nicht, aber es gibt scheinbar auch eine bereits eine HTTP-API dafür. Mir ist nicht so recht klar, welchen Vorteil dein Skript demgegenüber bietet.

Dafür wollen die einen Key (Dann kann man aber auch Angreifer dort einwerfen). Für die DNS-Abfrage nicht ...

Ein paar stilistische Anmerkungen zu deinem Skript hätte ich auch noch:

ini_set('display_errors', 0);

Wieso überschreibst du diese Server-Einstellung? Auf Produktiv-Servern kann man damit rechnen, dass diese Eigenschaft bereits deaktivert ist. Auf Entwicklungs-Servern möchte man Fehler bewusst anzeigen lassen, da erschwert diese Überschreibung nur die Fehlersuche.

Volle Absicht. Macht man das nicht werden die Notizen in der Ausgabe ausgeworfen und der HTTP-Status auf 200 gesetzt. Das genau will ich hier nicht.

Ich will (wenn man es denn als Web-Api nutzt:

• außer bei einem Konfiguarationsfehler JSON also Ausgabe
• Fehlercodes (404) im HTTP-Header
• und die Fehler also nur im Error-Log

if ( empty($_REQUEST['method'])) {
    $_REQUEST['method'] = 'HOST';
}

$_REQUEST ist nicht der Ort um benutzerdefinierte Werte darin zu speichern.

Du hast nicht erkannt, dass das eigentlich zum Test gehört. Wer das produktiv nutzt sollte natürlich den Testbereich rauswerfen und wird sich wohl je nach Nutzung auf eine andere Methode der Übergabe bzw. Fixierung festlegen.

if (
    $_REQUEST['method'] != 'PHP'
    && $_REQUEST['method'] != 'NSLOOKUP'
    && $_REQUEST['method'] != 'HOST'
    ) {
    $_REQUEST['method'] = 'HOST';
}

Die Stelle musste ich 3 Mal lesen, um die Klammernpaare einander zuzuordnen.

Hätte nicht gedacht, dass das schwer zu lesen ist. Ich habe die 3 Bedingungen auf 3 Zeilen verteilt, damit der Umbruch nicht durch die Darstellung hier im Forum an willkürlicher Stelle erfolgt - was den Code noch viel schlechter lesbar macht.

In meinem Styleguide steht: "Brich die Bedingungen um und rücke diese bei Klammerung genau sei ein wie Du es bei Codeblöcken tun würdest." Ist prima lesbar!

Ohnehin gilt: wenn jemand das produktiv nutzen will, dann sollte er sich auf eine Abfragemethode festlegen, den Test rauswerfen und kann sich so mehr als die Hälfte der Codezeilen sparen.

Deshalb steht da auch "Legt man sich auf eine Methode fest und entfernt den "Selbsttest", dann wird das Skript natürlich kürzer."

Ohne deklaratorische Zeilen wie das letzte exit vor der Funktion kommt das Skript nach dem Rauswerfen nicht benötigter Funktionen auf 60 Zeilen, ohne Leerzeilen sind es 51...

if ( $return['error_type'] == 'FATAL' ) {
    header('Status: 500 ');
    echo $return['error_string'];
    exit;
}

exit; macht es ebenfalls schwer die Struktur von Quelltexten zu erfassen.

Ich komme damit gut klar. trigger_error ("Foo, blah, blubb", USER_ERROR) bricht auch ab und ich muss es bis nach ganz rechts lesen um das zu wissen. Mir signalisiert das exit deutlich genug, was da los ist und ich empfinde es eher als Zumutung, wenn ich womöglich erst 3000 Zeilen und 24 Dutzend elseif weiter unten erst sehe: da kommt nichts mehr. Abbruchbedingung erfüllt? Exit! ist für mich eine klare und lesbare Ansage. Ich schätze, der Interpreter sieht das auch so...

    $return['Hint']             = 'Dies ist ein Test! Um eine stabile API zu erhalten fragen Sie beim Autor nach (www.fastix.org)';

Diser Kommentar verunsichert nur. Entweder würde ich hier direkt dokumentieren ...

Was bedeutet wohl, dass das Skript im Ordner /test/ ist? Könnte beides zusammen die sehr deutliche Nachricht beinhalten, dass ich so klar wie möglich machen will, dass niemand darauf vertrauen soll, dass das Skript auch übermorgen noch dort ist und so wie bisher funktioniert? Dieser Kommentar soll also gerade klar machen, dass es nicht sicher ist, das Skript zu benutzen. Zur Datensicherheit gehört die Verfügbarkeit. Hinsichtlich einer API-Sicherheit gilt das auch.

if ( 'NSLOOKUP' == strtoupper($method) && ! is_file(nslookup_exe) ) {

Du hast vier Codeblöcke dieser Art. Nur der erste Codeblock behandelt dabei einen Fehler, der durch eine Fehlbedienung der Funktion ausgelöst wird.

Fehlbedienung? Eigentlich Fehlkonfiguration. Fehlbedienung war weiter oben (falsche Methode im Request). Im übrigen unterscheiden sich diese Codeblöcke doch etwas zu sehr als dass ich hier von einer "wiederverwendbaren eigenen Einheit" ausgehen würde.

Die anderen Fehlermeldungen beziehen sich auf die Ausführungsumgebung: Existiert dieses Programm?

Ist hier auch drin.

Du hast dir sicher etwas dabei gedacht, diese und andere Variablen groß zu schreiben.

Der ursprüngliche Plan sah Konstanten vor. Davon bin ich abgerückt, weil PATTEN_SONSTWAS irgendwie zu allgemein ist. Wem das nicht passt, der kann mit "Suchen und Ersetzen" daraus machen was in seinem Styleguide steht.

Jörg Reinholz