Pfusch bei relativen Dateipfaden, die 100ste...



  • Hallo zusammen,

    dieser Post richtet sich vor allem an die @ChurchTools-Mitarbeiter, aber wer sich ein wenig mit PHP auskennt und seinen Senf dazugeben möchte, kann mich natürlich gerne korrigieren, kritisieren, etc.
    Ich werde versuchen, meine Kritik so sachlich und konstruktiv wie möglich anzubringen, auch wenn mich allein die Tatsache, dass ich das jetzt schon wieder monieren muss, mittlerweile ordentlich frustriert/in Unmut versetzt.

    Vor wenigen Tagen ereilte mich ein Hilferuf aus unserer Gemeinde, dass ChurchTools außer Rand und Band ist, und laufend irgendwelche Errors auswirft, die ein normales Arbeiten praktisch unmöglich machten.
    Nach dem Login hat es auch nicht sonderlich lange gedauert, bis ich den ersten Fehler gesehen habe und ich wusste sofort: Irgendwie kommt mir der bekannt vor...

    Error-Pic

    Schnell hat sich herausgestellt, dass es mal wieder Fehlermeldungen sind, die das Parsing der JSON-Responses vom Server versauen. Eigentlich sollte sowas im ausgelieferten Code eines Produktivsystems immer unterdrückt werden, besonders wenn es nur eine Warnung und kein harter Fehler ist. So weit, so nervig.

    Da stellt sich mir natürlich die Frage: Was verursacht diese Meldung denn eigentlich?
    Die Fehlermeldung stieß mich mit der Nase direkt auf Zeile 1122 in der system/includes/functions.php:
    file_put_contents($absoluteFilesDir . '/log/error.log', $txt2, FILE_APPEND);
    Aussage der Fehlermeldung: Unsere error.log ist nicht beschreibbar.
    Nach etwas Verwunderung und einem kurzen Check stelle ich fest: Die Datei ist sehr wohl beschreibbar, also woher kommt dieser schräge Fehler? Los geht die Schnitzeljagd durch den PHP-Code. Zunächst stellt man wenigen Zeilen darüber fest, dass offensichtlich eine Variable aus dem globalen Scope ihre Finger im Spiel hat:
    global $files_dir;
    $absoluteFilesDir = realpath($files_dir);
    Hm, schonmal eine vollkommen sinnlose Anweisung, denn wenn der Pfad von realpath() korrekt aufgelöst wird, kann man ihn auch direkt in Zeile 1122 verwenden, es existiert kein semantischer Unterschied.
    Und weiter geht's zu Zeile 413 in der system/includes/start.php, wo die Variable mit ihrem default-Value besetzt werden sollte:
    $files_dir = DEFAULT_SITE;

    An dieser Stelle kommt jetzt der kleine Schönheitsfehler:
    Da wir an dieser Stelle einen AJAX-Aufruf haben, wird diese Zeile niemals ausgeführt, und der Pfad ist Grütze, das führt dann zu einem Fehler, der wegen falscher PHP-Konfig auch noch ausgegeben wird, und das wiederum versaut mir meinen Tag.

    Lessons learnt:

    • In produktiv eingesetztem Code nicht error_reporting(0); aufzurufen - Anfängerfehler. (Das Gegenteil gilt für die Testsysteme, dort sollte man error_reporting(E_ALL); benutzen.)
    • realpath() ist eine Funktion zur Auflösung relativer Pfade. Es ist keine Kläranlage und auch keine Zauberkiste, die aus irgendwelchem Bull**** sinnvolle Werte herbeizaubern kann. Wenn eine PHP-Funktion aufgerufen werden soll, ist ihre Verwendung im Allgemeinen völlig überflüssig, denn es ändert für PHP nichts an der Pfad-Semantik.
    • Wichtige Variablen an einer Stelle zu initialisieren, die nicht jeder relevante Code-Pfad durchläuft - keine gute Idee.

    Provisorischer Patch und weitere Anregungen folgen.


  • ChurchTools Mitarbeiter

    Hallo @milux, ich versuche mal ein paar Dinge hier aufzuklären.
    Vielleicht kommen wir dann der Lösung des Problems näher.

    Der Fehler den du im Frontend siehst hat vermutlich nichts damit zu tun, dass das error log in eurer ChurchTools Installation nicht geschrieben werden konnte. Dieser ist ein Folgefehler davon weil er versucht den eigentlichen Fehler in euer Error Log zu schreiben, wobei er scheinbar scheitert.

    Du könntest mal im Apache/nginx Error Log nachschauen ob du dort den eigentlichen Fehler finden kannst.

    global $files_dir;
    $absoluteFilesDir = realpath($files_dir);
    Hm, schonmal eine vollkommen sinnlose Anweisung, denn wenn der Pfad von realpath() korrekt aufgelöst wird, kann man ihn auch direkt in Zeile 1122 verwenden, es existiert kein semantischer Unterschied.

    Die Funktion realpath macht aus einem relativen Pfad einen absoluten. In den meisten Fällen macht das keinen Unterschied. Es gibt allerdings Php Installationen die nur mit einem Absoluten Pfad zurecht kommen.

    Und weiter geht's zu Zeile 413 in der system/includes/start.php, wo die Variable mit ihrem default-Value besetzt werden sollte:
    $files_dir = DEFAULT_SITE;
    An dieser Stelle kommt jetzt der kleine Schönheitsfehler:
    Da wir an dieser Stelle einen AJAX-Aufruf haben, wird diese Zeile niemals ausgeführt, und der Pfad ist Grütze, das führt dann zu einem Fehler, der wegen falscher PHP-Konfig auch noch ausgegeben wird, und das wiederum versaut mir meinen Tag.

    Die Funktion churchtools_app in der die von dir erwähnte Zeile vorhanden ist wird auch bei Ajax-Requests aufgerufen. Daran sollte es nicht liegen. Wie lautet denn der Pfad bei euch an dieser Stelle? Ist er sicher nicht richtig gesetzt?

    Was ich mir vorstellen kann ist, dass der Fehler auftritt bevor diese Funktion aufgerufen wird.



  • Erstmal ein bisschen Lob:

    Euer Patch hat den Fehler behoben, und das, wie ich finde, recht gut!
    Es war in der Tat das falsche Working Directory, dass den Fehler verursacht hat. Im Shutdown-Handler können die nämlich scheinbar abweichen (https://secure.php.net/manual/de/function.register-shutdown-function.php#92657), gut gemerkt!
    Deshalb dachte ich auch, dass die Variable nicht gesetzt sei. In Wirklichkeit konnte realpath den Pfad wegen dem falschen W.D. aber nicht auflösen und hat deswegen false zurückgegeben, was bei Konkatenation in einen leeren String konvertiert wird - daher meine falsche Vermutung. (Habe ich schonmal erwähnt, wie ich die ultra-schwache Typisierung von PHP hasse?)

    Allerdings hätte man das noch unkomplizierter machen können, indem man die magische __DIR__ Konstante verwendet, und damit einen absoluten "Root Path" in der constants.php definiert, und zwar so:
    define('ROOT_PATH', realpath(__DIR__ . '/../../'));
    Diese Technik sollte maximal robust sein, auch wenn das Working Directory aus welchen Gründen auch immer falsch gesetzt ist.

    Zum Thema:

    Hallo @milux, ich versuche mal ein paar Dinge hier aufzuklären.
    Vielleicht kommen wir dann der Lösung des Problems näher.

    Der Fehler den du im Frontend siehst hat vermutlich nichts damit zu tun, dass das error log in eurer ChurchTools Installation nicht geschrieben werden konnte. Dieser ist ein Folgefehler davon weil er versucht den eigentlichen Fehler in euer Error Log zu schreiben, wobei er scheinbar scheitert.

    Wie du schon sehr richtig bemerkt hast - es handelt sich hier in der Tat um einen "Folgefehler". Allerdings sollte selbiger niemals vom Server ausgegeben werden, wenn man sich an best practices aka error_reporting(0) hält.

    Du könntest mal im Apache/nginx Error Log nachschauen ob du dort den eigentlichen Fehler finden kannst.

    Das ist gar nicht nötig, denn der Fehler wird auch in das DB-Log von ChurchTools protokolliert:
    Error in Shutdown: [ERROR] file:Unknown:0
    PHP Startup: Unable to load dynamic library '/usr/lib/php7/20160303/oauth.so' - /usr/lib/php7/20160303/oauth.so: cannot open shared object file: No such file or directory

    Es handelt sich um eine unkritische Warnung, weil ALL-INKL scheinbar ein Modul in der php.ini falsch verlinkt hat.
    Aber genau diese Warnung stieß unter bestimmten Umständen, die ich meinte verstanden zu haben die Fehlermeldung an.

    global $files_dir;
    $absoluteFilesDir = realpath($files_dir);
    Hm, schonmal eine vollkommen sinnlose Anweisung, denn wenn der Pfad von realpath() korrekt aufgelöst wird, kann man ihn auch direkt in Zeile 1122 verwenden, es existiert kein semantischer Unterschied.

    Die Funktion realpath macht aus einem relativen Pfad einen absoluten. In den meisten Fällen macht das keinen Unterschied. Es gibt allerdings Php Installationen die nur mit einem Absoluten Pfad zurecht kommen.

    Korrekt, und der Pfad ist dann nicht nur absolut, sondern auch eindeutig. (Letzteres ist in ersterem nicht eingeschlossen!)
    Allerdings würde mich interessieren, bei welchen Installationen ohne diese Funktion ein Fehler ausgelöst wird. Wie ich bereits sagte, sollten relative Pfade nach Bedarf automatisch von PHP aufgelöst werden, was die Instruktion überflüssig macht.

    Und weiter geht's zu Zeile 413 in der system/includes/start.php, wo die Variable mit ihrem default-Value besetzt werden sollte:
    $files_dir = DEFAULT_SITE;
    An dieser Stelle kommt jetzt der kleine Schönheitsfehler:
    Da wir an dieser Stelle einen AJAX-Aufruf haben, wird diese Zeile niemals ausgeführt, und der Pfad ist Grütze, das führt dann zu einem Fehler, der wegen falscher PHP-Konfig auch noch ausgegeben wird, und das wiederum versaut mir meinen Tag.

    Die Funktion churchtools_app in der die von dir erwähnte Zeile vorhanden ist wird auch bei Ajax-Requests aufgerufen. Daran sollte es nicht liegen. Wie lautet denn der Pfad bei euch an dieser Stelle? Ist er sicher nicht richtig gesetzt?

    Ja, ist er. Es handelt sich um den Standard-Pfad, keine Abweichungen von der 08/15-Konfiguration.
    Meine (falsche) Annahme rührte von den letzten Zeilen der Funktion churchtools_app her. Ich habe erst jetzt bemerkt, dass diese Zeilen mittels die() in der Funktion churchtools_processRequest "umschifft" werden. Auch wieder so eine Sache, die ich vom Code-Design her ein wenig unglücklich finde.

    Was ich mir vorstellen kann ist, dass der Fehler auftritt bevor diese Funktion aufgerufen wird.

    Das dachte ich mir ursprünglich ebenfalls.
    Allerdings habe ich mir dann zwecks Debugging an der Stelle den Stacktrace mittels debug_backtrace geholt, und der enthielt die Funktion handleShutdown. Das bedeutet, dass der Fehler ganz am Ende auftrat, wo die Variable durchaus bekannt sein sollte. Das Problem war die Auflösung mittels realpath bei falschem Working Directory.

    tl;dr

    Bitte zieht trotz dem Patch in Erwägung:

    • Sofort beim Einstiegspunkt (index.php) error_reporting(0) zu setzen, und bei Debugging error_reporting(E_ALL)
      Ohne das wird es höchstwahrscheinlich irgendwann wieder dazu kommen, dass eine Fehlermeldung hochkommt, die man eigentlich hätte unterdrücken sollen. Ich empfehle es wirklich dringendst!
    • Absolute Pfade zu benutzen, statt darauf zu setzen, dass das Working Directory durch das PHP-System immer korrekt gesetzt wird. __DIR__ FTW!

Anmelden zum Antworten
 

Es scheint als hättest du die Verbindung zu ChurchTools Forum verloren, bitte warte während wir versuchen sie wieder aufzubauen.