优化发信安全清洗与事务持久化一致性

server/controllers/email/send.go

@@ -5,6 +5,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"git.pcux.cn/xuans/Smail/config"
+	"git.pcux.cn/xuans/Smail/consts"
 	"git.pcux.cn/xuans/Smail/db"
 	"git.pcux.cn/xuans/Smail/dto/parsemail"
 	"git.pcux.cn/xuans/Smail/dto/response"
@@ -24,6 +25,7 @@ import (
 	"strings"
 	"time"
 	"unicode"
+	"xorm.io/xorm"
 )
 
 const (
@@ -159,9 +161,9 @@ func Send(ctx *context.Context, w http.ResponseWriter, req *http.Request) {
 		}
 	}
 
-	e.Text = []byte(reqData.Text)
+	e.Text = []byte(parsemail.SanitizeText(reqData.Text))
-	e.HTML = []byte(reqData.HTML)
+	e.HTML = []byte(parsemail.SanitizeHTML(reqData.HTML))
-	e.Subject = reqData.Subject
+	e.Subject = parsemail.SanitizeText(reqData.Subject)
 	totalAttachmentSize := 0
 	for _, att := range reqData.Attachments {
 		contentType, content, err := decodeAttachment(att.Data)
@@ -222,8 +224,9 @@ func Send(ctx *context.Context, w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	now := time.Now()
 	modelEmail := models.Email{
-		Type:         1,
+		Type:         consts.EmailTypeSend,
 		Subject:      e.Subject,
 		ReplyTo:      storedFields.ReplyTo,
 		FromName:     e.From.Name,
@@ -238,18 +241,38 @@ func Send(ctx *context.Context, w http.ResponseWriter, req *http.Request) {
 		SPFCheck:     1,
 		DKIMCheck:    1,
 		SendUserID:   ctx.UserID,
-		SendDate:     time.Now(),
+		SendDate:     now,
-		CronSendTime: time.Now(),
+		CronSendTime: now,
-		Status:       1,
+		Status:       consts.EmailStatusSent,
-		CreateTime:   time.Now(),
+		CreateTime:   now,
 		MsgID:        msgID,
 	}
 
-	_, err = db.Instance.Insert(&modelEmail)
+	err = db.Transaction(db.Instance, func(session *xorm.Session) error {
+		candidate := modelEmail
+		candidate.Id = 0
+		if _, insertErr := session.Insert(&candidate); insertErr != nil {
+			return insertErr
+		}
+		if candidate.Id <= 0 {
+			return fmt.Errorf("insert outgoing email returned no identifier")
+		}
+		relation := models.UserEmail{
+			UserID:  ctx.UserID,
+			EmailID: candidate.Id,
+			Status:  consts.EmailStatusSent,
+			IsRead:  1,
+		}
+		if _, insertErr := session.Insert(&relation); insertErr != nil {
+			return insertErr
+		}
+		modelEmail = candidate
+		return nil
+	})
 
 	if err != nil || modelEmail.Id <= 0 {
 		if err != nil {
-			log.WithContext(ctx).WithError(err).Error("insert outgoing email failed")
+			log.WithContext(ctx).WithError(err).Error("insert outgoing email transaction failed")
 		} else {
 			log.WithContext(ctx).Error("insert outgoing email returned no identifier")
 		}
@@ -280,36 +303,29 @@ func Send(ctx *context.Context, w http.ResponseWriter, req *http.Request) {
 
 		if err != nil {
 			errMsg = err.Error()
-			_, err := db.Instance.Exec(db.WithContext(ctx, "update email set status =2 ,error=? where id = ? "), errMsg, modelEmail.Id)
+			_ = db.Transaction(db.Instance, func(session *xorm.Session) error {
-			if err != nil {
+				if _, updateErr := session.Exec(db.WithContext(ctx, "update email set status = ?, error = ? where id = ?"), consts.EmailStatusFail, errMsg, modelEmail.Id); updateErr != nil {
-				log.WithContext(ctx).Errorf("sql Error :%+v", err)
+					log.WithContext(ctx).Errorf("update email status failed: %+v", updateErr)
-			}
+					return updateErr
-
+				}
-			ue := models.UserEmail{
+				if _, updateErr := session.Exec(db.WithContext(ctx, "update user_email set status = ? where email_id = ? and user_id = ?"), consts.EmailStatusFail, modelEmail.Id, ctx.UserID); updateErr != nil {
-				UserID:  ctx.UserID,
+					log.WithContext(ctx).Errorf("update user_email status failed: %+v", updateErr)
-				EmailID: modelEmail.Id,
+					return updateErr
-				Status:  2,
+				}
-				IsRead:  1,
+				return nil
-			}
+			})
-			if _, insertErr := db.Instance.Insert(&ue); insertErr != nil {
-				log.WithContext(ctx).WithError(insertErr).Error("insert failed outgoing email relation failed")
-			}
-
 		} else {
-			_, err := db.Instance.Exec(db.WithContext(ctx, "update email set status =1  where id = ? "), modelEmail.Id)
+			_ = db.Transaction(db.Instance, func(session *xorm.Session) error {
-			if err != nil {
+				if _, updateErr := session.Exec(db.WithContext(ctx, "update email set status = ?, error = '' where id = ?"), consts.EmailStatusSent, modelEmail.Id); updateErr != nil {
-				log.WithContext(ctx).Errorf("sql Error :%+v", err)
+					log.WithContext(ctx).Errorf("update email status failed: %+v", updateErr)
-			}
+					return updateErr
-
+				}
-			ue := models.UserEmail{
+				if _, updateErr := session.Exec(db.WithContext(ctx, "update user_email set status = ? where email_id = ? and user_id = ?"), consts.EmailStatusSent, modelEmail.Id, ctx.UserID); updateErr != nil {
-				UserID:  ctx.UserID,
+					log.WithContext(ctx).Errorf("update user_email status failed: %+v", updateErr)
-				EmailID: modelEmail.Id,
+					return updateErr
-				Status:  1,
+				}
-				IsRead:  1,
+				return nil
-			}
+			})
-			if _, insertErr := db.Instance.Insert(&ue); insertErr != nil {
-				log.WithContext(ctx).WithError(insertErr).Error("insert sent email relation failed")
-			}
 		}
 
 	}, nil)

server/controllers/email/send_test.go

@@ -0,0 +1,108 @@
+package email
+
+import (
+	"testing"
+	"time"
+
+	"git.pcux.cn/xuans/Smail/consts"
+	"git.pcux.cn/xuans/Smail/db"
+	"git.pcux.cn/xuans/Smail/models"
+	"git.pcux.cn/xuans/Smail/utils/context"
+	"xorm.io/xorm"
+)
+
+func TestSendTransactionalPersistenceConsistency(t *testing.T) {
+	engine, err := xorm.NewEngine("sqlite", "file:"+t.Name()+"?mode=memory&cache=shared")
+	if err != nil {
+		t.Fatalf("create test engine: %v", err)
+	}
+	engine.SetMaxOpenConns(1)
+	if err := engine.Sync2(&models.Email{}, &models.UserEmail{}); err != nil {
+		t.Fatalf("sync test database: %v", err)
+	}
+	defer engine.Close()
+
+	ctx := &context.Context{
+		UserID:      1,
+		UserAccount: "testuser",
+		UserName:    "Test User",
+	}
+
+	now := time.Now()
+	modelEmail := models.Email{
+		Type:         consts.EmailTypeSend,
+		Subject:      "Test Email",
+		FromName:     "Test User",
+		FromAddress:  "testuser@example.com",
+		SendUserID:   ctx.UserID,
+		SendDate:     now,
+		CronSendTime: now,
+		Status:       consts.EmailStatusSent,
+		CreateTime:   now,
+		MsgID:        "msg-123@example.com",
+	}
+
+	err = db.Transaction(engine, func(session *xorm.Session) error {
+		candidate := modelEmail
+		candidate.Id = 0
+		if _, insertErr := session.Insert(&candidate); insertErr != nil {
+			return insertErr
+		}
+		relation := models.UserEmail{
+			UserID:  ctx.UserID,
+			EmailID: candidate.Id,
+			Status:  consts.EmailStatusSent,
+			IsRead:  1,
+		}
+		if _, insertErr := session.Insert(&relation); insertErr != nil {
+			return insertErr
+		}
+		modelEmail = candidate
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("insert email transaction failed: %v", err)
+	}
+
+	var email models.Email
+	has, err := engine.ID(modelEmail.Id).Get(&email)
+	if err != nil || !has {
+		t.Fatalf("email not found in database: %v", err)
+	}
+	if email.Status != consts.EmailStatusSent || email.Type != consts.EmailTypeSend {
+		t.Fatalf("unexpected email status/type: status=%d, type=%d", email.Status, email.Type)
+	}
+
+	var userEmail models.UserEmail
+	has, err = engine.Where("email_id = ? AND user_id = ?", modelEmail.Id, ctx.UserID).Get(&userEmail)
+	if err != nil || !has {
+		t.Fatalf("user_email not found: %v", err)
+	}
+	if userEmail.Status != consts.EmailStatusSent {
+		t.Fatalf("unexpected user_email status: %d", userEmail.Status)
+	}
+}
+
+func TestSafeAttachmentName(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{name: "normal", input: "test.pdf", expected: "test.pdf"},
+		{name: "path traversal", input: "../../etc/passwd", expected: ".._.._etc_passwd"},
+		{name: "windows path", input: "C:\\Users\\file.txt", expected: "C:_Users_file.txt"},
+		{name: "control chars", input: "file\r\nname\x00.png", expected: "file__name_.png"},
+		{name: "empty", input: "", expected: "attachment"},
+		{name: "dots", input: "..", expected: "attachment"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := safeAttachmentName(tt.input)
+			if result != tt.expected {
+				t.Errorf("safeAttachmentName(%q) = %q, want %q", tt.input, result, tt.expected)
+			}
+		})
+	}
+}

server/dto/parsemail/email.go

@@ -195,7 +195,7 @@ func init() {
 	relaxedPolicy.SkipElementsContent("script", "object", "embed", "iframe", "frame", "frameset")
 }
 
-func sanitizeHTML(htmlContent string) string {
+func SanitizeHTML(htmlContent string) string {
 	if htmlContent == "" {
 		return ""
 	}
@@ -269,7 +269,7 @@ func sanitizeHTMLNode(node *xhtml.Node) bool {
 }
 
 // Sanitize Text
-func sanitizeText(text string) string {
+func SanitizeText(text string) string {
 	return strictPolicy.Sanitize(text)
 }
 
@@ -461,8 +461,8 @@ func formatContent(entity *message.Entity, ret *Email) error {
 			fileName := getFileName(entity, p)
 
 			ret.Attachments = append(ret.Attachments, &Attachment{
-				Filename:    sanitizeText(fileName),
+				Filename:    SanitizeText(fileName),
-				ContentType: sanitizeText(strings.TrimSpace(contentType)),
+				ContentType: SanitizeText(strings.TrimSpace(contentType)),
 				Content:     body,
 				ContentID:   strings.TrimPrefix(strings.TrimSuffix(entity.Header.Get("Content-Id"), ">"), "<"),
 			})
@@ -481,8 +481,8 @@ func formatContent(entity *message.Entity, ret *Email) error {
 			fileName := getFileName(entity, p)
 
 			ret.Attachments = append(ret.Attachments, &Attachment{
-				Filename:    sanitizeText(fileName),
+				Filename:    SanitizeText(fileName),
-				ContentType: sanitizeText(strings.TrimSpace(contentType)),
+				ContentType: SanitizeText(strings.TrimSpace(contentType)),
 				Content:     body,
 				ContentID:   strings.TrimPrefix(strings.TrimSuffix(entity.Header.Get("Content-Id"), ">"), "<"),
 			})
@@ -490,7 +490,7 @@ func formatContent(entity *message.Entity, ret *Email) error {
 			return nil
 		}
 
-		ret.HTML = []byte(sanitizeHTML(string(body)))
+		ret.HTML = []byte(SanitizeHTML(string(body)))
 	case "multipart/related":
 	default:
 		c, err := io.ReadAll(entity.Body)
@@ -500,8 +500,8 @@ func formatContent(entity *message.Entity, ret *Email) error {
 		fileName := getFileName(entity, p)
 
 		ret.Attachments = append(ret.Attachments, &Attachment{
-			Filename:    sanitizeText(fileName),
+			Filename:    SanitizeText(fileName),
-			ContentType: sanitizeText(strings.TrimSpace(contentType)),
+			ContentType: SanitizeText(strings.TrimSpace(contentType)),
 			Content:     c,
 			ContentID:   strings.TrimPrefix(strings.TrimSuffix(entity.Header.Get("Content-Id"), ">"), "<"),
 		})

server/dto/parsemail/email_xss_test.go

@@ -85,9 +85,9 @@ func TestSanitizeHTML(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			result := sanitizeHTML(tc.input)
+			result := SanitizeHTML(tc.input)
 			if result != tc.expected {
-				t.Errorf("sanitizeHTML(%q) = %q, want %q", tc.input, result, tc.expected)
+				t.Errorf("SanitizeHTML(%q) = %q, want %q", tc.input, result, tc.expected)
 			}
 		})
 	}
@@ -143,9 +143,9 @@ func TestSanitizeText(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			result := sanitizeText(tc.input)
+			result := SanitizeText(tc.input)
 			if result != tc.expected {
-				t.Errorf("sanitizeText(%q) = %q, want %q", tc.input, result, tc.expected)
+				t.Errorf("SanitizeText(%q) = %q, want %q", tc.input, result, tc.expected)
 			}
 		})
 	}
@@ -328,13 +328,13 @@ func TestAttachment_XSS(t *testing.T) {
 
 	// Test that sanitizeText function works correctly on attachment data
 	dangerousFilename := `<script>alert('xss')</script>malicious.txt`
-	sanitizedFilename := sanitizeText(dangerousFilename)
+	sanitizedFilename := SanitizeText(dangerousFilename)
 	if strings.Contains(sanitizedFilename, "<script>") {
 		t.Errorf("Sanitized filename should not contain script tags, got: %q", sanitizedFilename)
 	}
 
 	dangerousContentType := `application/octet-stream<script>alert('xss')</script>`
-	sanitizedContentType := sanitizeText(dangerousContentType)
+	sanitizedContentType := SanitizeText(dangerousContentType)
 	if strings.Contains(sanitizedContentType, "<script>") {
 		t.Errorf("Sanitized content type should not contain script tags, got: %q", sanitizedContentType)
 	}
@@ -377,9 +377,9 @@ func TestComplexXSSScenarios(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			var result string
 			if tc.testType == "html" {
-				result = sanitizeHTML(tc.input)
+				result = SanitizeHTML(tc.input)
 			} else {
-				result = sanitizeText(tc.input)
+				result = SanitizeText(tc.input)
 			}
 
 			// 检查结果中不应包含危险的内容