Por que tantas funções PHP não são permitidas no Magento ECG Coding Standard?

30

O Magento ECG Coding Standard parece ser (pelo menos uma espécie de) oficial como padrão para extensões Magento 1:

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

Mas eu não entendo o raciocínio por trás de todas as regras, e as regras do sniffer de código apenas com suas mensagens não ajudam muito. Existe alguma documentação detalhada sobre o padrão? Conheço as melhores práticas comuns e o guia do desenvolvedor, mas não consigo encontrar nada específico sobre esses padrões de codificação.

O que mais me incomoda é a rigidez em não usar as funções PHP.

Por exemplo: Por que todas as funções PHP relacionadas ao sistema de arquivos são proibidas ?

Eu acho que você deveria usar Varien_Io_File, Varien_File_Objectetc. , mas mesmo os desenvolvedores principais não estão cientes de todas as classes Varien e muitas vezes você encontra coisas como Mage_ImportExport_Model_Import_Adapter_Csv:

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

Portanto, o núcleo não é o melhor exemplo, com a mesma frequência.

Outras funções proibidas questionáveis ​​do IMHO:

  • mb_parse_str
  • parse_str
  • parse_url

  • base64_decode

    • Sim, é usado em backdoors, mas o banimento evaldeve ser suficiente e há casos de uso legítimos, como codificação de dados binários. Além de json_decode(o que não é proibido), não há um auxiliar básico disponível para isso.

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

Essencialmente, minha pergunta se resume a: Onde esse padrão está documentado? E / ou existe uma lista de "coisas a serem usadas em vez dessas funções nativas do PHP"?

code-analysis magento-ecg coding-standards documentation Fabian Schmengler
fonte
1
Magento construído sobre o Zend Framework. Por que você não usa os padrões Zend?
Zhartaunik
O ECG faz mais verificações específicas do Magento, como se houver modelos carregados em loops. Não se trata de verificações básicas de estilo, como indentação e colchetes.
Fabian Schmengler
1
A listagem de "consultas SQL brutas" como proibida também parece ingênua. Claro que você não faça SQL cru na maioria das situações, mas há definitivamente momentos em que não só é apropriado, mas necessário (ou seja, importações complexos / exportação)
pspahn
1
@pspahn Entendo seu ponto de vista, mas o Zend_Dbconstrutor de consultas não deveria ser capaz de gerar consultas SQL?
Fabian Schmengler
Claro, mas você também não pode criar uma selectdeclaração Zend_Dbusando o SQL bruto como entrada? Supus que é isso que o github.com/kalenjordan/custom-reports faz no back-end.
pspahn

Respostas:

28

Recebeu uma resposta não oficial da equipe do ECG sobre isso:

Antes de tudo, as funções sinalizadas não são necessariamente proibidas - elas devem acionar uma revisão manual sobre o uso para garantir o uso legítimo. É uma ferramenta auxiliar de revisão, não uma ferramenta de pontuação de código boa / ruim.

Segundo, pressupõe-se que é melhor usar os invólucros Magento (funções / classes) se eles existirem, pois podem oferecer funcionalidade ou proteção adicional.

Terceiro, como para chamadas específicas, base64_decode é usado frequentemente para código injetado mal-intencionado, e o restante, como parse_str, pode ser vulnerável, especialmente lidando com entradas desconhecidas ou fornecidas pelo usuário - veja, por exemplo, http://php-security.org/2010/05/ 31 / mops-2010-049-php-parse_str-interruption-memory-corrupt-vulnerability /

Novamente, isso está sinalizando para revisão, não rejeitando o código como incorreto.

Espero que ajude.

Piotr Kaminski
fonte
Então, por que eles estão escrevendo "a função é proibida" em vez de "você deve revisar o código para garantir seu uso legítimo" ?!
Preto
11

O Padrão de Codificação tem dois objetivos.

  1. tornar muito mais fácil encontrar peças possivelmente problemáticas. Um desenvolvedor experiente já sabe quais partes de um novo módulo ele precisa revisar, e esse padrão as marca e as lista para ele. Não é para ele remover essas partes, mas revisar se são necessárias, problemáticas ou ambas.

  2. apoiar desenvolvedores inexperientes sobre quais coisas ele deve evitar. Embora todas as funções marcadas possam ser boas soluções para problemas específicos, elas são muito fáceis de usar de maneira problemática. Isso leva os desenvolvedores a pensar mais sobre o problema e, frequentemente, a soluções melhores que não entram em conflito com o padrão.

E, às vezes, até os desenvolvedores mais experientes seguem cegamente o padrão e criam as soluções mais cruéis, apenas para não ofender um padrão forçado da comunidade.

um pouco de informações adicionais

As funções de arquivo geralmente permitem o uso de invólucros protocoll, significa que um caminho de arquivo que começa com http: // leva a uma readaptação http, que é freqüentemente usada para "ligar para casa" e, de tempos em tempos, mata uma loja, porque o servidor não está acessível (Tempo limite padrão de 30 segundos) e é incorporado em um local muito central.

Como o sql está realizando, ninguém acredita quantos buracos de injeção de sql ainda existem por aí. Um ótimo exemplo foi, como o site Search no Mysql tinha um.

existe um auxiliar central para json_decode em algum lugar, mas ele tem uma implementação muito antiga, basta usar json_decode. Não faço ideia por que deveria ser proibido.

gettext é uma parte interessante do php, lembro que ele usa alguma implementação nativa do sistema operacional que pode ter problemas, se você o usar paralelamente a diferentes idiomas (como geralmente acontece quando você tem mais de um idioma em sua loja. Nunca realmente investigou tanto , mas também não deve ser necessário no Magento Context.

Indo mais longe na lista, isso parece ser apenas uma lista com o máximo de funções possível. A história é realmente engraçada, parece que eles removeram algumas das funções da lista, depois que perceberam, fizeram uso dela. : D

Flyingmana
fonte