Selaa lähdekoodia

refactor(inbound-tag): drop protocol segment from canonical shape

The canonical tag is now "[n<id>-]in-[<listen>:]<port>-<transport>"
instead of "[n<id>-]in-[<listen>:]<port>-<protocol>-<transport>".
Two TCP inbounds on the same port are already blocked by
checkPortConflict, so only the transport segment is needed to
disambiguate the legitimate tcp/udp coexistence case.

Existing DB rows keep their current tags via resolveInboundTag's
"reuse if free" branch — no migration needed (protocol-segment
form was never released).
MHSanaei 9 tuntia sitten
vanhempi
sitoutus
c5dc84d314
2 muutettua tiedostoa jossa 26 lisäystä ja 134 poistoa
  1. 3 111
      web/service/port_conflict.go
  2. 23 23
      web/service/port_conflict_test.go

+ 3 - 111
web/service/port_conflict.go

@@ -10,9 +10,6 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/util/common"
 	"github.com/mhsanaei/3x-ui/v3/util/common"
 )
 )
 
 
-// transportBits is a bitmask of L4 transports an inbound listens on.
-// 0.0.0.0:443/tcp and 0.0.0.0:443/udp are independent sockets in linux,
-// so the conflict check needs more than just the port number.
 type transportBits uint8
 type transportBits uint8
 
 
 const (
 const (
@@ -20,19 +17,6 @@ const (
 	transportUDP
 	transportUDP
 )
 )
 
 
-// inboundTransports returns the L4 transports the given inbound listens on.
-// always returns at least one bit (falls back to tcp on parse errors), so
-// no parse failure can silently let a real socket collision through.
-//
-// the rules:
-//   - hysteria, wireguard: udp regardless of streamSettings
-//   - streamSettings.network=kcp or quic: udp (both ride on udp at L4)
-//   - shadowsocks: settings.network ("tcp" / "udp" / "tcp,udp"), overrides
-//     the streamSettings-derived bit when present
-//   - tunnel (xray dokodemo-door): same shape via settings.allowedNetwork
-//     (3x-ui's wrapper renames the field)
-//   - mixed (socks/http combo): tcp + udp when settings.udp is true
-//   - everything else: tcp
 func inboundTransports(protocol model.Protocol, streamSettings, settings string) transportBits {
 func inboundTransports(protocol model.Protocol, streamSettings, settings string) transportBits {
 	// protocols that ignore streamSettings entirely.
 	// protocols that ignore streamSettings entirely.
 	switch protocol {
 	switch protocol {
@@ -69,11 +53,6 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string)
 		if json.Unmarshal([]byte(settings), &st) == nil {
 		if json.Unmarshal([]byte(settings), &st) == nil {
 			switch protocol {
 			switch protocol {
 			case model.Shadowsocks, model.Tunnel:
 			case model.Shadowsocks, model.Tunnel:
-				// shadowsocks exposes settings.network, tunnel exposes
-				// settings.allowedNetwork (3x-ui's wrapper around xray's
-				// dokodemo-door). both carry "tcp" / "udp" / "tcp,udp"
-				// and, when present, win outright over the streamSettings-
-				// derived default; absent/empty keeps the inferred bit (tcp).
 				key := "network"
 				key := "network"
 				if protocol == model.Tunnel {
 				if protocol == model.Tunnel {
 					key = "allowedNetwork"
 					key = "allowedNetwork"
@@ -106,10 +85,6 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string)
 	return bits
 	return bits
 }
 }
 
 
-// listenOverlaps reports whether two listen addresses can collide on the
-// same port. preserves the rule from the original checkPortExist:
-// any-address (empty / 0.0.0.0 / :: / ::0) overlaps with everything,
-// otherwise only identical specific addresses overlap.
 func listenOverlaps(a, b string) bool {
 func listenOverlaps(a, b string) bool {
 	if isAnyListen(a) || isAnyListen(b) {
 	if isAnyListen(a) || isAnyListen(b) {
 		return true
 		return true
@@ -121,12 +96,6 @@ func isAnyListen(s string) bool {
 	return s == "" || s == "0.0.0.0" || s == "::" || s == "::0"
 	return s == "" || s == "0.0.0.0" || s == "::" || s == "::0"
 }
 }
 
 
-// portConflictDetail describes the existing inbound that an add/update
-// would collide with. it carries enough context for the API layer to
-// render a user-actionable error ("port 443 (tcp) already used by
-// inbound 'my-vless' (#7) on *") instead of the historical opaque
-// "Port exists". Transports holds only the bits the two inbounds
-// actually share, not the existing inbound's full transport mask.
 type portConflictDetail struct {
 type portConflictDetail struct {
 	InboundID  int
 	InboundID  int
 	Remark     string
 	Remark     string
@@ -155,22 +124,6 @@ func (d *portConflictDetail) String() string {
 		d.Port, transportTagSuffix(d.Transports), name, listen)
 		d.Port, transportTagSuffix(d.Transports), name, listen)
 }
 }
 
 
-// checkPortConflict reports the existing inbound (if any) that adding
-// or updating an inbound on (listen, port) would clash with. nil result
-// means no conflict.
-//
-// the check understands that tcp/443 and udp/443 are independent
-// sockets in linux and may coexist on the same address (see
-// inboundTransports for the per-protocol L4 mapping).
-//
-// node scope: inbounds with different NodeID run on different physical
-// machines (local panel xray vs a remote node, or two remote nodes),
-// so their sockets can't collide. only candidates with the same NodeID
-// participate in the listen/transport overlap check.
-//
-// listen overlap: a specific listen address conflicts with any-address
-// on the same port (both directions), otherwise only identical specific
-// addresses overlap.
 func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int) (*portConflictDetail, error) {
 func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int) (*portConflictDetail, error) {
 	db := database.GetDB()
 	db := database.GetDB()
 
 
@@ -208,11 +161,6 @@ func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int)
 	return nil, nil
 	return nil, nil
 }
 }
 
 
-// sameNode reports whether two NodeID pointers refer to the same xray
-// process. nil/nil means both inbounds run on the local panel; non-nil
-// with equal value means they share the same remote node. any mix
-// (local vs remote, remote-A vs remote-B) is "different node" and
-// can't produce a real socket collision.
 func sameNode(a, b *int) bool {
 func sameNode(a, b *int) bool {
 	if a == nil && b == nil {
 	if a == nil && b == nil {
 		return true
 		return true
@@ -223,9 +171,6 @@ func sameNode(a, b *int) bool {
 	return *a == *b
 	return *a == *b
 }
 }
 
 
-// baseInboundTag is the "in-<port>" / "in-<listen>:<port>" core used
-// by composeInboundTag and as a probe shape in setRemoteTrafficLocked
-// for node-side xray imports that pre-date the canonical naming.
 func baseInboundTag(listen string, port int) string {
 func baseInboundTag(listen string, port int) string {
 	if isAnyListen(listen) {
 	if isAnyListen(listen) {
 		return fmt.Sprintf("in-%v", port)
 		return fmt.Sprintf("in-%v", port)
@@ -255,53 +200,13 @@ func nodeTagPrefix(nodeID *int) string {
 	return fmt.Sprintf("n%d-", *nodeID)
 	return fmt.Sprintf("n%d-", *nodeID)
 }
 }
 
 
-// protocolShortName collapses the full protocol identifier into a 2–4
-// char tag-friendly token (shadowsocks → ss, wireguard → wg, …). Falls
-// back to the raw identifier for anything not in the table so future
-// protocols don't need a code change just to get a tag.
-func protocolShortName(p model.Protocol) string {
-	switch p {
-	case model.VMESS:
-		return "vm"
-	case model.VLESS:
-		return "vl"
-	case model.Trojan:
-		return "tr"
-	case model.Shadowsocks:
-		return "ss"
-	case model.Mixed:
-		return "mx"
-	case model.WireGuard:
-		return "wg"
-	case model.Hysteria:
-		return "hy"
-	case model.Tunnel:
-		return "tn"
-	case model.HTTP:
-		return "http"
-	}
-	if p == "" {
-		return "any"
-	}
-	return string(p)
-}
-
-// composeInboundTag returns the canonical
-// "[n<id>-]inbound-[<listen>:]<port>-<protocol>-<network>" shape used
-// for every newly created inbound. The protocol + network segments
-// disambiguate tcp/443 and udp/443 sharing a listener; the node prefix
-// lets the same port live on local + node.
-func composeInboundTag(listen string, port int, protocol model.Protocol, nodeID *int, bits transportBits) string {
-	return nodeTagPrefix(nodeID) + baseInboundTag(listen, port) + "-" + protocolShortName(protocol) + "-" + transportTagSuffix(bits)
+func composeInboundTag(listen string, port int, nodeID *int, bits transportBits) string {
+	return nodeTagPrefix(nodeID) + baseInboundTag(listen, port) + "-" + transportTagSuffix(bits)
 }
 }
 
 
-// generateInboundTag returns a free tag in the canonical shape. ignoreId
-// is the inbound's own id on update so it doesn't see itself as taken;
-// pass 0 on add. Numeric suffix fallback is defensive — the port check
-// should have already blocked an exact-collision insert.
 func (s *InboundService) generateInboundTag(inbound *model.Inbound, ignoreId int) (string, error) {
 func (s *InboundService) generateInboundTag(inbound *model.Inbound, ignoreId int) (string, error) {
 	bits := inboundTransports(inbound.Protocol, inbound.StreamSettings, inbound.Settings)
 	bits := inboundTransports(inbound.Protocol, inbound.StreamSettings, inbound.Settings)
-	candidate := composeInboundTag(inbound.Listen, inbound.Port, inbound.Protocol, inbound.NodeID, bits)
+	candidate := composeInboundTag(inbound.Listen, inbound.Port, inbound.NodeID, bits)
 	exists, err := s.tagExists(candidate, ignoreId)
 	exists, err := s.tagExists(candidate, ignoreId)
 	if err != nil {
 	if err != nil {
 		return "", err
 		return "", err
@@ -323,19 +228,6 @@ func (s *InboundService) generateInboundTag(inbound *model.Inbound, ignoreId int
 	return "", common.NewError("could not pick a unique inbound tag for port:", inbound.Port)
 	return "", common.NewError("could not pick a unique inbound tag for port:", inbound.Port)
 }
 }
 
 
-// resolveInboundTag chooses a tag for an Add or Update. when the caller
-// supplied a non-empty Tag (e.g. the central panel pushed its picked
-// tag to a node during a multi-node sync) and that tag is free in the
-// local DB, it's used verbatim so the two panels stay in agreement —
-// otherwise the node would regenerate (often back to bare
-// "inbound-<port>") and the eventual traffic sync-back would try to
-// INSERT a row whose tag already exists, hitting the UNIQUE constraint
-// on inbounds.tag and rolling the node-side row right back out.
-// when Tag is empty (the common UI path) or collides, fall back to the
-// transport-aware generateInboundTag.
-//
-// ignoreId mirrors generateInboundTag: pass 0 on add, the inbound's
-// own id on update so a row doesn't see its own current tag as taken.
 func (s *InboundService) resolveInboundTag(inbound *model.Inbound, ignoreId int) (string, error) {
 func (s *InboundService) resolveInboundTag(inbound *model.Inbound, ignoreId int) (string, error) {
 	if inbound.Tag != "" {
 	if inbound.Tag != "" {
 		taken, err := s.tagExists(inbound.Tag, ignoreId)
 		taken, err := s.tagExists(inbound.Tag, ignoreId)

+ 23 - 23
web/service/port_conflict_test.go

@@ -285,13 +285,13 @@ func TestGenerateInboundTag_DisambiguatesByTransportOnSamePort(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "in-443-hy-udp" {
-		t.Fatalf("expected in-443-hy-udp, got %q", got)
+	if got != "in-443-udp" {
+		t.Fatalf("expected in-443-udp, got %q", got)
 	}
 	}
 }
 }
 
 
-// when the port is free, the canonical tag carries protocol + transport
-// so tcp/8443 and udp/8443 get distinct tags out of the box.
+// when the port is free, the canonical tag carries the transport so
+// tcp/8443 and udp/8443 get distinct tags out of the box.
 func TestGenerateInboundTag_KeepsBaseTagWhenFree(t *testing.T) {
 func TestGenerateInboundTag_KeepsBaseTagWhenFree(t *testing.T) {
 	setupConflictDB(t)
 	setupConflictDB(t)
 
 
@@ -305,8 +305,8 @@ func TestGenerateInboundTag_KeepsBaseTagWhenFree(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "in-8443-vl-tcp" {
-		t.Fatalf("expected in-8443-vl-tcp, got %q", got)
+	if got != "in-8443-tcp" {
+		t.Fatalf("expected in-8443-tcp, got %q", got)
 	}
 	}
 }
 }
 
 
@@ -314,10 +314,10 @@ func TestGenerateInboundTag_KeepsBaseTagWhenFree(t *testing.T) {
 // that's what ignoreId is for.
 // that's what ignoreId is for.
 func TestGenerateInboundTag_IgnoresSelfOnUpdate(t *testing.T) {
 func TestGenerateInboundTag_IgnoresSelfOnUpdate(t *testing.T) {
 	setupConflictDB(t)
 	setupConflictDB(t)
-	seedInboundConflict(t, "in-443-vl-tcp", "0.0.0.0", 443, model.VLESS, `{"network":"tcp"}`, `{}`)
+	seedInboundConflict(t, "in-443-tcp", "0.0.0.0", 443, model.VLESS, `{"network":"tcp"}`, `{}`)
 
 
 	var existing model.Inbound
 	var existing model.Inbound
-	if err := database.GetDB().Where("tag = ?", "in-443-vl-tcp").First(&existing).Error; err != nil {
+	if err := database.GetDB().Where("tag = ?", "in-443-tcp").First(&existing).Error; err != nil {
 		t.Fatalf("read seeded row: %v", err)
 		t.Fatalf("read seeded row: %v", err)
 	}
 	}
 
 
@@ -326,7 +326,7 @@ func TestGenerateInboundTag_IgnoresSelfOnUpdate(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "in-443-vl-tcp" {
+	if got != "in-443-tcp" {
 		t.Fatalf("self-update must keep base tag, got %q", got)
 		t.Fatalf("self-update must keep base tag, got %q", got)
 	}
 	}
 }
 }
@@ -346,8 +346,8 @@ func TestGenerateInboundTag_SpecificListenSameDisambiguation(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "in-1.2.3.4:443-hy-udp" {
-		t.Fatalf("expected in-1.2.3.4:443-hy-udp, got %q", got)
+	if got != "in-1.2.3.4:443-udp" {
+		t.Fatalf("expected in-1.2.3.4:443-udp, got %q", got)
 	}
 	}
 }
 }
 
 
@@ -399,8 +399,8 @@ func TestCheckPortConflict_NodeScope(t *testing.T) {
 // panels diverged, causing a UNIQUE constraint failure on sync.
 // panels diverged, causing a UNIQUE constraint failure on sync.
 func TestResolveInboundTag_RespectsCallerTagWhenFree(t *testing.T) {
 func TestResolveInboundTag_RespectsCallerTagWhenFree(t *testing.T) {
 	setupConflictDB(t)
 	setupConflictDB(t)
-	seedInboundConflictNode(t, "in-5000-vl-tcp", "0.0.0.0", 5000, model.VLESS, `{"network":"tcp"}`, `{}`, nil)
-	seedInboundConflictNode(t, "in-5000-hy-udp", "0.0.0.0", 5000, model.Hysteria, ``, ``, nil)
+	seedInboundConflictNode(t, "in-5000-tcp", "0.0.0.0", 5000, model.VLESS, `{"network":"tcp"}`, `{}`, nil)
+	seedInboundConflictNode(t, "in-5000-udp", "0.0.0.0", 5000, model.Hysteria, ``, ``, nil)
 
 
 	svc := &InboundService{}
 	svc := &InboundService{}
 	pushed := &model.Inbound{
 	pushed := &model.Inbound{
@@ -436,8 +436,8 @@ func TestResolveInboundTag_GeneratesWhenTagEmpty(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("resolveInboundTag: %v", err)
 		t.Fatalf("resolveInboundTag: %v", err)
 	}
 	}
-	if got != "in-8443-vl-tcp" {
-		t.Fatalf("expected generated in-8443-vl-tcp, got %q", got)
+	if got != "in-8443-tcp" {
+		t.Fatalf("expected generated in-8443-tcp, got %q", got)
 	}
 	}
 }
 }
 
 
@@ -448,11 +448,11 @@ func TestResolveInboundTag_GeneratesWhenTagEmpty(t *testing.T) {
 // tag that the central will pick up via the AddInbound response.
 // tag that the central will pick up via the AddInbound response.
 func TestResolveInboundTag_RegeneratesOnCollision(t *testing.T) {
 func TestResolveInboundTag_RegeneratesOnCollision(t *testing.T) {
 	setupConflictDB(t)
 	setupConflictDB(t)
-	seedInboundConflictNode(t, "in-5000-vl-tcp", "0.0.0.0", 5000, model.VLESS, `{"network":"tcp"}`, `{}`, nil)
+	seedInboundConflictNode(t, "in-5000-tcp", "0.0.0.0", 5000, model.VLESS, `{"network":"tcp"}`, `{}`, nil)
 
 
 	svc := &InboundService{}
 	svc := &InboundService{}
 	pushed := &model.Inbound{
 	pushed := &model.Inbound{
-		Tag:            "in-5000-vl-tcp",
+		Tag:            "in-5000-tcp",
 		Listen:         "0.0.0.0",
 		Listen:         "0.0.0.0",
 		Port:           5000,
 		Port:           5000,
 		Protocol:       model.Hysteria,
 		Protocol:       model.Hysteria,
@@ -463,7 +463,7 @@ func TestResolveInboundTag_RegeneratesOnCollision(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("resolveInboundTag: %v", err)
 		t.Fatalf("resolveInboundTag: %v", err)
 	}
 	}
-	if got == "in-5000-vl-tcp" {
+	if got == "in-5000-tcp" {
 		t.Fatalf("colliding caller tag must be replaced, but resolver kept %q", got)
 		t.Fatalf("colliding caller tag must be replaced, but resolver kept %q", got)
 	}
 	}
 }
 }
@@ -486,8 +486,8 @@ func TestGenerateInboundTag_NodePrefix(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "n1-in-443-vl-tcp" {
-		t.Fatalf("expected n1-in-443-vl-tcp, got %q", got)
+	if got != "n1-in-443-tcp" {
+		t.Fatalf("expected n1-in-443-tcp, got %q", got)
 	}
 	}
 }
 }
 
 
@@ -495,7 +495,7 @@ func TestGenerateInboundTag_NodePrefix(t *testing.T) {
 // the prefix scopes the tag to that specific node.
 // the prefix scopes the tag to that specific node.
 func TestGenerateInboundTag_NodePrefixedDoesNotCollideWithLocal(t *testing.T) {
 func TestGenerateInboundTag_NodePrefixedDoesNotCollideWithLocal(t *testing.T) {
 	setupConflictDB(t)
 	setupConflictDB(t)
-	seedInboundConflict(t, "in-443-vl-tcp", "0.0.0.0", 443, model.VLESS, `{"network":"tcp"}`, `{}`)
+	seedInboundConflict(t, "in-443-tcp", "0.0.0.0", 443, model.VLESS, `{"network":"tcp"}`, `{}`)
 
 
 	svc := &InboundService{}
 	svc := &InboundService{}
 	in := &model.Inbound{
 	in := &model.Inbound{
@@ -508,8 +508,8 @@ func TestGenerateInboundTag_NodePrefixedDoesNotCollideWithLocal(t *testing.T) {
 	if err != nil {
 	if err != nil {
 		t.Fatalf("generateInboundTag: %v", err)
 		t.Fatalf("generateInboundTag: %v", err)
 	}
 	}
-	if got != "n1-in-443-vl-tcp" {
-		t.Fatalf("expected n1-in-443-vl-tcp, got %q", got)
+	if got != "n1-in-443-tcp" {
+		t.Fatalf("expected n1-in-443-tcp, got %q", got)
 	}
 	}
 }
 }