diff --git a/internal/web/service/client_bulk.go b/internal/web/service/client_bulk.go index 89b241dc8..f6fe94586 100644 --- a/internal/web/service/client_bulk.go +++ b/internal/web/service/client_bulk.go @@ -30,6 +30,7 @@ type BulkAttachResult struct { // all clients to a target in a single AddInboundClient call, and reports clients already // present on a target as skipped. func (s *ClientService) BulkAttach(inboundSvc *InboundService, emails []string, inboundIds []int) (*BulkAttachResult, bool, error) { + inboundIds = uniqueInts(inboundIds) result := &BulkAttachResult{} if len(emails) == 0 || len(inboundIds) == 0 { return result, false, nil @@ -1357,7 +1358,7 @@ func (s *ClientService) bulkCreate(inboundSvc *InboundService, payloads []Client seenEmail[le] = struct{}{} seenSubID[client.SubID] = le - prep = append(prep, prepared{client: client, inboundIds: payloads[i].InboundIds, limitHwid: payloads[i].LimitHwid, payloadIdx: i}) + prep = append(prep, prepared{client: client, inboundIds: uniqueInts(payloads[i].InboundIds), limitHwid: payloads[i].LimitHwid, payloadIdx: i}) emails = append(emails, email) subIDs = append(subIDs, client.SubID) } diff --git a/internal/web/service/client_create_fanout_test.go b/internal/web/service/client_create_fanout_test.go index 1d3b63d52..1d7dc57bc 100644 --- a/internal/web/service/client_create_fanout_test.go +++ b/internal/web/service/client_create_fanout_test.go @@ -3,6 +3,8 @@ package service import ( "context" "fmt" + goruntime "runtime" + "slices" "strings" "sync/atomic" "testing" @@ -338,3 +340,54 @@ func TestFanoutReportsEveryFailingInbound(t *testing.T) { } }) } + +// An inbound id costs two bytes of request body, so a repeated one must not cost +// the panel another copy of the client: 2000 ids x 8 MB of comment was 16 GB. +func TestRepeatedInboundIdCostsOneCopyOfTheClient(t *testing.T) { + const commentBytes = 1 << 20 + client := func(email string) model.Client { + return model.Client{Email: email, Comment: strings.Repeat("x", commentBytes), Enable: true} + } + + cases := map[string]func(svc *ClientService, inboundSvc *InboundService, spare int, ids []int) error{ + "Create": func(svc *ClientService, inboundSvc *InboundService, _ int, ids []int) error { + _, err := svc.Create(inboundSvc, &ClientCreatePayload{Client: client("repeated@x"), InboundIds: ids}) + return err + }, + "BulkCreate": func(svc *ClientService, inboundSvc *InboundService, _ int, ids []int) error { + res, _, err := svc.BulkCreate(inboundSvc, []ClientCreatePayload{{Client: client("repeated@x"), InboundIds: ids}}) + if err == nil && res.Created != 1 { + t.Fatalf("bulk create: created %d, skipped %+v", res.Created, res.Skipped) + } + return err + }, + "Attach": func(svc *ClientService, inboundSvc *InboundService, spare int, ids []int) error { + if _, err := svc.CreateOne(inboundSvc, spare, client("repeated@x")); err != nil { + return err + } + _, err := svc.AttachByEmail(inboundSvc, "repeated@x", ids) + return err + }, + } + for name, run := range cases { + t.Run(name, func(t *testing.T) { + setupBulkDB(t) + ib := mkInbound(t, 21011, model.VLESS, `{"clients":[]}`) + spare := mkInbound(t, 21012, model.VLESS, `{"clients":[]}`) + + var before, after goruntime.MemStats + goruntime.ReadMemStats(&before) + if err := run(&ClientService{}, &InboundService{}, spare.Id, slices.Repeat([]int{ib.Id}, 300)); err != nil { + t.Fatal(err) + } + goruntime.ReadMemStats(&after) + + if allocated := after.TotalAlloc - before.TotalAlloc; allocated > 100*commentBytes { + t.Fatalf("allocated %d MiB for a 1 MiB client named on one inbound", allocated>>20) + } + if emails := settingsClientEmails(t, ib.Id); len(emails) != 1 { + t.Fatalf("clients on the inbound = %v, want the one", emails) + } + }) + } +} diff --git a/internal/web/service/client_crud.go b/internal/web/service/client_crud.go index eb1135b9a..904f93037 100644 --- a/internal/web/service/client_crud.go +++ b/internal/web/service/client_crud.go @@ -191,6 +191,8 @@ func (s *ClientService) Create(inboundSvc *InboundService, payload *ClientCreate if payload == nil { return false, common.NewError("empty payload") } + // A repeated id adds nothing but another copy of the client held through the fan-out. + payload.InboundIds = uniqueInts(payload.InboundIds) client := payload.Client if strings.TrimSpace(client.Email) == "" { return false, common.NewError("client email is required") @@ -1138,6 +1140,7 @@ func addressesFitAmneziaWGInbound(addrs []string, ib *model.Inbound) bool { // Attach applies the client to every requested inbound: one failing inbound no // longer aborts the others, so the error can name several and needRestart holds. func (s *ClientService) Attach(inboundSvc *InboundService, id int, inboundIds []int) (bool, error) { + inboundIds = uniqueInts(inboundIds) existing, err := s.GetByID(id) if err != nil { return false, err diff --git a/internal/web/service/inbound_node.go b/internal/web/service/inbound_node.go index 986e16e2f..ba846a053 100644 --- a/internal/web/service/inbound_node.go +++ b/internal/web/service/inbound_node.go @@ -84,7 +84,7 @@ func (s *InboundService) AnyNodePending(inboundIds []int) bool { return false } nodeSvc := NodeService{} - for _, id := range inboundIds { + for _, id := range uniqueInts(inboundIds) { ib, err := s.GetInbound(id) if err != nil || ib.NodeID == nil { continue