Browse Source

fix(xray): apply shadowsocks client changes in place instead of rebuilding

The per-user hot path only covered vless, vmess, trojan, hysteria and
masque, so every client change on a multi-user shadowsocks inbound - a
node push for an auto-disable, renew, reset or add, a local Edit Inbound
save, or a config-diff hot apply - fell through to DelInbound +
AddInbound, closing the listener and dropping every live connection on
the port.

Shadowsocks joins userDiffableProtocols. The emitted user ops also carry
the inbound's method as "cipher": 2022 clients have no method of their
own, and without it AddUser cannot build the account, which would leave
2022 inbounds rebuilding anyway. Changes outside settings.clients
(method, 2022 server key, network) still differ in the rest of the
settings and still rebuild.

Verified against the pinned core: TestXrayAPI_E2E_Users and the
shadowsocks e2e tests pass with XRAY_E2E_BINARY set.

Closes #6820
MHSanaei 13 hours ago
parent
commit
a66b360c09

+ 74 - 0
internal/web/service/inbound_update_hot_users_test.go

@@ -2,6 +2,7 @@ package service
 
 
 import (
 import (
 	"context"
 	"context"
+	"encoding/json"
 	"errors"
 	"errors"
 	"slices"
 	"slices"
 	"strings"
 	"strings"
@@ -207,3 +208,76 @@ func TestNodeInboundUnchangedPushLeavesTheCoreAlone(t *testing.T) {
 		t.Fatalf("core operations = %q, want none for an unchanged inbound", got)
 		t.Fatalf("core operations = %q, want none for an unchanged inbound", got)
 	}
 	}
 }
 }
+
+func ssHotSettings(t *testing.T, method, serverKey string, clients []model.Client) string {
+	t.Helper()
+	var settings map[string]any
+	if err := json.Unmarshal([]byte(clientsSettings(t, clients)), &settings); err != nil {
+		t.Fatalf("unmarshal settings: %v", err)
+	}
+	settings["method"] = method
+	settings["network"] = "tcp,udp"
+	if serverKey != "" {
+		settings["password"] = serverKey
+	} else {
+		for _, c := range settings["clients"].([]any) {
+			c.(map[string]any)["method"] = method
+		}
+	}
+	out, err := json.Marshal(settings)
+	if err != nil {
+		t.Fatalf("marshal settings: %v", err)
+	}
+	return string(out)
+}
+
+// Multi-user shadowsocks has the same per-user API as vless, so a pushed client
+// change must not rebuild the handler and drop every connection on the port.
+func TestNodeShadowsocksPushChangingOnlyClientsKeepsTheHandler(t *testing.T) {
+	ciphers := []struct{ name, method, serverKey, keyA, keyB string }{
+		{"legacy aead", "aes-256-gcm", "", "a-pass", "b-pass"},
+		{"2022", "2022-blake3-aes-128-gcm", "AAECAwQFBgcICQoLDA0ODw==", "EBESExQVFhcYGRobHB0eHw==", "ICEiIyQlJicoKSorLC0uLw=="},
+	}
+	for _, cipher := range ciphers {
+		a := model.Client{Email: "ss-a", Password: cipher.keyA, Enable: true, CreatedAt: 1, UpdatedAt: 1}
+		b := model.Client{Email: "ss-b", Password: cipher.keyB, Enable: true, CreatedAt: 1, UpdatedAt: 1}
+		bOff := b
+		bOff.Enable = false
+		cases := []struct {
+			name           string
+			stored, pushed []model.Client
+			wantOps        []string
+		}{
+			{"disabling one client removes only that user", []model.Client{a, b}, []model.Client{a, bOff}, []string{"remove-user ss-b"}},
+			{"renewing a disabled client adds only that user", []model.Client{a, bOff}, []model.Client{a, b}, []string{"add-user ss-b"}},
+		}
+		for _, tc := range cases {
+			t.Run(cipher.name+"/"+tc.name, func(t *testing.T) {
+				setupConflictDB(t)
+				setRestartOnClientDisable(t, true)
+				probe := &inboundOpsProbe{}
+				useTestRuntimeManager(t).SetLocalRuntimeOverride(probe)
+
+				seedInboundConflict(t, "hot-ss", "0.0.0.0", 50083, model.Shadowsocks, `{"network":"tcp"}`,
+					ssHotSettings(t, cipher.method, cipher.serverKey, tc.stored))
+				stored := loadInboundByTag(t, "hot-ss")
+				if err := (&ClientService{}).SyncInbound(nil, stored.Id, tc.stored); err != nil {
+					t.Fatalf("SyncInbound: %v", err)
+				}
+
+				pushed := *stored
+				pushed.Settings = ssHotSettings(t, cipher.method, cipher.serverKey, tc.pushed)
+				_, needRestart, err := (&InboundService{FromNodeSync: true}).UpdateInbound(&pushed)
+				if err != nil {
+					t.Fatalf("UpdateInbound: %v", err)
+				}
+				if got := probe.recorded(); !slices.Equal(got, tc.wantOps) {
+					t.Fatalf("core operations = %q, want %q", got, tc.wantOps)
+				}
+				if needRestart {
+					t.Fatal("needRestart = true, want the push applied without restarting the core")
+				}
+			})
+		}
+	}
+}

