Browse Source

fix(sub): drop external Clash shadowsocks nodes the panel cannot express (#6508)

* fix(sub): gate external Clash shadowsocks links like the inbound path

clashProxyFromExternal returned as soon as it had built the ss proxy, so an
ss:// link skipped applyTransport/applySecurity: a node whose tcp/http
obfuscation Clash cannot express was emitted anyway (mihomo then opens a plain
shadowsocks stream at a server that requires the header, and the node silently
never connects), and security=tls was silently stripped. The inbound path runs
both helpers for every protocol, so the two Clash importers disagreed about the
same node.

* fix(sub): count a dropped external link in the quota header

The client email that feeds AggregateTrafficByEmails was recorded only when a
proxy came out of the link, so a node Clash cannot represent also vanished from
the Subscription-Userinfo header of every other node in the same subscription —
the header reported another client's numbers as the whole subscription's. The
inactive-link branch already counted an email without a proxy; make that
unconditional so the header describes the subscribers, not the representable
subset of their nodes.

* docs(sub): describe clashProxyFromExternal by what it does, not by protocol

The protocol list in the doc comment went stale the moment the shadowsocks
branch stopped returning early, and it restated what the switch already says.
BlindMaster24 7 giờ trước cách đây
mục cha
commit
ff1a6c3caf

+ 4 - 6
internal/sub/clash_external.go

@@ -6,11 +6,8 @@ import (
 	"strings"
 )
 
-// clashProxyFromExternal parses a pasted share link and converts it into a
-// mihomo/Clash proxy entry named `name`. Returns nil for links Clash can't
-// represent (the entry is then skipped, mirroring how getProxies drops
-// unsupported inbound protocols). vmess/vless/trojan reuse the existing
-// applyTransport/applySecurity helpers; ss/hysteria2/wireguard map directly.
+// clashProxyFromExternal converts a pasted share link into a mihomo/Clash proxy
+// entry, or nil when Clash can't represent it — the same gate getProxies runs.
 func (s *SubClashService) clashProxyFromExternal(rawLink, name string) map[string]any {
 	ob := parseExternalLink(rawLink)
 	if ob == nil {
@@ -81,7 +78,8 @@ func (s *SubClashService) clashProxyFromExternal(rawLink, name string) map[strin
 		proxy["port"] = clashInt(server["port"])
 		proxy["cipher"] = method
 		proxy["password"] = fmt.Sprint(server["password"])
-		return proxy
+		// No early return: the shared transport/security tail is what drops an
+		// obfs node Clash cannot express, exactly as buildProxy does for inbounds.
 	case "hysteria":
 		return clashHysteriaFromExternal(settings, stream, name)
 	case "wireguard":

+ 118 - 0
internal/sub/clash_external_quota_test.go

@@ -0,0 +1,118 @@
+package sub
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gin-gonic/gin"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+	"github.com/mhsanaei/3x-ui/v3/internal/xray"
+)
+
+// base64("aes-256-gcm:clientpw"), the SIP002 userinfo both spellings share. The
+// panel emits this node as `plugin=obfs-local;obfs=http`, and Clash has no way
+// to represent it, so both the inbound path and the link path drop it.
+const clashDroppedExternalLink = "ss://[email protected]:8443?type=tcp&headerType=http&host=test#obfs"
+
+const tcpObfsStream = `{"network":"tcp","security":"none","tcpSettings":{"header":{"type":"http","request":{"path":["/"],"headers":{"Host":["test"]}}}}}`
+
+func clashSubRouter(t *testing.T) *gin.Engine {
+	t.Helper()
+	oldDistFS := distFS
+	distFS = testDistFS
+	t.Cleanup(func() { distFS = oldDistFS })
+	gin.SetMode(gin.TestMode)
+	router := gin.New()
+	NewSUBController(
+		router.Group("/"),
+		WithSUBJsonEnabled(true),
+		WithSUBClashEnabled(true),
+		WithSUBEncryption(false),
+	)
+	return router
+}
+
+func fetchClashSub(t *testing.T, router *gin.Engine, path string) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodGet, path, nil)
+	req.Host = "sub.example.com"
+	w := httptest.NewRecorder()
+	router.ServeHTTP(w, req)
+	return w
+}
+
+func seedClashQuotaSub(t *testing.T, subID string, expiry int64) {
+	t.Helper()
+	db := database.GetDB()
+	seedSubInbound(t, subID, "A", 10001, 1, wsTLSStream)
+	if err := db.Create(&xray.ClientTraffic{Email: "A@e", Up: 11, Down: 22, Total: 1024, ExpiryTime: expiry}).Error; err != nil {
+		t.Fatalf("seed A traffic: %v", err)
+	}
+	rec := &model.ClientRecord{Email: "B@e", SubID: subID, UUID: "22222222-2222-4222-8222-222222222222", Enable: true, ExpiryTime: expiry}
+	if err := db.Create(rec).Error; err != nil {
+		t.Fatalf("seed B client: %v", err)
+	}
+	if err := db.Create(&xray.ClientTraffic{Email: "B@e", Up: 100, Down: 200, Total: 2048, ExpiryTime: expiry}).Error; err != nil {
+		t.Fatalf("seed B traffic: %v", err)
+	}
+	if err := db.Create(&model.ClientExternalLink{ClientId: rec.Id, Kind: model.ExternalLinkKindLink, Value: clashDroppedExternalLink, SortIndex: 1}).Error; err != nil {
+		t.Fatalf("seed B external link: %v", err)
+	}
+}
+
+// B's node cannot be represented in Clash, but B still owns quota: the header
+// must keep counting B's traffic, not quietly serve A's numbers alone.
+func TestClashQuotaHeaderCountsDroppedExternalLink(t *testing.T) {
+	initSubDB(t)
+	subID := "clash-quota-drop"
+	expiry := time.Now().Add(24 * time.Hour).UnixMilli()
+	seedClashQuotaSub(t, subID, expiry)
+
+	w := fetchClashSub(t, clashSubRouter(t), "/clash/"+subID+"?view=raw")
+	if w.Code != http.StatusOK {
+		t.Fatalf("status = %d, want 200; body=%s", w.Code, w.Body.String())
+	}
+	if strings.Contains(w.Body.String(), "198.51.100.9") {
+		t.Fatalf("unrepresentable node leaked into the profile: %s", w.Body.String())
+	}
+	wantHeader := fmt.Sprintf("upload=111; download=222; total=3072; expire=%d", expiry/1000)
+	if got := w.Header().Get("Subscription-Userinfo"); got != wantHeader {
+		t.Fatalf("Subscription-Userinfo = %q, want %q", got, wantHeader)
+	}
+}
+
+// Nothing to serve is answered the same way whether the unrepresentable node is
+// an inbound or an external link — the drop must not depend on where it came from.
+func TestClashAllUnrepresentableNodesAnswerAlike(t *testing.T) {
+	statuses := make(map[string]int, 2)
+	for _, source := range []string{"inbound", "external-link"} {
+		t.Run(source, func(t *testing.T) {
+			initSubDB(t)
+			if source == "inbound" {
+				seedSubInbound(t, "clash-parity", "obfs", 10001, 1, tcpObfsStream)
+			} else {
+				rec := &model.ClientRecord{Email: "B@e", SubID: "clash-parity", UUID: "22222222-2222-4222-8222-222222222222", Enable: true}
+				if err := database.GetDB().Create(rec).Error; err != nil {
+					t.Fatalf("seed client: %v", err)
+				}
+				if err := database.GetDB().Create(&model.ClientExternalLink{ClientId: rec.Id, Kind: model.ExternalLinkKindLink, Value: clashDroppedExternalLink, SortIndex: 1}).Error; err != nil {
+					t.Fatalf("seed external link: %v", err)
+				}
+			}
+			w := fetchClashSub(t, clashSubRouter(t), "/clash/clash-parity?view=raw")
+			if w.Body.Len() != 0 {
+				t.Fatalf("body = %q, want empty", w.Body.String())
+			}
+			statuses[source] = w.Code
+		})
+	}
+	if statuses["inbound"] != statuses["external-link"] {
+		t.Fatalf("status differs by node source: %v", statuses)
+	}
+}

+ 75 - 0
internal/sub/clash_external_shadowsocks_test.go

@@ -0,0 +1,75 @@
+package sub
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+// Clash has no shadowsocks tcp/http header, so the inbound path drops the node
+// (applyTransport returns false) and the external-link path must drop it too,
+// instead of handing mihomo a proxy that connects with the wrong obfuscation.
+func TestClashExternalShadowsocksMatchesInboundPath(t *testing.T) {
+	const settings = `{"method":"aes-256-gcm","password":"inboundpw","clients":[{"password":"clientpw","email":"user"}]}`
+	// base64("aes-256-gcm:clientpw") — the SIP002 userinfo of the links below.
+	const userinfo = "YWVzLTI1Ni1nY206Y2xpZW50cHc"
+
+	tests := []struct {
+		name        string
+		stream      string
+		link        string
+		wantDropped bool
+	}{
+		{
+			name:   "plain tcp",
+			stream: `{"network":"tcp","security":"none"}`,
+			link:   "ss://" + userinfo + "@203.0.113.1:8443?type=tcp#ss",
+		},
+		{
+			name:        "tcp http header",
+			stream:      `{"network":"tcp","security":"none","tcpSettings":{"header":{"type":"http","request":{"path":["/"],"headers":{"Host":["test"]}}}}}`,
+			link:        "ss://" + userinfo + "@203.0.113.1:8443?type=tcp&headerType=http&host=test#ss",
+			wantDropped: true,
+		},
+		{
+			name:   "tls",
+			stream: `{"network":"tcp","security":"tls","tlsSettings":{"serverName":"ss.sni"}}`,
+			link:   "ss://" + userinfo + "@203.0.113.1:8443?type=tcp&security=tls&sni=ss.sni#ss",
+		},
+	}
+
+	svc := NewSubClashService(false, "", &SubService{})
+	client := model.Client{Password: "clientpw", Email: "user"}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			inbound := &model.Inbound{
+				Listen:         "203.0.113.1",
+				Port:           8443,
+				Protocol:       model.Shadowsocks,
+				Remark:         "ss",
+				Settings:       settings,
+				StreamSettings: tc.stream,
+			}
+
+			fromInbound := svc.buildProxy(svc.SubService, inbound, client, svc.streamData(tc.stream), nil)
+			fromLink := svc.clashProxyFromExternal(tc.link, "external")
+
+			if tc.wantDropped {
+				if fromInbound != nil || fromLink != nil {
+					t.Fatalf("not representable in clash, want both dropped: inbound %#v, link %#v", fromInbound, fromLink)
+				}
+				return
+			}
+			if fromInbound == nil || fromLink == nil {
+				t.Fatalf("want a proxy from both paths: inbound %#v, link %#v", fromInbound, fromLink)
+			}
+			delete(fromInbound, "name")
+			delete(fromLink, "name")
+			if !reflect.DeepEqual(fromInbound, fromLink) {
+				t.Fatalf("inbound %#v != link %#v", fromInbound, fromLink)
+			}
+		})
+	}
+}

+ 3 - 2
internal/sub/clash_service.go

@@ -78,8 +78,10 @@ func (s *SubClashService) getClash(subId string, host string, legacy bool) (stri
 		if ext.Enable {
 			hasEnabledClient = true
 		}
+		// Count the client even when no proxy comes out of this link, so the
+		// quota header does not shrink because a node is unrepresentable in Clash.
+		seenEmails[ext.Email] = struct{}{}
 		if !ext.Active {
-			seenEmails[ext.Email] = struct{}{}
 			hasInactiveExternal = true
 			continue
 		}
@@ -89,7 +91,6 @@ func (s *SubClashService) getClash(subId string, host string, legacy bool) (stri
 				name = ext.Email
 			}
 			if proxy := s.clashProxyFromExternal(el.Link, name); proxy != nil {
-				seenEmails[ext.Email] = struct{}{}
 				proxies = append(proxies, proxy)
 			}
 		}