From df7ccd3a6497f053447c6db61727d0f9fdd1cee6 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 3 Jun 2026 13:41:44 +0200 Subject: [PATCH] fix(clients): use client_inbounds link to resolve inbound, not stale id client_traffics.inbound_id is a legacy single-inbound pointer that goes stale when an inbound is deleted and recreated: the email-keyed traffic row survives but references a missing inbound. Code that resolved the owning inbound from it broke several client operations. - adjustTraffics: 'Start After First Use' (negative expiry) never converted to an absolute deadline on first traffic, so the countdown never started. Now resolves inbounds via the client_inbounds link and computes the new expiry once per email so multi-inbound clients stay consistent. - GetClientInboundByEmail / GetClientInboundByTrafficID: fall back to client_inbounds when the pointer is dead, fixing reset traffic ('record not found'), client info, and Telegram set-tgId. - autoRenewClients: resolve renew targets via client_inbounds so scheduled renews are not silently skipped. - clients page: allow resetting a client with no inbound attachment (the backend already zeroes counters by email). Add regression test for the delayed-start conversion under a stale inbound_id. --- frontend/src/pages/clients/ClientsPage.tsx | 2 +- web/service/inbound.go | 209 ++++++++++++++------- web/service/inbound_client_traffic_test.go | 72 +++++++ 3 files changed, 214 insertions(+), 69 deletions(-) diff --git a/frontend/src/pages/clients/ClientsPage.tsx b/frontend/src/pages/clients/ClientsPage.tsx index 44eb4986..6d6740ff 100644 --- a/frontend/src/pages/clients/ClientsPage.tsx +++ b/frontend/src/pages/clients/ClientsPage.tsx @@ -445,7 +445,7 @@ export default function ClientsPage() { } function onResetTraffic(row: ClientRecord) { - if (!row?.email || !Array.isArray(row.inboundIds) || row.inboundIds.length === 0) { + if (!row?.email) { messageApi.warning(t('pages.clients.resetNotPossible')); return; } diff --git a/web/service/inbound.go b/web/service/inbound.go index 0e3263ba..69825678 100644 --- a/web/service/inbound.go +++ b/web/service/inbound.go @@ -1877,71 +1877,101 @@ func (s *InboundService) addClientTraffic(tx *gorm.DB, traffics []*xray.ClientTr } func (s *InboundService) adjustTraffics(tx *gorm.DB, dbClientTraffics []*xray.ClientTraffic) ([]*xray.ClientTraffic, error) { - inboundIds := make([]int, 0, len(dbClientTraffics)) - for _, dbClientTraffic := range dbClientTraffics { - if dbClientTraffic.ExpiryTime < 0 { - inboundIds = append(inboundIds, dbClientTraffic.InboundId) + now := time.Now().UnixMilli() + + // "Start After First Use" stores a negative expiry (the duration). On the + // first traffic tick it becomes an absolute deadline of now+duration. Compute + // it once per email so every inbound the client is attached to lands on the + // same value (recomputing per inbound would skip all but the first one). + newExpiryByEmail := make(map[string]int64, len(dbClientTraffics)) + for traffic_index := range dbClientTraffics { + if dbClientTraffics[traffic_index].ExpiryTime < 0 { + newExpiryByEmail[dbClientTraffics[traffic_index].Email] = now - dbClientTraffics[traffic_index].ExpiryTime + } + } + if len(newExpiryByEmail) == 0 { + return dbClientTraffics, nil + } + + delayedEmails := make([]string, 0, len(newExpiryByEmail)) + for email := range newExpiryByEmail { + delayedEmails = append(delayedEmails, email) + } + + // Resolve the owning inbounds through the client_inbounds link, which is + // authoritative. client_traffics.inbound_id goes stale when an inbound is + // deleted and recreated, which would leave the negative expiry unconverted. + var inboundIds []int + err := tx.Table("client_inbounds"). + Joins("JOIN clients ON clients.id = client_inbounds.client_id"). + Where("clients.email IN (?)", delayedEmails). + Distinct(). + Pluck("client_inbounds.inbound_id", &inboundIds).Error + if err != nil { + return nil, err + } + if len(inboundIds) == 0 { + return dbClientTraffics, nil + } + + var inbounds []*model.Inbound + err = tx.Model(model.Inbound{}).Where("id IN (?)", inboundIds).Find(&inbounds).Error + if err != nil { + return nil, err + } + for inbound_index := range inbounds { + settings := map[string]any{} + json.Unmarshal([]byte(inbounds[inbound_index].Settings), &settings) + clients, ok := settings["clients"].([]any) + if ok { + var newClients []any + for client_index := range clients { + c := clients[client_index].(map[string]any) + email, _ := c["email"].(string) + if newExpiry, ok := newExpiryByEmail[email]; ok { + c["expiryTime"] = newExpiry + c["updated_at"] = now + } + if _, ok := c["created_at"]; !ok { + c["created_at"] = now + } + if _, ok := c["updated_at"]; !ok { + c["updated_at"] = now + } + newClients = append(newClients, any(c)) + } + settings["clients"] = newClients + modifiedSettings, err := json.MarshalIndent(settings, "", " ") + if err != nil { + return nil, err + } + + inbounds[inbound_index].Settings = string(modifiedSettings) } } - if len(inboundIds) > 0 { - var inbounds []*model.Inbound - err := tx.Model(model.Inbound{}).Where("id IN (?)", inboundIds).Find(&inbounds).Error - if err != nil { - return nil, err + for traffic_index := range dbClientTraffics { + if newExpiry, ok := newExpiryByEmail[dbClientTraffics[traffic_index].Email]; ok { + dbClientTraffics[traffic_index].ExpiryTime = newExpiry } - for inbound_index := range inbounds { - settings := map[string]any{} - json.Unmarshal([]byte(inbounds[inbound_index].Settings), &settings) - clients, ok := settings["clients"].([]any) - if ok { - var newClients []any - for client_index := range clients { - c := clients[client_index].(map[string]any) - for traffic_index := range dbClientTraffics { - if dbClientTraffics[traffic_index].ExpiryTime < 0 && c["email"] == dbClientTraffics[traffic_index].Email { - oldExpiryTime := c["expiryTime"].(float64) - newExpiryTime := (time.Now().Unix() * 1000) - int64(oldExpiryTime) - c["expiryTime"] = newExpiryTime - c["updated_at"] = time.Now().Unix() * 1000 - dbClientTraffics[traffic_index].ExpiryTime = newExpiryTime - break - } - } - if _, ok := c["created_at"]; !ok { - c["created_at"] = time.Now().Unix() * 1000 - } - if _, ok := c["updated_at"]; !ok { - c["updated_at"] = time.Now().Unix() * 1000 - } - newClients = append(newClients, any(c)) - } - settings["clients"] = newClients - modifiedSettings, err := json.MarshalIndent(settings, "", " ") - if err != nil { - return nil, err - } + } - inbounds[inbound_index].Settings = string(modifiedSettings) + err = tx.Save(inbounds).Error + if err != nil { + logger.Warning("AddClientTraffic update inbounds ", err) + logger.Error(inbounds) + } else { + for _, ib := range inbounds { + if ib == nil { + continue } - } - err = tx.Save(inbounds).Error - if err != nil { - logger.Warning("AddClientTraffic update inbounds ", err) - logger.Error(inbounds) - } else { - for _, ib := range inbounds { - if ib == nil { - continue - } - cs, gcErr := s.GetClients(ib) - if gcErr != nil { - logger.Warning("AddClientTraffic sync clients: GetClients failed", gcErr) - continue - } - if syncErr := s.clientService.SyncInbound(tx, ib.Id, cs); syncErr != nil { - logger.Warning("AddClientTraffic sync clients: SyncInbound failed", syncErr) - } + cs, gcErr := s.GetClients(ib) + if gcErr != nil { + logger.Warning("AddClientTraffic sync clients: GetClients failed", gcErr) + continue + } + if syncErr := s.clientService.SyncInbound(tx, ib.Id, cs); syncErr != nil { + logger.Warning("AddClientTraffic sync clients: SyncInbound failed", syncErr) } } } @@ -1976,8 +2006,23 @@ func (s *InboundService) autoRenewClients(tx *gorm.DB) (bool, int64, error) { client map[string]any } + // Resolve the inbounds to renew through the client_inbounds link rather than + // client_traffics.inbound_id, which goes stale after an inbound is deleted and + // recreated and would otherwise skip the renew entirely. + renewEmails := make([]string, 0, len(traffics)) for _, traffic := range traffics { - inbound_ids = append(inbound_ids, traffic.InboundId) + renewEmails = append(renewEmails, traffic.Email) + } + for _, batch := range chunkStrings(renewEmails, sqliteMaxVars) { + var ids []int + if err = tx.Table("client_inbounds"). + Joins("JOIN clients ON clients.id = client_inbounds.client_id"). + Where("clients.email IN ?", batch). + Distinct(). + Pluck("client_inbounds.inbound_id", &ids).Error; err != nil { + return false, 0, err + } + inbound_ids = append(inbound_ids, ids...) } // Dedupe so an inbound hosting N expired clients is fetched and saved once // per tick instead of N times across chunk boundaries. @@ -2401,11 +2446,24 @@ func (s *InboundService) GetClientInboundByTrafficID(trafficId int) (traffic *xr logger.Warningf("Error retrieving ClientTraffic with trafficId %d: %v", trafficId, err) return nil, nil, err } - if len(traffics) > 0 { - inbound, err = s.GetInbound(traffics[0].InboundId) - return traffics[0], inbound, err + if len(traffics) == 0 { + return nil, nil, nil } - return nil, nil, nil + traffic = traffics[0] + + inbound, err = s.GetInbound(traffic.InboundId) + if errors.Is(err, gorm.ErrRecordNotFound) { + // client_traffics.inbound_id goes stale when an inbound is deleted and + // recreated; fall back to the authoritative client_inbounds link by email. + ids, idErr := s.clientService.GetInboundIdsForEmail(db, traffic.Email) + if idErr != nil { + return traffic, nil, idErr + } + if len(ids) > 0 { + inbound, err = s.GetInbound(ids[0]) + } + } + return traffic, inbound, err } func (s *InboundService) GetClientInboundByEmail(email string) (traffic *xray.ClientTraffic, inbound *model.Inbound, err error) { @@ -2416,11 +2474,26 @@ func (s *InboundService) GetClientInboundByEmail(email string) (traffic *xray.Cl logger.Warningf("Error retrieving ClientTraffic with email %s: %v", email, err) return nil, nil, err } - if len(traffics) > 0 { - inbound, err = s.GetInbound(traffics[0].InboundId) - return traffics[0], inbound, err + if len(traffics) == 0 { + return nil, nil, nil } - return nil, nil, nil + traffic = traffics[0] + + inbound, err = s.GetInbound(traffic.InboundId) + if errors.Is(err, gorm.ErrRecordNotFound) { + // client_traffics.inbound_id is a legacy single-inbound pointer that goes + // stale when an inbound is deleted and recreated: the email-keyed traffic + // row survives but still references the missing inbound. Fall back to the + // authoritative client_inbounds link so email lookups (reset, info, …) work. + ids, idErr := s.clientService.GetInboundIdsForEmail(db, email) + if idErr != nil { + return traffic, nil, idErr + } + if len(ids) > 0 { + inbound, err = s.GetInbound(ids[0]) + } + } + return traffic, inbound, err } func (s *InboundService) GetClientByEmail(clientEmail string) (*xray.ClientTraffic, *model.Client, error) { diff --git a/web/service/inbound_client_traffic_test.go b/web/service/inbound_client_traffic_test.go index 41eb266c..f75c3f8a 100644 --- a/web/service/inbound_client_traffic_test.go +++ b/web/service/inbound_client_traffic_test.go @@ -3,6 +3,7 @@ package service import ( "path/filepath" "testing" + "time" "github.com/mhsanaei/3x-ui/v3/database" "github.com/mhsanaei/3x-ui/v3/database/model" @@ -76,3 +77,74 @@ func TestAddClientTraffic_MatchesDespiteStaleInboundId(t *testing.T) { t.Errorf("node-owned row should not be touched by local traffic: up=%d down=%d, want 0/0", node.Up, node.Down) } } + +// TestAdjustTraffics_DelayedStartConvertsDespiteStaleInboundId covers "Start After +// First Use": a delayed-start client carries a negative expiry (the duration) that +// must convert to an absolute deadline on its first traffic tick. When the client's +// email-keyed client_traffics row still points at a deleted inbound (stale inbound_id +// after an inbound delete+recreate), the conversion used to resolve no inbound and +// silently skip, leaving the client perpetually "not started". The fix resolves the +// owning inbound via the client_inbounds link instead. +func TestAdjustTraffics_DelayedStartConvertsDespiteStaleInboundId(t *testing.T) { + dbDir := t.TempDir() + t.Setenv("XUI_DB_FOLDER", dbDir) + if err := database.InitDB(filepath.Join(dbDir, "x-ui.db")); err != nil { + t.Fatalf("InitDB: %v", err) + } + t.Cleanup(func() { _ = database.CloseDB() }) + + db := database.GetDB() + + const email = "delayed-user" + const uid = "ce8d33df-3a64-4f10-8f9b-91c3a8e0d001" + const sevenDays = int64(7 * 86400000) + + client := model.Client{Email: email, ID: uid, Auth: uid, Enable: true, ExpiryTime: -sevenDays} + inbound := &model.Inbound{ + Tag: "vless-delayed", Enable: true, Port: 45001, Protocol: model.VLESS, + StreamSettings: `{"network":"tcp","security":"reality"}`, + Settings: clientsSettings(t, []model.Client{client}), + } + if err := db.Create(inbound).Error; err != nil { + t.Fatalf("create inbound: %v", err) + } + + svc := InboundService{} + if err := svc.clientService.SyncInbound(db, inbound.Id, []model.Client{client}); err != nil { + t.Fatalf("SyncInbound: %v", err) + } + + // The email-keyed traffic row survives an inbound delete+recreate pointing at a + // dead inbound id; client_inbounds still links the client to the live inbound. + if err := db.Create(&xray.ClientTraffic{InboundId: 9999, Email: email, Enable: true, ExpiryTime: -sevenDays}).Error; err != nil { + t.Fatalf("create stale traffic row: %v", err) + } + + before := time.Now().UnixMilli() + if err := svc.addClientTraffic(db, []*xray.ClientTraffic{{Email: email, Up: 100, Down: 200}}); err != nil { + t.Fatalf("addClientTraffic: %v", err) + } + + var row xray.ClientTraffic + if err := db.Model(xray.ClientTraffic{}).Where("email = ?", email).First(&row).Error; err != nil { + t.Fatalf("reload traffic row: %v", err) + } + if row.ExpiryTime <= 0 { + t.Fatalf("delayed-start expiry not converted: still %d (stale inbound_id skipped the conversion)", row.ExpiryTime) + } + if row.ExpiryTime < before+sevenDays-5000 || row.ExpiryTime > before+sevenDays+5000 { + t.Errorf("converted expiry = %d, want ~now+7d (%d)", row.ExpiryTime, before+sevenDays) + } + + reloaded, err := svc.GetInbound(inbound.Id) + if err != nil { + t.Fatalf("GetInbound: %v", err) + } + cs, err := svc.GetClients(reloaded) + if err != nil { + t.Fatalf("GetClients: %v", err) + } + if len(cs) != 1 || cs[0].ExpiryTime <= 0 { + t.Errorf("inbound settings expiry not converted: %#v", cs) + } +}