+ 29 - 3
internal/xray/hot_diff.go

@@ -3,6 +3,7 @@ package xray
 import (
 import (
 	"bytes"
 	"bytes"
 	"encoding/json"
 	"encoding/json"
+	"maps"
 
 
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
 	"github.com/mhsanaei/3x-ui/v3/internal/util/json_util"
 	"github.com/mhsanaei/3x-ui/v3/internal/util/json_util"
@@ -234,7 +235,7 @@ func InboundUserOps(oldIb, newIb *InboundConfig) (*HotDiff, bool) {
 	return diff, true
 	return diff, true
 }
 }
 
 
-var userDiffableProtocols = map[string]struct{}{"vless": {}, "vmess": {}, "trojan": {}, "hysteria": {}, "masque": {}}
+var userDiffableProtocols = map[string]struct{}{"vless": {}, "vmess": {}, "trojan": {}, "hysteria": {}, "masque": {}, "shadowsocks": {}}
 
 
 // diffInboundUsers emits per-user AlterInbound ops when two same-tag inbounds
 // diffInboundUsers emits per-user AlterInbound ops when two same-tag inbounds
 // differ only in settings.clients, so the handler (and its listener) survives.
 // differ only in settings.clients, so the handler (and its listener) survives.
@@ -261,6 +262,10 @@ func diffInboundUsers(oldIb, newIb *InboundConfig, diff *HotDiff) bool {
 	if !bytes.Equal(oldRest, newRest) {
 	if !bytes.Equal(oldRest, newRest) {
 		return false
 		return false
 	}
 	}
+	cipher := ""
+	if newIb.Protocol == "shadowsocks" {
+		cipher = shadowsocksSettingsMethod(newRest)
+	}
 	for email, oldC := range oldClients {
 	for email, oldC := range oldClients {
 		newC, exists := newClients[email]
 		newC, exists := newClients[email]
 		if exists && bytes.Equal(oldC.norm, newC.norm) {
 		if exists && bytes.Equal(oldC.norm, newC.norm) {
@@ -268,17 +273,38 @@ func diffInboundUsers(oldIb, newIb *InboundConfig, diff *HotDiff) bool {
 		}
 		}
 		diff.RemovedUsers = append(diff.RemovedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email})
 		diff.RemovedUsers = append(diff.RemovedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email})
 		if exists {
 		if exists {
-			diff.AddedUsers = append(diff.AddedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email, User: newC.user})
+			diff.AddedUsers = append(diff.AddedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email, User: userWithCipher(newC.user, cipher)})
 		}
 		}
 	}
 	}
 	for email, newC := range newClients {
 	for email, newC := range newClients {
 		if _, exists := oldClients[email]; !exists {
 		if _, exists := oldClients[email]; !exists {
-			diff.AddedUsers = append(diff.AddedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email, User: newC.user})
+			diff.AddedUsers = append(diff.AddedUsers, UserOp{Tag: oldIb.Tag, Protocol: oldIb.Protocol, Email: email, User: userWithCipher(newC.user, cipher)})
 		}
 		}
 	}
 	}
 	return true
 	return true
 }
 }
 
 
