SeniorДебаггингРедкоЕщё не отвечали
Code review — исправьте этот стор истории статусов заказа для Postgres
Разберите этот метод слоя хранения, который должен сохранять смену статуса заказа. Найдите боевые ошибки и объясните, как исправить каждую.
package domain
type OrderStatusHistoryStore struct{}
func NewStore() OrderStatusHistoryStore {
return OrderStatusHistoryStore{}
}
func (s OrderStatusHistoryStore) saveOrderStatusHistory(orderID, status, partition string) {
db, err := sql.Open("postgres", "user=foo dbname=bar sslmode=disable")
if err != nil {
fmt.Errorf("%v", err)
}
query := fmt.Sprintf(
"insert into order_status_history (order_id, status, partition, inserted_at) "+
"values (%v, %v, %v, %v)",
orderID, status, partition, time.Now())
go db.Exec(query)
}Найдите и исправьте ошибки.
Пять ошибок. sql.Open вызывается на каждый вызов, утекая новым пулом каждый раз — откройте один *sql.DB в NewStore и переиспользуйте. Результат fmt.Errorf отбрасывается, а метод не возвращает ошибку — верните обёрнутую ошибку. Запрос через fmt.Sprintf уязвим к SQL-инъекции — используйте параметризованный запрос $1..$4, передавая аргументы в Exec. go db.Exec — это fire-and-forget, теряющий ошибку и порядок — вызывайте синхронно. И нет context — принимайте ctx и используйте ExecContext.
- ✗Считать, что
db.Execобеззараживает запрос изfmt.Sprintf, поэтому инъекция невозможна - ✗Думать, что fire-and-forget
go db.Execприемлем, ведь вставка выполнится «когда-нибудь» - ✗Вызывать
sql.Openна каждый запрос, считая, что он открывает одно реальное соединение
- →Почему
sql.Openна самом деле не открывает соединение и что добавляетdb.Ping? - →Как подстановка
$1останавливает инъекцию, которую не остановит экранирование строки?
Оглавление
Найдите ошибки
func (s OrderStatusHistoryStore) saveOrderStatusHistory(orderID, status, partition string) {
db, err := sql.Open("postgres", "user=foo dbname=bar sslmode=disable") // ❌ новый пул на каждый вызов
if err != nil {
fmt.Errorf("%v", err) // ❌ ошибка построена и выброшена
}
query := fmt.Sprintf( // ❌ SQL-инъекция
"insert into order_status_history (order_id, status, partition, inserted_at) values (%v, %v, %v, %v)",
orderID, status, partition, time.Now())
go db.Exec(query) // ❌ fire-and-forget: ошибка и порядок потеряны; ❌ нет context
}Разбор и исправление
sql.Openна каждый вызов.sql.Openсоздаёт пул соединений, а не одно соединение, и его нельзя открывать на каждую операцию — пул не закрывается и утекает. Откройте*sql.DBодин раз при старте и положите его в стор.- Проглоченная ошибка + нет возврата.
fmt.Errorfлишь строит значение ошибки; здесь оно отбрасывается (go vetэто ловит). Метод обязан возвращатьerror. - SQL-инъекция.
fmt.Sprintfсо значениями в текст запроса — это и инъекция, и некорректный SQL для строк. Используйте параметризованный запрос с$1..$4; драйвер сам безопасно подставит значения. - fire-and-forget goroutine.
go db.Exec(...)теряет ошибку, не даёт порядка и может пережить запрос. Вызывайте синхронно (или планируйте намеренно, с обработкой ошибки). - Нет
context. Долгая операция должна отменяться — принимайтеctxи вызывайтеExecContext.
✅ Исправленная версия:
type OrderStatusHistoryStore struct {
db *sql.DB // один пул, переиспользуется
}
func NewStore(db *sql.DB) (*OrderStatusHistoryStore, error) {
if db == nil {
return nil, fmt.Errorf("store: nil db")
}
return &OrderStatusHistoryStore{db: db}, nil
}
func (s *OrderStatusHistoryStore) SaveOrderStatusHistory(
ctx context.Context, orderID, status, partition string,
) error {
const q = `insert into order_status_history
(order_id, status, partition, inserted_at)
values ($1, $2, $3, now())`
if _, err := s.db.ExecContext(ctx, q, orderID, status, partition); err != nil {
return fmt.Errorf("save order status history: %w", err)
}
return nil
}Оглавление
