Рецепты код-ревью
Данная статья написана в поддержку презентации Код-ревью в Drupal.
Главной целью практически любого ревью является отсев деффектов, а в последнюю очередь просев форматирования. Посему, именно на них стоит концентрировать внимание в первую очередь при прогоне глазами кода.
Вот этапы, которые лично я использую в ревью (можно предлагать в комментах дополнения):
1. Поиск уязвимостей
1.1. Находим функции темизации и детально анализируем что выводится. Именно в них присутствует большинство XSS. Важно помнить, что это касается не только модулей, но и темы.
1.2. Если функций нет или мы всех их прошли, просматриваем код на наличие запросов в базу и *_load
функций. Отслеживаем куда ведут эти данные и ловим XSS. На этом этапе можно сразу проверять запросы — на отсутсвие переменных прямо в запросе (SQL injections), на правильную фильтрацию, а также правильное использование API, как в случае с LIMIT, которы пашет в зависимости от СУБД по-разному.
2. Поиск багов перевода
2.1. Ищете любые строки в коде. Они могут не переводится только в трех случаях — в хуке меню, в хуке схемы и когда там перевод мало нужен (чтобы не засорять бд).
2.2. Анализируете все вызовы t()
. В них не должно быть любой динамики. Вся динамика вставляется через плейсхолдеры. Будьте бдительны, так как тут может скрываться XSS, если используются плейсхолдеры с префиксом!
2.3. Номера переводятся через format_plural()
.
2.4. watchdog()
переводит все сам, не нужно пихать в него динамики и уже переведенных строк
2.5. Стоит помнить, что в некоторых случаях, как то рассылка писем, стоит использовать не текущий язык, а предпочитаемый язык конкретного пользователя.
3. Поиск багов темизации
3.1. Прямые вызовы theme_*
.
3.2. Объявление функции темизации без определения hook_theme()
.
3.3. Большие куски HTML кода практически всегда оправданно совать в шаблоны.
3.4. Изменение вывода в хуках node_api вмесмто использования препроцессинг функции.
3.5. В препроцессинг функциях переменная $vars
а использование $variables
и наоборот.
4. Плохо-выглядящий код (кандидаты на рефакторинг)
5. Ошибки форматирования
5.1. При обнаружении массовых ошибок, стоит посылать код на прогон модулем Coder самим автором.
Got anything to add?