fix: api_endpoint URL validation#39
Conversation
madmatvey
left a comment
There was a problem hiding this comment.
0. Общее описание изменений
Ценность для продукта:
Ветка добавляет строгую валидацию параметра api_endpoint в конфиге SDK: теперь при установке некорректного URL (не http/https, без host и т.д.) выбрасывается осмысленная ошибка. Это защищает пользователей от трудноотлавливаемых ошибок подключения и потенциальных SSRF-уязвимостей, делая интеграцию безопаснее и предсказуемее.
Инженерный подход:
Использован явный валидатор с помощью стандартного URI.parse и дополнительных проверок, реализованный через приватный метод и вызываемый при конфигурировании. Добавлены edge-case тесты, обновлена документация. Решение следует принципам fail-fast, separation of concerns и TDD.
1. Получение изменений и область анализа
Изменённые файлы:
README.mdissue5.mdlib/gasfree_sdk.rbspec/gasfree_sdk_spec.rb
Результаты тестов и линтера:
- Все 74 теста прошли успешно (
rspec) rubocopне выявил ошибок стиля
2. Критерии оценки изменений
README.md
- Документация: Добавлено явное описание требования к формату
api_endpointи поведения при ошибке.
— 👍 Соответствует best practices: пользователь сразу видит новые требования.
issue5.md
- Документация/План: Подробный план действий, история обсуждения, промежуточные варианты кода.
— 👍 Хорошая инженерная культура, traceability изменений.
lib/gasfree_sdk.rb
-
Проектирование:
- Валидация вынесена в отдельный приватный метод
validate_api_endpoint!, вызывается только при изменении значения. - Нет лишней связанности, логика не дублируется.
- Метод
reset_client!остался публичным, что важно для тестов.
- Валидация вынесена в отдельный приватный метод
-
Сложность:
- Логика проверки проста, читаема, покрывает граничные случаи (host, схема, пустота).
- Нет избыточной вложенности.
-
Работоспособность:
- Ошибки обрабатываются через кастомный класс
GasfreeSdk::Error, сообщения информативны. - Проверка схемы и host защищает от SSRF и silent failures.
- Ошибки обрабатываются через кастомный класс
-
Читаемость:
- Названия методов и переменных осмысленны.
- Порядок кода логичен, приватные методы отделены.
-
Лучшие практики:
- Fail-fast, separation of concerns, явная обработка ошибок.
- Нет side-effects вне зоны ответственности.
-
Тестирование:
- Логика покрыта тестами (см. ниже).
-
Стиль:
- Нет нарушений стиля, отступы и порядок соблюдены.
-
Безопасность:
- Валидация URL снижает риск SSRF и ошибок конфигурации.
-
Производительность:
- Валидация не влияет на runtime, вызывается только при изменении значения.
spec/gasfree_sdk_spec.rb
- Тестирование:
- Добавлены тесты на невалидные значения (
not a url,ftp://,http:///missinghost). - Проверяются как ошибки, так и допустимые значения (
http,https). - Названия тестов осмысленны, покрыты edge-cases.
- Тесты изолированы, не зависят от внешних факторов.
- Добавлены тесты на невалидные значения (
3. Сообщение об ошибках
Нет критичных или мажорных проблем, но отмечу потенциальные улучшения:
-
Файл:
lib/gasfree_sdk.rb- (Enhancement) Можно добавить комментарий к методу
validate_api_endpoint!, поясняющий, почему именно такие проверки (например, защита от SSRF). - (Enhancement) В будущем можно вынести валидацию в отдельный модуль/класс, если появятся другие параметры с похожими требованиями.
- (Enhancement) Можно добавить комментарий к методу
-
Файл:
spec/gasfree_sdk_spec.rb- (Enhancement) Можно добавить тест на пустую строку и строку с пробелами для
api_endpoint.
- (Enhancement) Можно добавить тест на пустую строку и строку с пробелами для
Prioritized Issues
Critical
- Нет
Major
- Нет
Minor
- Нет
Enhancement
lib/gasfree_sdk.rb: добавить поясняющий комментарий к методу валидации.spec/gasfree_sdk_spec.rb: добавить тесты на пустую строку и строку с пробелами.
Положительные моменты
- Валидация реализована идиоматично, без лишней связанности.
- Покрытие тестами отличное, edge-cases учтены.
- Документация обновлена своевременно.
- Нет новых ошибок линтера или тестов.
- Решение повышает безопасность и UX SDK.
Рекомендация:
PR можно мержить. Улучшения — необязательны и могут быть внесены в будущем.
No description provided.