mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-16 23:27:14 +00:00
fix(clients): let an explicit 0 actually disable PersistentKeepalive
Addresses review feedback on the previous commit. UpdateInboundClient carries a stored keepalive forward whenever the incoming one is zero, so the settings JSON and the running peer survive a metadata-only edit that omits the field. That was a 0 -> 0 no-op while no UI could set a nonzero value. Now that the client form can, the carry-forward became reachable in the other direction: a client created at the form's default of 25 could never be returned to 0, and the hint text shipped to all 13 locales -- "0 disables it" -- described something the backend silently refused. The save even reported success, because a settings blob that came back byte-identical skips the transaction entirely. The zero value cannot carry that distinction, so model.Client.KeepAlive becomes *int: nil means the field was never sent, &0 means "send no keepalives". The pointer survives the internal marshal in ClientService.Update, which is where an explicit 0 was being erased by omitempty before UpdateInboundClient ever saw it. ClientRecord.KeepAlive stays a plain int -- it is the stored column, where "unset" has no meaning -- and the conversions bridge the two. Two tests, both red before this change in the direction they cover: an explicit 0 must reach wg_keep_alive, and an update that omits the field must still leave a stored 25 alone. Also adds the output transform every other numeric field in the client form already has, so a cleared box sends 0 rather than null.
This commit is contained in:
@@ -583,7 +583,7 @@ func (s *ClientService) AddInboundClient(inboundSvc *InboundService, data *model
|
||||
"publicKey": client.PublicKey,
|
||||
"allowedIPs": client.AllowedIPs,
|
||||
"preSharedKey": client.PreSharedKey,
|
||||
"keepAlive": keepAliveStr(client.KeepAlive),
|
||||
"keepAlive": keepAliveStr(client.KeepAliveSeconds()),
|
||||
})
|
||||
if err1 == nil {
|
||||
logger.Debug("Client added on", rt.Name(), ":", client.Email)
|
||||
@@ -727,7 +727,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
if clients[0].PreSharedKey == "" {
|
||||
clients[0].PreSharedKey = old.PreSharedKey
|
||||
}
|
||||
if clients[0].KeepAlive == 0 {
|
||||
if clients[0].KeepAlive == nil {
|
||||
clients[0].KeepAlive = old.KeepAlive
|
||||
}
|
||||
// ForwardedPorts is AmneziaWG-only (WireGuard's own inbound never
|
||||
@@ -794,8 +794,8 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
if clients[0].PreSharedKey != "" {
|
||||
newMap["preSharedKey"] = clients[0].PreSharedKey
|
||||
}
|
||||
if clients[0].KeepAlive > 0 {
|
||||
newMap["keepAlive"] = clients[0].KeepAlive
|
||||
if ka := clients[0].KeepAliveSeconds(); ka > 0 {
|
||||
newMap["keepAlive"] = ka
|
||||
}
|
||||
if oldInbound.Protocol == model.AmneziaWG && clients[0].ForwardedPorts != "" {
|
||||
newMap["forwardedPorts"] = clients[0].ForwardedPorts
|
||||
@@ -1007,7 +1007,7 @@ func (s *ClientService) UpdateInboundClient(inboundSvc *InboundService, data *mo
|
||||
"publicKey": clients[0].PublicKey,
|
||||
"allowedIPs": clients[0].AllowedIPs,
|
||||
"preSharedKey": clients[0].PreSharedKey,
|
||||
"keepAlive": keepAliveStr(clients[0].KeepAlive),
|
||||
"keepAlive": keepAliveStr(clients[0].KeepAliveSeconds()),
|
||||
})
|
||||
if err1 == nil {
|
||||
logger.Debug("Client edited on", rt.Name(), ":", clients[0].Email)
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
||||
)
|
||||
|
||||
func inboundKeepAlive(t *testing.T, inboundSvc *InboundService, ibId int, email string) int {
|
||||
t.Helper()
|
||||
ib, err := inboundSvc.GetInbound(ibId)
|
||||
if err != nil {
|
||||
t.Fatalf("GetInbound %d: %v", ibId, err)
|
||||
}
|
||||
clients, err := inboundSvc.GetClients(ib)
|
||||
if err != nil {
|
||||
t.Fatalf("GetClients %d: %v", ibId, err)
|
||||
}
|
||||
for i := range clients {
|
||||
if clients[i].Email == email {
|
||||
return clients[i].KeepAliveSeconds()
|
||||
}
|
||||
}
|
||||
t.Fatalf("email %q not found on inbound %d", email, ibId)
|
||||
return 0
|
||||
}
|
||||
|
||||
// seedKeepAliveClient attaches one WireGuard client already carrying a
|
||||
// PersistentKeepalive, and returns its inbound and client-record id.
|
||||
func seedKeepAliveClient(t *testing.T, email string, keepAlive int) (*model.Inbound, int) {
|
||||
t.Helper()
|
||||
svc := &ClientService{}
|
||||
|
||||
seeded := model.Client{
|
||||
Email: email,
|
||||
SubID: "sub-" + email,
|
||||
Enable: true,
|
||||
AllowedIPs: []string{"10.0.0.5/32"},
|
||||
KeepAlive: model.KeepAlivePtr(keepAlive),
|
||||
}
|
||||
ib := mkInbound(t, 51820, model.WireGuard, clientsSettings(t, []model.Client{seeded}))
|
||||
if err := svc.SyncInbound(nil, ib.Id, []model.Client{seeded}); err != nil {
|
||||
t.Fatalf("seed linkage: %v", err)
|
||||
}
|
||||
return ib, lookupClientRecord(t, email).Id
|
||||
}
|
||||
|
||||
// The update path restores the stored keepalive whenever the incoming one is
|
||||
// zero. That was a 0 -> 0 no-op while no UI could set the field; once the
|
||||
// client form could, "0 disables it" became unreachable on an existing client.
|
||||
func TestUpdateCanClearKeepAliveOnAnExistingClient(t *testing.T) {
|
||||
setupBulkDB(t)
|
||||
inboundSvc := &InboundService{}
|
||||
svc := &ClientService{}
|
||||
|
||||
ib, recId := seedKeepAliveClient(t, "ka@x", 25)
|
||||
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 25 {
|
||||
t.Fatalf("seeded keepAlive = %d, want 25", got)
|
||||
}
|
||||
|
||||
updated := model.Client{
|
||||
Email: "ka@x",
|
||||
Enable: true,
|
||||
AllowedIPs: []string{"10.0.0.5/32"},
|
||||
KeepAlive: model.KeepAlivePtr(0),
|
||||
}
|
||||
if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 0 {
|
||||
t.Fatalf("inbound keepAlive after an explicit 0 = %d, want 0", got)
|
||||
}
|
||||
if got := lookupClientRecord(t, "ka@x").KeepAlive; got != 0 {
|
||||
t.Fatalf("stored wg_keep_alive after an explicit 0 = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// The other half of the same contract: a payload that never mentions
|
||||
// keepAlive (a metadata-only edit from the bot or the API) must still leave
|
||||
// the stored value alone.
|
||||
func TestUpdateWithoutKeepAlivePreservesTheStoredValue(t *testing.T) {
|
||||
setupBulkDB(t)
|
||||
inboundSvc := &InboundService{}
|
||||
svc := &ClientService{}
|
||||
|
||||
ib, recId := seedKeepAliveClient(t, "ka@x", 25)
|
||||
|
||||
updated := model.Client{
|
||||
Email: "ka@x",
|
||||
Enable: true,
|
||||
AllowedIPs: []string{"10.0.0.5/32"},
|
||||
}
|
||||
if _, err := svc.Update(inboundSvc, recId, updated, 0); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
if got := inboundKeepAlive(t, inboundSvc, ib.Id, "ka@x"); got != 25 {
|
||||
t.Fatalf("inbound keepAlive after an edit that omitted it = %d, want 25", got)
|
||||
}
|
||||
if got := lookupClientRecord(t, "ka@x").KeepAlive; got != 25 {
|
||||
t.Fatalf("stored wg_keep_alive after an edit that omitted it = %d, want 25", got)
|
||||
}
|
||||
}
|
||||
@@ -249,8 +249,8 @@ func defaultWireguardClients(settingsJSON string, existing, clients []model.Clie
|
||||
if c.PreSharedKey != "" {
|
||||
m["preSharedKey"] = c.PreSharedKey
|
||||
}
|
||||
if c.KeepAlive > 0 {
|
||||
m["keepAlive"] = c.KeepAlive
|
||||
if ka := c.KeepAliveSeconds(); ka > 0 {
|
||||
m["keepAlive"] = ka
|
||||
}
|
||||
interfaceClients[i] = m
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@ func wgPeerList(t *testing.T, settings map[string]any) []map[string]any {
|
||||
|
||||
func TestGetXrayConfigWireGuardPeers(t *testing.T) {
|
||||
clients := []model.Client{
|
||||
{Email: "alice@wg.test", Enable: true, PublicKey: "pub-alice", AllowedIPs: []string{"10.0.0.2/32"}, KeepAlive: 25},
|
||||
{Email: "alice@wg.test", Enable: true, PublicKey: "pub-alice", AllowedIPs: []string{"10.0.0.2/32"}, KeepAlive: model.KeepAlivePtr(25)},
|
||||
{Email: "bob@wg.test", Enable: true, PublicKey: "pub-bob", AllowedIPs: []string{"10.0.0.3/32"}},
|
||||
}
|
||||
seedWGInbound(t, "wg-multi", 51820, clients)
|
||||
|
||||
Reference in New Issue
Block a user