diff --git a/internal/web/service/tgbot/screen.go b/internal/web/service/tgbot/screen.go index 18ddc3563..3bf0ea1ac 100644 --- a/internal/web/service/tgbot/screen.go +++ b/internal/web/service/tgbot/screen.go @@ -2,6 +2,7 @@ package tgbot import ( "context" + "errors" "strings" "sync" "time" @@ -9,6 +10,7 @@ import ( "github.com/mhsanaei/3x-ui/v3/internal/logger" "github.com/mymmrac/telego" + "github.com/mymmrac/telego/telegoapi" tu "github.com/mymmrac/telego/telegoutil" ) @@ -185,37 +187,13 @@ func (t *Tgbot) editScreen(chatID int64, prev *screen, next *screen) bool { var err error switch { - case next.hasPhoto && next.photo != "": - _, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{ - ChatID: tu.ID(chatID), - MessageID: prev.msgID, - Media: &telego.InputMediaPhoto{ - Type: "photo", Media: tu.FileFromID(next.photo), Caption: next.text, ParseMode: "HTML", - }, - ReplyMarkup: next.markup, - }) - case next.attachment != nil: - _, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{ - ChatID: tu.ID(chatID), - MessageID: prev.msgID, - Media: &telego.InputMediaPhoto{ - Type: "photo", Media: *next.attachment, Caption: next.text, ParseMode: "HTML", - }, - ReplyMarkup: next.markup, - }) - case next.hasPhoto: - // No file_id yet (the avatar was never read): upload the fallback tile. - upload, ok := artUpload(next.kind) - if !ok { - return false - } - _, err = liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{ - ChatID: tu.ID(chatID), - MessageID: prev.msgID, - Media: &telego.InputMediaPhoto{ - Type: "photo", Media: upload, Caption: next.text, ParseMode: "HTML", - }, - ReplyMarkup: next.markup, + case next.hasPhoto || next.attachment != nil: + _, err = requestScreenPhoto(next, func(photo telego.InputFile) (*telego.Message, error) { + return liveBot().EditMessageMedia(ctx, &telego.EditMessageMediaParams{ + ChatID: tu.ID(chatID), MessageID: prev.msgID, + Media: &telego.InputMediaPhoto{Type: "photo", Media: photo, Caption: next.text, ParseMode: "HTML"}, + ReplyMarkup: next.markup, + }) }) case prev.hasPhoto: // A photo message cannot become a text one: drop the picture and let @@ -250,24 +228,12 @@ func (t *Tgbot) replaceScreen(chatID int64, sc *screen) int { var sent *telego.Message var err error switch { - case sc.hasPhoto && sc.photo != "": - sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{ - ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: tu.FileFromID(sc.photo), - Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup, - }) - case sc.attachment != nil: - sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{ - ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: *sc.attachment, - Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup, - }) - case sc.hasPhoto: - upload, ok := artUpload(sc.kind) - if !ok { - return 0 - } - sent, err = liveBot().SendPhoto(ctx, &telego.SendPhotoParams{ - ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: upload, - Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup, + case sc.hasPhoto || sc.attachment != nil: + sent, err = requestScreenPhoto(sc, func(photo telego.InputFile) (*telego.Message, error) { + return liveBot().SendPhoto(ctx, &telego.SendPhotoParams{ + ChatID: tu.ID(chatID), MessageThreadID: adminTopic(chatID), Photo: photo, + Caption: sc.text, ParseMode: "HTML", ReplyMarkup: sc.markup, + }) }) default: sent, err = liveBot().SendMessage(ctx, &telego.SendMessageParams{ @@ -292,6 +258,45 @@ func (t *Tgbot) replaceScreen(chatID int64, sc *screen) int { return sent.MessageID } +// Only a rejected cached decoration gets one upload retry; QR pictures remain intact. +// The cache is invalidated only after the same caption and keyboard succeed without that id. +func requestScreenPhoto(sc *screen, request func(telego.InputFile) (*telego.Message, error)) (*telego.Message, error) { + var photo telego.InputFile + switch { + case sc.hasPhoto && sc.photo != "": + photo = tu.FileFromID(sc.photo) + case sc.attachment != nil: + photo = *sc.attachment + default: + var ok bool + photo, ok = artUpload(sc.kind) + if !ok { + return nil, errors.New("screen artwork is unavailable") + } + } + sent, err := request(photo) + var apiErr *telegoapi.Error + if err == nil || sc.photo == "" || sc.qrPicture != nil || sc.attachment != nil || + !errors.As(err, &apiErr) || apiErr.ErrorCode != 400 || isTelegramNotModifiedError(err) { + return sent, err + } + upload, ok := artUpload(sc.kind) + if !ok { + return sent, err + } + sent, err = request(upload) + if err != nil { + return sent, err + } + rejectArtFileID(sc.photo) + sc.photo = "" + if sent != nil && len(sent.Photo) > 0 { + sc.photo = sent.Photo[len(sent.Photo)-1].FileID + cacheArtFileID(sc.kind, sc.photo) + } + return sent, nil +} + // ensureBackRow guarantees every screen can be left (a screen with only its own // actions traps the user) and checks by callback, not label. diff --git a/internal/web/service/tgbot/screen_media_test.go b/internal/web/service/tgbot/screen_media_test.go new file mode 100644 index 000000000..a5ff4f5bf --- /dev/null +++ b/internal/web/service/tgbot/screen_media_test.go @@ -0,0 +1,362 @@ +package tgbot + +import ( + "encoding/json" + "fmt" + "image/png" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "sync" + "testing" + "time" + + "github.com/mymmrac/telego" + "github.com/nicksnyder/go-i18n/v2/i18n" +) + +type screenMediaRequest struct { + Method string + ChatID int64 `json:"chat_id"` + MessageID int `json:"message_id"` + Caption string `json:"caption"` + Photo string `json:"photo"` + Media struct { + Media string `json:"media"` + Caption string `json:"caption"` + } `json:"media"` + ReplyMarkup telego.InlineKeyboardMarkup `json:"reply_markup"` + Uploaded bool + UploadName string +} + +type screenMediaAPI struct { + mu sync.Mutex + avatar string + code int + failUpload bool + failEdit bool + badResponse bool + uploadStarted chan struct{} + releaseUpload <-chan struct{} + requests []screenMediaRequest +} + +func newScreenMediaAPI(t *testing.T) (*Tgbot, *screenMediaAPI) { + t.Helper() + api := &screenMediaAPI{avatar: "rejected-avatar", code: 400} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + method := r.URL.Path[strings.LastIndex(r.URL.Path, "/")+1:] + w.Header().Set("Content-Type", "application/json") + api.mu.Lock() + avatar := api.avatar + api.mu.Unlock() + switch method { + case "getMe": + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{ + "id": 42, "is_bot": true, "first_name": "test", "username": "test_bot", "supports_inline_queries": true, + }}) + return + case "getUserProfilePhotos": + if avatar == "" { + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{"total_count": 0, "photos": [][]telego.PhotoSize{}}}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{ + "total_count": 1, "photos": [][]telego.PhotoSize{{{FileID: avatar, FileUniqueID: "avatar", Width: 640, Height: 640}}}, + }}) + return + } + + request := screenMediaRequest{Method: method} + if strings.HasPrefix(r.Header.Get("Content-Type"), "multipart/form-data") { + if err := r.ParseMultipartForm(1 << 20); err != nil { + t.Errorf("parse upload: %v", err) + return + } + defer r.MultipartForm.RemoveAll() + request.ChatID = parseInt64(r.FormValue("chat_id")) + request.MessageID = int(parseInt64(r.FormValue("message_id"))) + request.Caption, request.Photo = r.FormValue("caption"), r.FormValue("photo") + _ = json.Unmarshal([]byte(r.FormValue("reply_markup")), &request.ReplyMarkup) + _ = json.Unmarshal([]byte(r.FormValue("media")), &request.Media) + for _, files := range r.MultipartForm.File { + for _, file := range files { + reader, err := file.Open() + if err != nil { + t.Errorf("open uploaded photo: %v", err) + return + } + config, err := png.DecodeConfig(reader) + _ = reader.Close() + if err != nil || (file.Filename == "screen.png" && (config.Width != 512 || config.Height != 512)) { + t.Errorf("fallback is not the generated 512x512 PNG: %+v, %v", config, err) + } + request.Uploaded = true + request.UploadName = file.Filename + } + } + } else if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode %s: %v", method, err) + return + } + if method == "editMessageMedia" { + request.Caption = request.Media.Caption + request.Photo = request.Media.Media + } + api.mu.Lock() + api.requests = append(api.requests, request) + code, failUpload, failEdit := api.code, api.failUpload, api.failEdit + badResponse, started, release := api.badResponse, api.uploadStarted, api.releaseUpload + id := 100 + len(api.requests) + api.mu.Unlock() + if request.Uploaded && started != nil { + select { + case started <- struct{}{}: + default: + } + select { + case <-release: + case <-r.Context().Done(): + return + } + } + if badResponse { + _, _ = w.Write([]byte("{")) + return + } + if (request.Photo == "rejected-avatar" && !request.Uploaded) || (failUpload && request.Uploaded) || (failEdit && method == "editMessageMedia") { + _ = json.NewEncoder(w).Encode(map[string]any{"ok": false, "error_code": code, "description": "Bad Request: wrong file identifier/HTTP URL specified"}) + return + } + if method == "deleteMessage" { + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": true}) + return + } + if request.MessageID != 0 { + id = request.MessageID + } + photoID := request.Photo + if request.Uploaded { + photoID = "fallback-photo" + } + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "result": map[string]any{ + "message_id": id, "date": 0, "chat": map[string]any{"id": request.ChatID, "type": "private"}, + "photo": []telego.PhotoSize{{FileID: photoID, FileUniqueID: photoID, Width: 512, Height: 512}}, + }}) + })) + t.Cleanup(srv.Close) + swapTestBot(t, srv.URL) + tb := &Tgbot{} + tb.resolveScreenArt() + return tb, api +} + +func (api *screenMediaAPI) snapshot() []screenMediaRequest { + api.mu.Lock() + defer api.mu.Unlock() + return append([]screenMediaRequest(nil), api.requests...) +} + +func TestStartRecoversWhenTelegramRejectsAvatar(t *testing.T) { + tb, api := newScreenMediaAPI(t) + initReportDB(t) + setTestAdmins(t, 1) + draftLocalizer(t, + &i18n.Message{ID: "tgbot.buttons.catServer", Other: "Server"}, + &i18n.Message{ID: "tgbot.buttons.catClients", Other: "Clients"}, + &i18n.Message{ID: "tgbot.buttons.catTraffic", Other: "Traffic"}, + &i18n.Message{ID: "tgbot.buttons.catMaintenance", Other: "Maintenance"}, + &i18n.Message{ID: "tgbot.buttons.refresh", Other: "Refresh"}, + &i18n.Message{ID: "tgbot.messages.hostname", Other: "Server: {{.Hostname}}"}, + ) + tb.answerCommand(&telego.Message{Chat: telego.Chat{ID: 500, Type: "private"}, From: &telego.User{ID: 1}, Text: "/start"}, 500, true) + screen, ok := tb.screens().get(500) + if !ok || screen.msgID == 0 { + t.Fatal("/start left the chat without a menu after Telegram rejected its avatar") + } + requests := api.snapshot() + if len(requests) != 2 || !requests[1].Uploaded { + t.Fatalf("requests = %+v, want one failed file-id send and one fallback upload", requests) + } + if requests[0].Caption != requests[1].Caption || !reflect.DeepEqual(requests[0].ReplyMarkup, requests[1].ReplyMarkup) || requests[1].ChatID != 500 { + t.Fatalf("fallback changed the menu content or destination: %+v", requests) + } + if len(requests[1].ReplyMarkup.InlineKeyboard) != 3 || requests[1].ReplyMarkup.InlineKeyboard[0][0].CallbackData != cbCatServer { + t.Fatalf("fallback did not carry the admin menu: %+v", requests[1].ReplyMarkup) + } + if screen.photo != "fallback-photo" { + t.Errorf("tracked photo = %q, want reusable fallback-photo", screen.photo) + } + + tb.resolveScreenArt() + tb.renderScreen(500, tb.newScreen("main", "next", tb.homeRows()...)) + tb.renderScreen(501, tb.newScreen("status", "another screen", tb.homeRows()...)) + for _, request := range api.snapshot()[2:] { + if request.Photo == "rejected-avatar" && !request.Uploaded { + t.Errorf("the unchanged rejected avatar was reused: %+v", request) + } + } + + api.mu.Lock() + api.avatar = "new-avatar" + api.mu.Unlock() + tb.resolveScreenArt() + tb.renderScreen(500, tb.newScreen("main", "new avatar", tb.homeRows()...)) + last := api.snapshot() + if last[len(last)-1].Photo != "new-avatar" || last[len(last)-1].Uploaded { + t.Errorf("a changed avatar was not adopted: %+v", last[len(last)-1]) + } +} + +func TestScreenMediaEditRecoversWithoutReplacingMessage(t *testing.T) { + tb, api := newScreenMediaAPI(t) + tb.screens().put(1, &screen{msgID: 9, kind: "main", hasPhoto: true}) + tb.renderScreen(1, tb.newScreen("main", "updated", tb.homeRows()...)) + requests := api.snapshot() + if len(requests) != 2 || requests[0].Method != "editMessageMedia" || requests[1].Method != "editMessageMedia" || !requests[1].Uploaded { + t.Fatalf("requests = %+v, want a bounded media-edit fallback on the existing message", requests) + } + tracked, _ := tb.screens().get(1) + if tracked.msgID != 9 || tracked.photo != "fallback-photo" { + t.Errorf("recovered screen = %+v, want message 9 with fallback-photo", tracked) + } +} + +func TestScreenMediaFailureKeepsThePreviousMenuAndCache(t *testing.T) { + tb, api := newScreenMediaAPI(t) + api.mu.Lock() + api.failUpload, api.failEdit = true, true + api.mu.Unlock() + previous := &screen{msgID: 9, kind: "main", hasPhoto: true, photo: "rejected-avatar"} + tb.screens().put(1, previous) + tb.renderScreen(1, tb.newScreen("main", "update", tb.homeRows()...)) + tracked, _ := tb.screens().get(1) + if tracked != previous || tracked.msgID != 9 { + t.Errorf("failed recovery replaced the previous menu: %+v", tracked) + } + if id, _ := artFileID("main"); id != "rejected-avatar" { + t.Errorf("failed upload poisoned the artwork cache: %q", id) + } + requests := api.snapshot() + if len(requests) > 4 { + t.Fatalf("requests = %+v, want at most one upload retry per edit/send operation", requests) + } + for _, request := range requests { + if request.Method == "deleteMessage" { + t.Fatal("the previous menu was deleted before a replacement succeeded") + } + } +} + +func TestScreenMediaHealthyBotsKeepTheirExistingRendering(t *testing.T) { + for _, avatar := range []string{"valid-avatar", ""} { + t.Run(avatar, func(t *testing.T) { + tb, api := newScreenMediaAPI(t) + api.mu.Lock() + api.avatar = avatar + api.mu.Unlock() + resetScreenArt() + tb.resolveScreenArt() + tb.renderScreen(1, tb.newScreen("main", "first", tb.homeRows()...)) + tb.renderScreen(1, tb.newScreen("main", "second", tb.homeRows()...)) + requests := api.snapshot() + if len(requests) != 2 || requests[0].Method != "sendPhoto" || requests[1].Method != "editMessageMedia" { + t.Fatalf("healthy bot changed its send/edit behavior: %+v", requests) + } + wantPhoto := avatar + if avatar == "" { + wantPhoto = "fallback-photo" + if !requests[0].Uploaded { + t.Error("an avatar-less bot did not upload its usual fallback tile") + } + } else if requests[0].Uploaded { + t.Error("a valid avatar was unnecessarily replaced") + } + if requests[1].Photo != wantPhoto || requests[1].Uploaded || requests[1].Caption != "second" { + t.Errorf("healthy menu was not edited normally: %+v", requests[1]) + } + }) + } +} + +func TestScreenMediaDoesNotRetryUncertainOrForbiddenFailures(t *testing.T) { + for _, code := range []int{403, 429, 500, 0} { + t.Run(fmt.Sprint(code), func(t *testing.T) { + tb, api := newScreenMediaAPI(t) + api.mu.Lock() + api.code, api.badResponse = code, code == 0 + api.mu.Unlock() + tb.renderScreen(1, tb.newScreen("main", "menu", tb.homeRows()...)) + requests := api.snapshot() + if len(requests) != 1 || requests[0].Uploaded { + t.Errorf("non-recoverable failure retried or uploaded art: %+v", requests) + } + if _, ok := tb.screens().get(1); ok { + t.Error("an unconfirmed send was recorded as a live screen") + } + if id, _ := artFileID("main"); id != "rejected-avatar" { + t.Errorf("non-media failure invalidated the avatar: %q", id) + } + }) + } +} + +func TestRejectedQRIsNeverReplacedByDecorativeArt(t *testing.T) { + for _, cached := range []bool{false, true} { + t.Run(fmt.Sprint(cached), func(t *testing.T) { + tb, api := newScreenMediaAPI(t) + api.mu.Lock() + api.failUpload = true + api.mu.Unlock() + sc := tb.newScreen("qr", "QR", tb.backRow()) + sc.qrPicture = qrSheetPicture([]subscriptionQRCode{{kind: "sub", content: "https://example.invalid/sub"}}) + if cached { + sc.photo = "rejected-avatar" + } + tb.renderScreen(1, sc) + requests := api.snapshot() + if len(requests) != 1 || requests[0].UploadName == "screen.png" { + t.Errorf("QR rejection substituted decorative artwork: %+v", requests) + } + if id, _ := artFileID("main"); id != "rejected-avatar" { + t.Errorf("QR failure invalidated unrelated menu artwork: %q", id) + } + }) + } +} + +func TestScreenMediaRecoveryPreservesAConcurrentlyRefreshedAvatar(t *testing.T) { + tb, api := newScreenMediaAPI(t) + started, release := make(chan struct{}, 1), make(chan struct{}) + var once sync.Once + releaseUpload := func() { once.Do(func() { close(release) }) } + t.Cleanup(releaseUpload) + api.mu.Lock() + api.uploadStarted, api.releaseUpload = started, release + api.mu.Unlock() + done := make(chan struct{}) + go func() { + defer close(done) + tb.renderScreen(1, tb.newScreen("main", "menu", tb.homeRows()...)) + }() + select { + case <-started: + case <-time.After(5 * time.Second): + t.Fatal("fallback upload never started") + } + api.mu.Lock() + api.avatar = "new-avatar" + api.mu.Unlock() + tb.resolveScreenArt() + releaseUpload() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("screen recovery never completed") + } + if id, _ := artFileID("main"); id != "new-avatar" { + t.Errorf("late recovery discarded the newly refreshed avatar: %q", id) + } +} diff --git a/internal/web/service/tgbot/screens_art.go b/internal/web/service/tgbot/screens_art.go index 0ae4b5bdc..3312267a3 100644 --- a/internal/web/service/tgbot/screens_art.go +++ b/internal/web/service/tgbot/screens_art.go @@ -28,8 +28,9 @@ var ( artOverrides = map[string]string{} // artFromAvatar records whether the cached ids came from the bot's avatar: a // tile cached from an upload must never win over an avatar set afterwards. - artFromAvatar bool - artProbedAt time.Time + artFromAvatar bool + artProbedAt time.Time + rejectedAvatarID string // artProbeRunning keeps a burst of renders from starting a burst of probes. artProbeRunning atomic.Bool // artProbes lets resetScreenArt outlast a probe still running against a previous bot. @@ -81,12 +82,17 @@ func (t *Tgbot) resolveScreenArt() { // Remember the miss so the next render does not probe again, but keep // any tile already cached: an avatar-less bot still shows a picture. artFromAvatar = false + rejectedAvatarID = "" logger.Debug("Bot has no avatar; screens will use the black fallback") return } // The last entry is the largest size Telegram keeps. sizes := photos.Photos[0] fileID := sizes[len(sizes)-1].FileID + if fileID == rejectedAvatarID { + return + } + rejectedAvatarID = "" screenArtIDs = map[string]string{} for _, kind := range screenKinds { screenArtIDs[kind] = fileID @@ -163,6 +169,28 @@ func cacheArtFileID(kind, fileID string) { screenArtMu.Unlock() } +// Compare before eviction: an asynchronous probe may already have installed a new avatar. +func rejectArtFileID(fileID string) { + screenArtMu.Lock() + defer screenArtMu.Unlock() + for kind, id := range screenArtIDs { + if id == fileID { + delete(screenArtIDs, kind) + if artFromAvatar { + rejectedAvatarID = fileID + } + } + } + if rejectedAvatarID == fileID { + artFromAvatar = false + } + for kind, id := range artOverrides { + if id == fileID { + delete(artOverrides, kind) + } + } +} + // resetScreenArt drops the cached file_ids; tests use it between mock servers. func resetScreenArt() { artProbes.Wait() @@ -170,6 +198,7 @@ func resetScreenArt() { screenArtIDs = map[string]string{} artOverrides = map[string]string{} artFromAvatar = false + rejectedAvatarID = "" artProbedAt = time.Time{} screenArtMu.Unlock() }