JianFeeeee
6e4bcd66be
## 缺陷
`/mail/send` 的回滚块注释写着「三件事都要退：邮件本身、本次往返预算、relay
幂等键」，但实际**一件都退不掉**：
relayed_mails.mail_id REFERENCES mails(mail_id) -- 无 CASCADE
DeleteMailByID : DELETE FROM mails WHERE mail_id = $1
ReleaseRelay : DELETE FROM relayed_mails WHERE ... AND mail_id IS NULL
`BindRelayMail` 原来在 `attachAll` **之前**执行，所以走到回滚块时该键已经绑上了：
- `DeleteMailByID` 撞外键失败（生产 DSN 有 `foreign_keys(1)`）⇒ 邮件留在库里；
- `ReleaseRelay` 的 WHERE 是 `mail_id IS NULL`，对已绑定的行是 no-op ⇒ 键没退。
后果与注释想避免的正好相反：发件方收到 4xx 会重试，收件方看到那封残余邮件 ——
两封。
## 修法
把 `BindRelayMail` 挪到回滚块**之后**。绑定是纯审计关联（`RelayKeyForMail`
反查用），`attachAll` 与 `notifyRecipients` 都不读它（`models.Mail` 零 relay
字段），所以推迟没有副作用。
## 判据
`TestMailSendRollbackRemovesMailAndRelayKey`：同一个 `attachment_id` 传两次 ——
`checkAttachable` 逐条查时都还没挂载（都通过），`attachAll` 的原子 UPDATE 在
第二条改到 0 行 ⇒ 409 ⇒ 回滚。这样无需真并发就能确定性地走到那条路径。
三条断言：① 触发条件成立（409，否则用例会静默退化成空跑）；② 邮件没留下；
③ 幂等键退回去了。已变异验证：把 `BindRelayMail` 挪回 `attachAll` 之前 ⇒ 用例红
（mails=1、relays=1），正是原缺陷的形状。
`me.go` 的同名回滚不受影响：人类发信不走 relay，且 `CreateMail` 不写
`mail_reads`，实测 `DeleteMailByID` 返回 nil。
`go test ./...` 13 包全绿；handler 组 -race 通过；gofmt/vet 干净。
80 lines
3.2 KiB
Go
80 lines
3.2 KiB
Go
package handler
|
||
|
||
import (
|
||
"bytes"
|
||
"context"
|
||
"encoding/json"
|
||
"net/http"
|
||
"net/http/httptest"
|
||
"testing"
|
||
|
||
"github.com/agentmail/gateway/internal/db"
|
||
"github.com/agentmail/gateway/internal/middleware"
|
||
"github.com/agentmail/gateway/internal/repo"
|
||
)
|
||
|
||
// 附件竞态回滚必须**真的**把邮件和幂等键都退掉。
|
||
//
|
||
// 触发方式：同一个 attachment_id 传两次 —— `checkAttachable` 逐个查时两条都还
|
||
// 没挂载（都通过），`attachAll` 的原子 UPDATE 在第二条上改到 0 行 ⇒ 409。
|
||
// 这正是那条回滚路径要处理的形状（"另一个请求把同一个附件挂走了"），
|
||
// 只是这里由同一个请求自己造出竞态，无需真并发。
|
||
//
|
||
// 背景：relayed_mails.mail_id 对 mails 是外键且**无 CASCADE**。原先
|
||
// BindRelayMail 在 attachAll **之前**执行，于是回滚块里的 DeleteMailByID 撞外键
|
||
// 失败（邮件留在库里），而 ReleaseRelay 的 WHERE 是 `mail_id IS NULL`，对已绑定的
|
||
// 行也是 no-op —— 注释写着"三件事都要退"，实际一件都没退掉。
|
||
//
|
||
// 判据两条都要：
|
||
// 1. 邮件没留下（否则会重复投递给收件方）；
|
||
// 2. 幂等键退回去了（否则上游那条消息永远转不出来，重试只会 duplicate）。
|
||
//
|
||
// 只断言 1 会放过"邮件删了但键被永久占住"；只断言 2 会放过残余邮件。
|
||
func TestMailSendRollbackRemovesMailAndRelayKey(t *testing.T) {
|
||
setupPermissionHandlerDB(t)
|
||
ctx := context.Background()
|
||
if err := repo.CreateOrUpdateAgent(ctx, "pi", "secret", "test", nil); err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
sid, err := repo.CreateSession(ctx, nil, "pi", "x", "/tmp")
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
att, err := repo.CreateAttachment(ctx, "pi", "f.txt", "text/plain", 3, "sum-x")
|
||
if err != nil {
|
||
t.Fatal(err)
|
||
}
|
||
payload, _ := json.Marshal(map[string]any{
|
||
"to": "pi", "subject": "s", "body": "b",
|
||
"from_session_id": sid.String(),
|
||
"attachment_ids": []string{att.ID.String(), att.ID.String()},
|
||
"relay_key": "probe:rb-1",
|
||
"relay": "summary",
|
||
})
|
||
req := httptest.NewRequest(http.MethodPost, "/api/v1/mail/send", bytes.NewReader(payload))
|
||
req.Header.Set("Content-Type", "application/json")
|
||
reqCtx := context.WithValue(req.Context(), middleware.AgentNameKey, "pi")
|
||
resp := httptest.NewRecorder()
|
||
SendMail(resp, req.WithContext(reqCtx))
|
||
|
||
// 前提：触发条件成立，确实走到了回滚那条路。
|
||
//
|
||
// 这一条不能省：若哪天 parseAttachmentIDs 开始去重（或 attachAll 变得容忍
|
||
// 重复 id），请求会变成 200 且**根本没建邮件**，于是下面两个计数都是 0 ——
|
||
// 用例会"通过"，但它一次都没验到回滚。断言触发本身，才不会静默退化成空跑。
|
||
if resp.Code != http.StatusConflict {
|
||
t.Fatalf("重复 attachment_id 应触发 409 以走到回滚路径，实际 HTTP %d: %s",
|
||
resp.Code, resp.Body.String())
|
||
}
|
||
|
||
var mails, relays int
|
||
_ = db.DB.QueryRowContext(ctx, `SELECT count(*) FROM mails`).Scan(&mails)
|
||
_ = db.DB.QueryRowContext(ctx, `SELECT count(*) FROM relayed_mails WHERE relay_key='probe:rb-1'`).Scan(&relays)
|
||
if mails != 0 {
|
||
t.Errorf("回滚没删掉邮件（mails=%d）", mails)
|
||
}
|
||
if relays != 0 {
|
||
t.Errorf("回滚没退掉幂等键（relays=%d）", relays)
|
||
}
|
||
}
|