api.php - маршруты
test_original.php - старый код контроллера
test_new.php - новый код контроллера
- Новый код сделан по принципу API для фронта.
- Функции разделены на 2 логические части и 2 отдельных метода. Оптимизирован код и адаптирован под Laravel либо любой другой фреймворк.
- Валидацию можно делать как внутри контроллера, так и в отдельном Request
- Ответ в формате JSON
- Если фронт тоже на PHP, то эту переменную с данными можно отправить и отрендерить через blade и twig
Код-ревью файла - test_original.php
Краткое резюме: в результате анализа кода были выявлены несколько ключевых проблем,
таких как уязвимость к SQL-инъекциям, использование устаревших методов,
отсутствие корректной обработки данных и проблемы с безопасностью
при работе с файлами.
Рекомендуется: для улучшения качества кода внедрить защиту от SQL-инъекций,
заменить устаревшие глобальные переменные на более безопасные методы,
обеспечить правильную обработку и валидацию данных,
а также оптимизировать работу с файлами для предотвращения
потенциальных уязвимостей и самое главное - использовать методы из фреймворка.
- Использование $_REQUEST: Это может привести к неожиданному поведению. Лучше использовать $_GET или $_POST, но лучшим решением будет валидация входящих данных через $data = $request->validated();
- SQL-инъекция: Оператор LIKE используется без правильной экранизации. Нужно экранировать символы % и _. Также лучше использовать методы из ORM Eloquent. Запросы более гибкие, меньше ошибок и лучше производительность.
- $unit = VBPost::query()->whereLike('text’, $unit_text)->get();
- Хардкод данных для подключения к БД:
- $db = new \PDO('mysql:dbname=vbforum;host=127.0.0.1', 'forum', '123456'); Использование и хранение паролей и доступов в открытом виде. Необходимо хранить все ключи в conf файле, либо env, чтобы при изменении доступов не менять все пароли во всех файлах.
- Логгирование через fopen(): Может привести к проблемам с производительностью и блокировкой файлов. Лучше писать это в БД, а при ошибках фатальных - писать логами фреймворка.
- Глобальная переменная $render: Использование глобальных переменных вносит сложность в код и дополнительную уязвимость, это ведет к зависимости от глобального состояния и некорректное поведение кода.
- Отсутствие обработки ошибок PDO: Нет проверки на успешность выполнения запросов к базе данных.
- fetchAll() может перегрузить память, т.к. неизвестно количество значений в БД. Лучше использовать постраничный вывод, либо ограничивать выдачу записей.
- Уязвимость к XSS: Данные выводятся без экранирования.
- Устаревший метод: Использование global и суперглобальных переменных усложняет тестируемость и поддержку кода.
- $render->render_searh_result($row); - опечатка в коде, и неизвестно есть ли этот метод и его наличие. Если это рекурсивная функция,то она приведет к ошибке и бесконечному циклу.