From f67111a08a20b64d0217d912613e4c80400ee4a8 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Sat, 31 Oct 2026 10:43:40 +0100 Subject: [PATCH] fix(traffic): keep enforcing limits when one inbound's settings are broken Disabling depleted clients rewrote each affected inbound's settings and aborted the whole lifecycle tick when one blob did not parse, so a node storing a malformed blob on the master switched expiry and quota checks off for every inbound. Unparseable node settings are now treated like a snapshot that drops the hub's clients (the hub keeps its copy), and an unparseable stored blob is left as it is while the client's rows are still disabled; the rewrite also works one level deep like the others. --- internal/web/service/global_traffic_test.go | 33 ++++++++++++++ internal/web/service/inbound_disable.go | 45 ++++++++++++------- internal/web/service/inbound_node.go | 4 +- internal/web/service/node_sweep_guard_test.go | 21 +++++++++ 4 files changed, 85 insertions(+), 18 deletions(-) diff --git a/internal/web/service/global_traffic_test.go b/internal/web/service/global_traffic_test.go index 8412f0768..b195f7cdd 100644 --- a/internal/web/service/global_traffic_test.go +++ b/internal/web/service/global_traffic_test.go @@ -322,3 +322,36 @@ func TestAutoRenewClientsPastTheSQLVariableLimit(t *testing.T) { t.Fatalf("autoRenewClients = %d, %v; want all %d renewed", count, err, n) } } + +// One inbound whose stored settings do not parse (a node can send such a blob) must not +// stop the master disabling depleted clients everywhere else. +func TestDisableInvalidClientsSurvivesAnInboundWithBrokenSettings(t *testing.T) { + db := initTrafficTestDB(t) + for _, ib := range []*model.Inbound{ + {UserId: 1, Tag: "broken", Port: 41201, Protocol: model.VLESS, Enable: true, Settings: `{"clients":[`}, + {UserId: 1, Tag: "local", Port: 41202, Protocol: model.VLESS, Enable: true, Settings: `{"clients":[{"email":"bob","enable":true}]}`}, + } { + if err := db.Create(ib).Error; err != nil { + t.Fatalf("seed inbound: %v", err) + } + } + for i, email := range []string{"alice", "bob"} { + seedClientRow(t, email, i+1, 10, 10, 5) + rec := &model.ClientRecord{Email: email, Enable: true, SubID: "s-" + email} + if err := db.Create(rec).Error; err != nil { + t.Fatalf("seed client: %v", err) + } + if err := db.Create(&model.ClientInbound{ClientId: rec.Id, InboundId: i + 1}).Error; err != nil { + t.Fatalf("link client: %v", err) + } + } + _, count, _, err := (&InboundService{}).disableInvalidClients(db, newTrafficMutationBatch()) + if err != nil || count != 2 { + t.Fatalf("disableInvalidClients = %d, %v; want both depleted clients disabled", count, err) + } + var bob model.Inbound + db.Where("tag = ?", "local").First(&bob) + if !strings.Contains(bob.Settings, `"enable":false`) { + t.Fatalf("local inbound settings = %s, want bob disabled", bob.Settings) + } +} diff --git a/internal/web/service/inbound_disable.go b/internal/web/service/inbound_disable.go index c29b68fda..45e2632f1 100644 --- a/internal/web/service/inbound_disable.go +++ b/internal/web/service/inbound_disable.go @@ -3,9 +3,11 @@ package service import ( "encoding/json" "slices" + "strconv" "time" "github.com/mhsanaei/3x-ui/v3/internal/database/model" + "github.com/mhsanaei/3x-ui/v3/internal/logger" "github.com/mhsanaei/3x-ui/v3/internal/xray" "gorm.io/gorm" @@ -197,39 +199,48 @@ func (s *InboundService) markClientsDisabledInSettings(tx *gorm.DB, inboundID in } snapshot := ib - settings := map[string]any{} - if err := json.Unmarshal([]byte(ib.Settings), &settings); err != nil { - return nil, nil, err + // A blob that does not parse (a node can send one) leaves the settings as they are: the + // stats and client rows still disable the client, and one inbound must not stop the tick. + members, entries, ok := splitSettingsClients(ib.Settings) + if !ok { + logger.Warningf("disable clients: inbound %d settings do not parse; leaving them unchanged", inboundID) + return &snapshot, &ib, nil } - clients, _ := settings["clients"].([]any) now := time.Now().Unix() * 1000 mutated := false - for i := range clients { - entry, ok := clients[i].(map[string]any) + for i, entry := range entries { + c, ok := readSettingsClient(entry) if !ok { continue } - email, _ := entry["email"].(string) - if _, hit := emails[email]; !hit { + if _, hit := emails[c.Email]; !hit { continue } - if cur, _ := entry["enable"].(bool); !cur { + var enabled bool + if json.Unmarshal(c.Enable, &enabled) != nil || !enabled { continue } - entry["enable"] = false - entry["updated_at"] = now - clients[i] = entry + var fields map[string]json.RawMessage + if json.Unmarshal(entry, &fields) != nil { + continue + } + fields["enable"] = json.RawMessage("false") + fields["updated_at"] = json.RawMessage(strconv.FormatInt(now, 10)) + rewritten, err := json.Marshal(fields) + if err != nil { + continue + } + entries[i] = rewritten mutated = true } if !mutated { return &snapshot, &ib, nil } - settings["clients"] = clients - bs, marshalErr := json.MarshalIndent(settings, "", " ") - if marshalErr != nil { - return nil, nil, marshalErr + joined, ok := joinSettingsClients(ib.Settings, members, entries) + if !ok { + return &snapshot, &ib, nil } - ib.Settings = string(bs) + ib.Settings = joined if err := tx.Model(&model.Inbound{}).Where("id = ?", inboundID). Update("settings", ib.Settings).Error; err != nil { return nil, nil, err diff --git a/internal/web/service/inbound_node.go b/internal/web/service/inbound_node.go index e0839f905..1b888a0e8 100644 --- a/internal/web/service/inbound_node.go +++ b/internal/web/service/inbound_node.go @@ -468,7 +468,9 @@ func adoptedWireInbound(c, snapIb *model.Inbound, adoptedSettings string) *model // still links some: a reset or half-started node, never an authoritative removal. func snapshotDropsEveryHubClient(tx *gorm.DB, inboundID int, wireSettings string) bool { clients, err := ParseInboundSettingsClients(wireSettings) - if err != nil || len(clients) > 0 { + // Settings that do not parse list no client the hub could keep either; storing them + // would only hand every later reader of the row a blob it cannot use. + if err == nil && len(clients) > 0 { return false } var links int64 diff --git a/internal/web/service/node_sweep_guard_test.go b/internal/web/service/node_sweep_guard_test.go index f006ad748..74640ef6f 100644 --- a/internal/web/service/node_sweep_guard_test.go +++ b/internal/web/service/node_sweep_guard_test.go @@ -213,3 +213,24 @@ func TestRepeatedSnapshotEntriesMergeTheInboundOnce(t *testing.T) { t.Fatalf("remarks = %v, want the first entry merged once", remarks) } } + +// Settings a node sends that do not parse must not replace the hub's copy: every later +// reader of the row (expiry, quota, renewals, subscriptions) would get a blob it cannot use. +func TestUnparseableSnapshotSettingsKeepTheHubCopy(t *testing.T) { + db := initTrafficTestDB(t) + svc := &InboundService{} + seedNodeRow(t, db, &model.Node{Id: 1, Name: "n1", Address: "127.0.0.1", Port: 2096, ApiToken: "tok", Enable: true}) + createNodeInboundWithClient(t, db, 1, "keep", 41301, "kept@x") + good := `{"clients":[{"email":"kept@x","enable":true}]}` + if _, err := svc.setRemoteTrafficLocked(1, snapshotWithClients(t, "keep", good, xray.ClientTraffic{Email: "kept@x", Enable: true}), false, false); err != nil { + t.Fatalf("seed sync: %v", err) + } + if _, err := svc.setRemoteTrafficLocked(1, snapshotWithClients(t, "keep", `{"clients":[`, xray.ClientTraffic{Email: "kept@x", Enable: true}), false, false); err != nil { + t.Fatalf("broken sync: %v", err) + } + var ib model.Inbound + db.Where("tag = ?", "keep").First(&ib) + if _, err := ParseInboundSettingsClients(ib.Settings); err != nil { + t.Fatalf("stored settings %q no longer parse: %v", ib.Settings, err) + } +}