fix(subscriptions): avoid shared mutable state during generation (#5270)

* fix(subscriptions): avoid shared mutable state during generation

* fix(subscriptions): serve external-link-only subs in JSON/Clash; load remark settings per request

The ForRequest refactor added an early `len(inbounds) == 0` return to
GetJson/GetClash that fired before external links were fetched, so a
subscription whose only entries are external links (or whose inbounds are
all disabled) rendered empty in the JSON and Clash formats. Drop the
premature check — the existing inbounds+externalLinks empty guard already
covers the truly-empty case.

Also load datepicker/emailInRemark in PrepareForRequest rather than only in
getSubs, so JSON and Clash remarks honor these settings instead of seeing
the zero values (emailInRemark previously depended on the shared-state leak
this PR fixes).

Add a regression test covering an external-link-only sub across both formats.

---------

Co-authored-by: Sanaei <[email protected]>

internal/sub/build_urls_test.go

@@ -70,6 +70,27 @@ func TestBuildURLs_EmptySubId(t *testing.T) {
 	}
 }
 
+func TestForRequestDoesNotMutateSharedService(t *testing.T) {
+	initSubDB(t)
+	base := &SubService{}
+
+	first := base.ForRequest("first.example.com")
+	second := base.ForRequest("second.example.com")
+
+	if base.address != "" || base.nodesByID != nil {
+		t.Fatalf("ForRequest mutated the shared service: address=%q nodes=%v", base.address, base.nodesByID)
+	}
+
+	firstURL, _, _ := first.BuildURLs("/sub/", "/json/", "/clash/", "ABC")
+	secondURL, _, _ := second.BuildURLs("/sub/", "/json/", "/clash/", "ABC")
+	if !strings.Contains(firstURL, "first.example.com") {
+		t.Fatalf("first request URL = %q, want first.example.com", firstURL)
+	}
+	if !strings.Contains(secondURL, "second.example.com") {
+		t.Fatalf("second request URL = %q, want second.example.com", secondURL)
+	}
+}
+
 // A subscriber arriving via a reverse proxy (subURI configured with full
 // HTTPS URL) must see the same scheme+host in the JSON and Clash Copy
 // URLs as in the main subURL — not the raw sub-server port 2096.

internal/sub/clash_service.go