+// shadowsocksSettingsMethod reads the inbound-level method: 2022 clients carry
+// none, and AddUser needs it to pick the account type the running inbound expects.
+func shadowsocksSettingsMethod(rest []byte) string {
+	var settings struct {
+		Method string `json:"method"`
+	}
+	if json.Unmarshal(rest, &settings) != nil {
+		return ""
+	}
+	return settings.Method
+}
+
+func userWithCipher(user map[string]any, cipher string) map[string]any {
+	if cipher == "" {
+		return user
+	}
+	withCipher := maps.Clone(user)
+	withCipher["cipher"] = cipher
+	return withCipher
+}
+
 type clientEntry struct {
 type clientEntry struct {
 	user map[string]any
 	user map[string]any
 	norm []byte
 	norm []byte

+ 42 - 1
internal/xray/hot_diff_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/internal/util/json_util"
 	"github.com/mhsanaei/3x-ui/v3/internal/util/json_util"
 
 
 	"github.com/op/go-logging"
 	"github.com/op/go-logging"
+	"github.com/xtls/xray-core/proxy/shadowsocks"
 )
 )
 
 
 func TestMain(m *testing.M) {
 func TestMain(m *testing.M) {
@@ -242,7 +243,7 @@ func TestComputeHotDiff_ClientChangeFallsBackToReplace(t *testing.T) {
 		{
 		{
 			name: "unsupported protocol",
 			name: "unsupported protocol",
 			mutate: func(cfg *Config) {
 			mutate: func(cfg *Config) {
-				cfg.InboundConfigs[1].Protocol = "shadowsocks"
+				cfg.InboundConfigs[1].Protocol = "http"
 			},
 			},
 		},
 		},
 		{
 		{
@@ -559,3 +560,43 @@ func TestComputeHotDiff_HysteriaClientOnlyChangeKeepsListener(t *testing.T) {
 		t.Fatalf("expected a single AddUser op for b, got %+v", diff)
 		t.Fatalf("expected a single AddUser op for b, got %+v", diff)
 	}
 	}
 }
 }
+
+// 2022 clients carry no method of their own, so a user op built from the client
+// object alone has no cipher and its account cannot be built for the core.
+func TestInboundUserOps_ShadowsocksAddedUserBuildsTheInboundAccount(t *testing.T) {
+	cases := []struct {
+		name, settingsHead, clientMethod string
+		wantLegacy                       bool
+	}{
+		{"legacy aead", `"method":"aes-256-gcm"`, `"method":"aes-256-gcm",`, true},
+		{"2022", `"method":"2022-blake3-aes-128-gcm","password":"AAECAwQFBgcICQoLDA0ODw=="`, "", false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			inbound := func(clients ...string) *InboundConfig {
+				return &InboundConfig{
+					Port: 8388, Protocol: "shadowsocks", Tag: "inbound-8388",
+					Listen:   json_util.RawMessage(`"0.0.0.0"`),
+					Settings: json_util.RawMessage(`{` + tc.settingsHead + `,"network":"tcp,udp","clients":[` + strings.Join(clients, ",") + `]}`),
+				}
+			}
+			clientA := `{` + tc.clientMethod + `"email":"a","password":"EBESExQVFhcYGRobHB0eHw=="}`
+			clientB := `{` + tc.clientMethod + `"email":"b","password":"ICEiIyQlJicoKSorLC0uLw=="}`
+
+			diff, ok := InboundUserOps(inbound(clientA), inbound(clientA, clientB))
+			if !ok {
+				t.Fatal("a client-only shadowsocks change must apply through per-user ops")
+			}
+			if len(diff.RemovedUsers) != 0 || len(diff.AddedUsers) != 1 || diff.AddedUsers[0].Email != "b" {
+				t.Fatalf("expected only user b added, got added %+v removed %+v", diff.AddedUsers, diff.RemovedUsers)
+			}
+			_, legacy, modern := decodeSSAccount(t, diff.AddedUsers[0].User)
+			if tc.wantLegacy && (legacy == nil || legacy.CipherType != shadowsocks.CipherType_AES_256_GCM) {
+				t.Fatalf("want a legacy aes-256-gcm account, got legacy %v modern %v", legacy, modern)
+			}
+			if !tc.wantLegacy && (modern == nil || modern.Key != "ICEiIyQlJicoKSorLC0uLw==") {
+				t.Fatalf("want a 2022 account keyed by b's password, got legacy %v modern %v", legacy, modern)
+			}
+		})
+	}
+}