Warum sind im Magento ECG Coding Standard so viele PHP-Funktionen nicht erlaubt?

30

Der Magento-EKG-Kodierungsstandard scheint als Standard für Magento 1-Erweiterungen (zumindest in gewisser Weise) offiziell zu sein:

https://github.com/magento-ecg/coding-standard

Aber ich verstehe nicht, was hinter allen Regeln steckt, und die Code-Sniffer-Regeln mit ihren Nachrichten allein helfen nicht viel. Gibt es eine detaillierte Dokumentation zum Standard? Ich kenne die gängigen Best Practices und das Entwicklerhandbuch , kann jedoch keine spezifischen Informationen zu diesen Codierungsstandards finden.

Was mich am meisten stört, ist die Strenge, keine PHP-Funktionen zu verwenden.

Zum Beispiel: Warum ist jede einzelne dateisystembezogene PHP-Funktion verboten ?

Ich schätze, Sie sollten verwenden Varien_Io_File, Varien_File_Objectusw., aber selbst die Kernentwickler kennen nicht alle Varien-Klassen, und Sie finden häufig Dinge in Mage_ImportExport_Model_Import_Adapter_Csv:

    $this->_fileHandler = fopen($this->_source, 'r');

Der Kern ist also nicht so oft das beste Beispiel.

Andere meiner Meinung nach fragwürdige verbotene Funktionen:

  • mb_parse_str
  • parse_str
  • parse_url
  • base64_decode

    • Ja, es wird in Backdoors verwendet, aber Banning evalsollte ausreichen, und es gibt legitime Anwendungsfälle, wie das Codieren von Binärdaten. Und abgesehen von json_decode(was nicht verboten ist) gibt es dafür keinen Core-Helfer.

Quelle: https://github.com/magento-ecg/coding-standard/blob/master/Sniffs/Security/ForbiddenFunctionSniff.php

Meine Frage lautet im Wesentlichen: Wo ist diese Norm dokumentiert? Und / oder gibt es eine Liste von "Dingen, die anstelle dieser nativen PHP-Funktionen verwendet werden können"?

Fabian Schmengler
quelle
1
Magento basiert auf Zend Framework. Warum verwenden Sie keine Zend-Standards?
Zhartaunik
Das EKG führt mehr Magento-spezifische Prüfungen durch, z. B. ob Modelle in Schleifen geladen sind. Hier geht es nicht um grundlegende Stilprüfungen wie Einrückungen und Klammern.
Fabian Schmengler
1
Das Auflisten von "Raw SQL-Abfragen" als verboten erscheint ebenfalls naiv. Natürlich machen Sie in den meisten Situationen kein rohes SQL, aber es gibt definitiv Zeiten, in denen es nicht nur angemessen, sondern notwendig ist (dh komplexe Importe / Exporte)
pspahn
1
@pspahn Ich verstehe Ihren Standpunkt, aber sollte der Zend_DbAbfrage-Generator keine SQL-Abfragen generieren können?
Fabian Schmengler
Sicher, aber können Sie nicht auch eine selectAnweisung erstellen, Zend_Dbindem Sie Roh-SQL als Eingabe verwenden? Ich nahm an, dass github.com/kalenjordan/custom-reports das im Backend macht.
Pspahn

Antworten:

28

Ich habe eine inoffizielle Antwort vom EKG-Team erhalten:

Erstens müssen die gekennzeichneten Funktionen nicht unbedingt unzulässig sein - sie sollten eine manuelle Überprüfung der Verwendung auslösen, um eine legitime Verwendung sicherzustellen. Es ist ein Bewertungshilfswerkzeug, kein gutes / schlechtes Code-Bewertungswerkzeug.

Zweitens wurde davon ausgegangen, dass es besser ist, Magento-Wrapper (Funktionen / Klassen) zu verwenden, wenn diese existieren, da sie möglicherweise zusätzliche Funktionen oder zusätzlichen Schutz bieten.

Drittens wird base64_decode für bestimmte Aufrufe häufig für böswilligen eingeschleusten Code verwendet, und der Rest wie parse_str kann anfällig sein, insbesondere für unbekannte oder vom Benutzer eingegebene Daten - siehe zum Beispiel http://php-security.org/2010/05/ 31 / mops-2010-049-php-parse_str-interruption-memory-corruption-vulnerability /

Auch dies markiert es zur Überprüfung, ohne den Code als schlecht abzulehnen.

Ich hoffe es hilft.

Piotr Kaminski
quelle
Warum schreiben sie dann "Die Funktion ist verboten" anstatt "Sie sollten den Code überprüfen, um seine legitime Verwendung sicherzustellen" ?!
Schwarz
11

Der Kodierungsstandard hat zwei Ziele.

  1. das Auffinden möglicherweise problematischer Teile erheblich erleichtern. Ein erfahrener Entwickler weiß bereits, welche Teile eines neuen Moduls überprüft werden müssen, und dieser Standard markiert und listet sie für ihn auf. Es ist nicht so, dass er diese Teile entfernen kann, sondern zu überprüfen, ob sie notwendig, problematisch oder beides sind.

  2. Unterstützung unerfahrener Entwickler, auf welche Dinge er verzichten sollte. Während alle markierten Funktionen gute Lösungen für bestimmte Probleme sein können, sind sie auf problematische Weise sehr einfach zu verwenden. Es führt die Entwickler dazu, mehr über das Problem nachzudenken und oft zu besseren Lösungen, die nicht mit dem Standard in Konflikt stehen.

Und manchmal folgen sogar die erfahrensten Entwickler blind dem Standard und schaffen die grausamsten Umgehungslösungen, um einen Community-Standard nicht zu verletzen.

ein bisschen zusätzliche Informationen

Dateifunktionen ermöglichen häufig die Verwendung von Protokoll-Wrappern. Dies bedeutet, dass ein Dateipfad, der mit http: // beginnt, zu einem http-Reaquest führt, der häufig für "Anrufe nach Hause" verwendet wird, und von Zeit zu Zeit einen Shop beendet, da der Server nicht erreichbar ist (30 Sekunden Standard-Timeout) und es ist an einem sehr zentralen Ort eingebaut.

In jedem Fall glaubt niemand, wie viele Sql-Injektionslöcher es noch gibt. Ein gutes Beispiel war, wie die Suche auf der MySQL-Website hatte.

Irgendwo gibt es einen Core-Helfer für json_decode, der jedoch eine sehr alte Implementierung hat. Verwenden Sie einfach json_decode. Keine Ahnung warum es verboten sein sollte.

gettext ist ein interessanter PHP-Teil. Ich erinnere mich, dass er eine native OS-Implementierung verwendet, die Probleme verursachen kann, wenn Sie ihn parallel zu verschiedenen Sprachen verwenden (wie es normalerweise der Fall ist, wenn Sie mehr als eine Sprache in Ihrem Shop haben) , aber es sollte auch im Magento-Kontext keine Notwendigkeit dafür geben.

Wenn Sie die Liste weiter durchgehen, scheint dies nur eine Liste mit möglichst vielen Funktionen zu sein. Die Geschichte ist eigentlich lustig, scheint, dass sie einige der Funktionen aus der Liste entfernt haben, nachdem sie bemerkt haben, dass sie davon Gebrauch machen. : D

Flyingmana
quelle