@@ -22,13 +22,12 @@ func NewSubClashService(enableRouting bool, clashRules string, subService *SubSe
 }
 
 func (s *SubClashService) GetClash(subId string, host string) (string, string, error) {
-	// Set per-request state so resolveInboundAddress sees the node map.
-	s.SubService.PrepareForRequest(host)
-	inbounds, err := s.SubService.getInboundsBySubId(subId)
+	subReq := s.SubService.ForRequest(host)
+	inbounds, err := subReq.getInboundsBySubId(subId)
 	if err != nil {
 		return "", "", err
 	}
-	externalLinks, err := s.SubService.getClientExternalLinksBySubId(subId)
+	externalLinks, err := subReq.getClientExternalLinksBySubId(subId)
 	if err != nil {
 		return "", "", err
 	}
@@ -40,14 +39,14 @@ func (s *SubClashService) GetClash(subId string, host string) (string, string, e
 
 	seenEmails := make(map[string]struct{})
 	for _, inbound := range inbounds {
-		clients := s.SubService.matchingClients(inbound, subId)
+		clients := subReq.matchingClients(inbound, subId)
 		if len(clients) == 0 {
 			continue
 		}
-		s.SubService.projectThroughFallbackMaster(inbound)
+		subReq.projectThroughFallbackMaster(inbound)
 		for _, client := range clients {
 			seenEmails[client.Email] = struct{}{}
-			proxies = append(proxies, s.getProxies(inbound, client, host)...)
+			proxies = append(proxies, s.getProxies(subReq, inbound, client, host)...)
 		}
 	}
 	for _, ext := range externalLinks {
@@ -73,7 +72,7 @@ func (s *SubClashService) GetClash(subId string, host string) (string, string, e
 	for e := range seenEmails {
 		emails = append(emails, e)
 	}
-	traffic, _ := s.SubService.AggregateTrafficByEmails(emails)
+	traffic, _ := subReq.AggregateTrafficByEmails(emails)
 
 	proxyNames := make([]string, 0, len(proxies)+1)
 	for _, proxy := range proxies {
@@ -140,12 +139,12 @@ func fallbackProxyName(proxy map[string]any, idx int) string {
 	return fmt.Sprintf("proxy-%d", idx+1)
 }
 
-func (s *SubClashService) getProxies(inbound *model.Inbound, client model.Client, host string) []map[string]any {
+func (s *SubClashService) getProxies(subReq *SubService, inbound *model.Inbound, client model.Client, host string) []map[string]any {
 	stream := s.streamData(inbound.StreamSettings)
 	// For node-managed inbounds the Clash proxy "server" must be the
 	// node's address, not the request host. resolveInboundAddress handles
 	// the node→subscriber-host fallback chain.
-	defaultDest := s.SubService.resolveInboundAddress(inbound)
+	defaultDest := subReq.resolveInboundAddress(inbound)
 	if defaultDest == "" {
 		defaultDest = host
 	}
@@ -187,7 +186,7 @@ func (s *SubClashService) getProxies(inbound *model.Inbound, client model.Client
 			applyExternalProxyTLSToStream(extPrxy, workingStream, security)
 		}
 
-		proxy := s.buildProxy(&workingInbound, client, workingStream, extPrxy["remark"].(string))
+		proxy := s.buildProxy(subReq, &workingInbound, client, workingStream, extPrxy["remark"].(string))
 		if len(proxy) > 0 {
 			proxies = append(proxies, proxy)
 		}
@@ -195,15 +194,15 @@ func (s *SubClashService) getProxies(inbound *model.Inbound, client model.Client
 	return proxies
 }
 
-func (s *SubClashService) buildProxy(inbound *model.Inbound, client model.Client, stream map[string]any, extraRemark string) map[string]any {
+func (s *SubClashService) buildProxy(subReq *SubService, inbound *model.Inbound, client model.Client, stream map[string]any, extraRemark string) map[string]any {
 	// Hysteria has its own transport + TLS model, applyTransport /
 	// applySecurity don't fit.
 	if inbound.Protocol == model.Hysteria {
-		return s.buildHysteriaProxy(inbound, client, extraRemark)
+		return s.buildHysteriaProxy(subReq, inbound, client, extraRemark)
 	}
 
 	proxy := map[string]any{
-		"name":   s.SubService.genRemark(inbound, client.Email, extraRemark),
+		"name":   subReq.genRemark(inbound, client.Email, extraRemark),
 		"server": inbound.Listen,
 		"port":   inbound.Port,
 		"udp":    true,
@@ -274,7 +273,7 @@ func (s *SubClashService) buildProxy(inbound *model.Inbound, client model.Client
 // directly instead of going through streamData/tlsData, because those
 // helpers prune fields (like `allowInsecure` / the salamander obfs
 // block) that the hysteria proxy wants preserved.
-func (s *SubClashService) buildHysteriaProxy(inbound *model.Inbound, client model.Client, extraRemark string) map[string]any {
+func (s *SubClashService) buildHysteriaProxy(subReq *SubService, inbound *model.Inbound, client model.Client, extraRemark string) map[string]any {
 	var inboundSettings map[string]any
 	_ = json.Unmarshal([]byte(inbound.Settings), &inboundSettings)
 
@@ -286,7 +285,7 @@ func (s *SubClashService) buildHysteriaProxy(inbound *model.Inbound, client mode
 	}
 
 	proxy := map[string]any{
-		"name":   s.SubService.genRemark(inbound, client.Email, extraRemark),
+		"name":   subReq.genRemark(inbound, client.Email, extraRemark),
 		"type":   proxyType,
 		"server": inbound.Listen,
 		"port":   inbound.Port,

internal/sub/clash_service_test.go

@@ -56,7 +56,7 @@ func TestBuildProxy_VLESSRealityFieldsForClash(t *testing.T) {
 		"realitySettings": map[string]any{"serverName": "reality.example.com", "publicKey": "PBKvalue", "shortId": "ab12", "fingerprint": "chrome"},
 	}
 
-	proxy := svc.buildProxy(inbound, client, stream, "")
+	proxy := svc.buildProxy(svc.SubService, inbound, client, stream, "")
 	if proxy == nil {
 		t.Fatal("buildProxy returned nil for a valid reality stream")
 	}
@@ -199,7 +199,7 @@ func TestBuildProxy_VLESSPostQuantumEncryptionUsesMihomoEncryptionField(t *testi
 		},
 	}
 
-	proxy := svc.buildProxy(inbound, client, stream, "")
+	proxy := svc.buildProxy(svc.SubService, inbound, client, stream, "")
 
 	if proxy["encryption"] != encryption {
 		t.Fatalf("encryption = %v, want %q", proxy["encryption"], encryption)
@@ -231,7 +231,7 @@ func TestBuildProxy_VLESSFlowXhttpRealityVlessenc(t *testing.T) {
 		},
 	}
 
-	proxy := svc.buildProxy(inbound, client, stream, "")
+	proxy := svc.buildProxy(svc.SubService, inbound, client, stream, "")
 
 	if proxy["flow"] != "xtls-rprx-vision" {
 		t.Fatalf("xhttp+reality+vlessenc Clash proxy must carry the vision flow (#5232): %#v", proxy)
@@ -256,7 +256,7 @@ func TestBuildProxy_VLESSFlowDroppedWithoutVisionSupport(t *testing.T) {
 		},
 	}
 
-	proxy := svc.buildProxy(inbound, client, stream, "")
+	proxy := svc.buildProxy(svc.SubService, inbound, client, stream, "")
 
 	if _, ok := proxy["flow"]; ok {
 		t.Fatalf("tcp without tls/reality must not carry a flow: %#v", proxy)
@@ -281,7 +281,7 @@ func TestBuildProxy_VLESSNoneEncryptionOmittedForClash(t *testing.T) {
 		},
 	}
 
-	proxy := svc.buildProxy(inbound, client, stream, "")
+	proxy := svc.buildProxy(svc.SubService, inbound, client, stream, "")
 
 	if _, ok := proxy["encryption"]; ok {
 		t.Fatalf("plain vless encryption should be omitted for mihomo: %#v", proxy)

internal/sub/controller.go

@@ -137,7 +137,8 @@ func (a *SUBController) initRouter(g *gin.RouterGroup) {
 func (a *SUBController) subs(c *gin.Context) {
 	subId := c.Param("subid")
 	scheme, host, hostWithPort, hostHeader := a.subService.ResolveRequest(c)
-	subs, emails, lastOnline, traffic, err := a.subService.GetSubs(subId, host)
+	subReq := a.subService.ForRequest(host)
+	subs, emails, lastOnline, traffic, err := subReq.getSubs(subId)
 	if err != nil || len(subs) == 0 {
 		writeSubError(c, err)
 	} else {
@@ -149,7 +150,7 @@ func (a *SUBController) subs(c *gin.Context) {
 		// If the request expects HTML (e.g., browser) or explicitly asked (?html=1 or ?view=html), render the info page here
 		accept := c.GetHeader("Accept")
 		if strings.Contains(strings.ToLower(accept), "text/html") || c.Query("html") == "1" || strings.EqualFold(c.Query("view"), "html") {
-			subURL, subJsonURL, subClashURL := a.subService.BuildURLs(a.subPath, a.subJsonPath, a.subClashPath, subId)
+			subURL, subJsonURL, subClashURL := subReq.BuildURLs(a.subPath, a.subJsonPath, a.subClashPath, subId)
 			if !a.jsonEnabled {
 				subJsonURL = ""
 			}
@@ -161,7 +162,7 @@ func (a *SUBController) subs(c *gin.Context) {
 				basePath = "/"
 			}
 			basePathStr := basePath.(string)
-			page := a.subService.BuildPageData(subId, hostHeader, traffic, lastOnline, subs, emails, subURL, subJsonURL, subClashURL, basePathStr, a.subTitle, a.subSupportUrl)
+			page := subReq.BuildPageData(subId, hostHeader, traffic, lastOnline, subs, emails, subURL, subJsonURL, subClashURL, basePathStr, a.subTitle, a.subSupportUrl)
 			a.serveSubPage(c, basePathStr, page)
 			return
 		}

internal/sub/external_only_sub_test.go

@@ -0,0 +1,51 @@
+package sub
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/internal/database"
+	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
+)
+
+// A subscription whose only entries are external links — no enabled standard
+// inbound — must still render in the JSON and Clash formats, not just the raw
+// one. Regression guard for the premature len(inbounds)==0 early return that
+// short-circuited GetJson/GetClash before external links were ever fetched.
+func TestJsonAndClashServeExternalLinkOnlySub(t *testing.T) {
+	initSubDB(t)
+	db := database.GetDB()
+
+	rec := &model.ClientRecord{Email: "ext@x", SubID: "ext-only", UUID: "ext-uuid", Enable: true}
+	if err := db.Create(rec).Error; err != nil {
+		t.Fatalf("seed client: %v", err)
+	}
+	link := "vless://[email protected]:443?type=tcp&security=reality&pbk=abc&sid=12&fp=chrome#orig"
+	if err := db.Create(&model.ClientExternalLink{ClientId: rec.Id, Kind: model.ExternalLinkKindLink, Value: link, Remark: "DE-Provider", SortIndex: 1}).Error; err != nil {
+		t.Fatalf("seed external link: %v", err)
+	}
+
+	base := NewSubService(false, "-io")
+
+	jsonOut, _, err := NewSubJsonService("", "", "", base).GetJson("ext-only", "sub.example.com")
+	if err != nil {
+		t.Fatalf("GetJson err = %v", err)
+	}
+	if jsonOut == "" {
+		t.Fatal("GetJson returned empty for an external-link-only sub")
+	}
+	if !strings.Contains(jsonOut, "DE-Provider") {
+		t.Fatalf("GetJson missing external remark: %s", jsonOut)
+	}
+
+	clashOut, _, err := NewSubClashService(false, "", base).GetClash("ext-only", "sub.example.com")
+	if err != nil {
+		t.Fatalf("GetClash err = %v", err)
+	}
+	if clashOut == "" {
+		t.Fatal("GetClash returned empty for an external-link-only sub")
+	}
+	if !strings.Contains(clashOut, "DE-Provider") {
+		t.Fatalf("GetClash missing external proxy: %s", clashOut)
+	}
+}

internal/sub/json_service.go

@@ -58,14 +58,12 @@ func NewSubJsonService(mux string, rules string, finalMask string, subService *S
 
 // GetJson generates a JSON subscription configuration for the given subscription ID and host.
 func (s *SubJsonService) GetJson(subId string, host string) (string, string, error) {
-	// Set per-request state on the shared SubService so any
-	// resolveInboundAddress call inside picks node-aware host values.
-	s.SubService.PrepareForRequest(host)
-	inbounds, err := s.SubService.getInboundsBySubId(subId)
+	subReq := s.SubService.ForRequest(host)
+	inbounds, err := subReq.getInboundsBySubId(subId)
 	if err != nil {
 		return "", "", err
 	}
-	externalLinks, err := s.SubService.getClientExternalLinksBySubId(subId)
+	externalLinks, err := subReq.getClientExternalLinksBySubId(subId)
 	if err != nil {
 		return "", "", err
 	}
@@ -79,15 +77,15 @@ func (s *SubJsonService) GetJson(subId string, host string) (string, string, err
 	seenEmails := make(map[string]struct{})
 	// Prepare Inbounds
 	for _, inbound := range inbounds {
-		clients := s.SubService.matchingClients(inbound, subId)
+		clients := subReq.matchingClients(inbound, subId)
 		if len(clients) == 0 {
 			continue
 		}
-		s.SubService.projectThroughFallbackMaster(inbound)
+		subReq.projectThroughFallbackMaster(inbound)
 
 		for _, client := range clients {
 			seenEmails[client.Email] = struct{}{}
-			configArray = append(configArray, s.getConfig(inbound, client, host)...)
+			configArray = append(configArray, s.getConfig(subReq, inbound, client, host)...)
 		}
 	}
 	for _, ext := range externalLinks {
@@ -120,7 +118,7 @@ func (s *SubJsonService) GetJson(subId string, host string) (string, string, err
 	for e := range seenEmails {
 		emails = append(emails, e)
 	}
-	traffic, _ := s.SubService.AggregateTrafficByEmails(emails)
+	traffic, _ := subReq.AggregateTrafficByEmails(emails)
 
 	// Combile outbounds
 	var finalJson []byte
@@ -134,7 +132,7 @@ func (s *SubJsonService) GetJson(subId string, host string) (string, string, err
 	return string(finalJson), header, nil
 }
 
-func (s *SubJsonService) getConfig(inbound *model.Inbound, client model.Client, host string) []json_util.RawMessage {
+func (s *SubJsonService) getConfig(subReq *SubService, inbound *model.Inbound, client model.Client, host string) []json_util.RawMessage {
 	var newJsonArray []json_util.RawMessage
 	stream := s.streamData(inbound.StreamSettings)
 
@@ -143,7 +141,7 @@ func (s *SubJsonService) getConfig(inbound *model.Inbound, client model.Client,
 	// For node-managed inbounds we want the node's address — request
 	// host won't reach the right xray. resolveInboundAddress already
 	// implements the node→subscriber-host fallback chain.
-	defaultDest := s.SubService.resolveInboundAddress(inbound)
+	defaultDest := subReq.resolveInboundAddress(inbound)
 	if defaultDest == "" {
 		defaultDest = host
 	}
@@ -204,7 +202,7 @@ func (s *SubJsonService) getConfig(inbound *model.Inbound, client model.Client,
 		maps.Copy(newConfigJson, s.configJson)
 
 		newConfigJson["outbounds"] = newOutbounds
-		newConfigJson["remarks"] = s.SubService.genRemark(inbound, client.Email, extPrxy["remark"].(string))
+		newConfigJson["remarks"] = subReq.genRemark(inbound, client.Email, extPrxy["remark"].(string))
 
 		newConfig, _ := json.MarshalIndent(newConfigJson, "", "  ")
 		newJsonArray = append(newJsonArray, newConfig)

internal/sub/service.go

@@ -49,11 +49,18 @@ func NewSubService(showInfo bool, remarkModel string) *SubService {
 	}
 }
 
-// PrepareForRequest sets per-request state (host + nodes map) on the
-// shared SubService. Called by every entry point — GetSubs, GetJson,
-// GetClash — so resolveInboundAddress sees the right host and the
-// freshly-loaded node map regardless of which sub flavour the client
-// hit.
+// ForRequest returns a shallow copy with request-scoped state populated.
+// Subscription controllers share one base SubService, so request-specific
+// fields such as address and nodesByID must live on a per-request copy.
+func (s *SubService) ForRequest(host string) *SubService {
+	req := *s
+	req.PrepareForRequest(host)
+	return &req
+}
+
+// PrepareForRequest sets per-request state (host + nodes map) on this
+// SubService instance. HTTP handlers should call ForRequest instead so the
+// controller's shared base service is never mutated by concurrent requests.
 func (s *SubService) PrepareForRequest(host string) {
 	if !isRoutableHost(host) {
 		if d := s.configuredPublicHost(); d != "" {
@@ -64,6 +71,23 @@ func (s *SubService) PrepareForRequest(host string) {
 	}
 	s.address = host
 	s.loadNodes()
+	s.loadRemarkSettings()
+}
+
+// loadRemarkSettings populates the per-request remark formatting state so
+// every subscription format — raw, JSON, Clash — renders remarks the same
+// way. genRemark reads emailInRemark and the date formatter reads datepicker;
+// loading these only in getSubs left JSON/Clash with the zero values.
+func (s *SubService) loadRemarkSettings() {
+	var err error
+	s.datepicker, err = s.settingService.GetDatepicker()
+	if err != nil {
+		s.datepicker = "gregorian"
+	}
+	s.emailInRemark, err = s.settingService.GetSubEmailInRemark()
+	if err != nil {
+		s.emailInRemark = true
+	}
 }
 
 func (s *SubService) configuredPublicHost() string {
@@ -139,7 +163,10 @@ func (s *SubService) matchingClients(inbound *model.Inbound, subId string) []mod
 
 // GetSubs retrieves subscription links for a given subscription ID and host.
 func (s *SubService) GetSubs(subId string, host string) ([]string, []string, int64, xray.ClientTraffic, error) {
-	s.PrepareForRequest(host)
+	return s.ForRequest(host).getSubs(subId)
+}
+
+func (s *SubService) getSubs(subId string) ([]string, []string, int64, xray.ClientTraffic, error) {
 	var result []string
 	var emails []string
 	var traffic xray.ClientTraffic
@@ -157,16 +184,6 @@ func (s *SubService) GetSubs(subId string, host string) ([]string, []string, int
 		return nil, nil, 0, traffic, nil
 	}
 
-	s.datepicker, err = s.settingService.GetDatepicker()
-	if err != nil {
-		s.datepicker = "gregorian"
-	}
-
-	s.emailInRemark, err = s.settingService.GetSubEmailInRemark()
-	if err != nil {
-		s.emailInRemark = true
-	}
-
 	seenEmails := make(map[string]struct{})
 	for _, inbound := range inbounds {
 		clients := s.matchingClients(inbound, subId)