From c5dc84d314a422d32b8863e1a6e3de8463b88f03 Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Thu, 28 May 2026 15:50:00 +0200 Subject: [PATCH] refactor(inbound-tag): drop protocol segment from canonical shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonical tag is now "[n-]in-[:]-" instead of "[n-]in-[:]--". 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). --- web/service/port_conflict.go | 114 +----------------------------- web/service/port_conflict_test.go | 46 ++++++------ 2 files changed, 26 insertions(+), 134 deletions(-) diff --git a/web/service/port_conflict.go b/web/service/port_conflict.go index 2b7739da..2c36d54f 100644 --- a/web/service/port_conflict.go +++ b/web/service/port_conflict.go @@ -10,9 +10,6 @@ import ( "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 const ( @@ -20,19 +17,6 @@ const ( 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 { // protocols that ignore streamSettings entirely. switch protocol { @@ -69,11 +53,6 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string) if json.Unmarshal([]byte(settings), &st) == nil { switch protocol { 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" if protocol == model.Tunnel { key = "allowedNetwork" @@ -106,10 +85,6 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string) 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 { if isAnyListen(a) || isAnyListen(b) { return true @@ -121,12 +96,6 @@ func isAnyListen(s string) bool { 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 { InboundID int Remark string @@ -155,22 +124,6 @@ func (d *portConflictDetail) String() string { 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) { db := database.GetDB() @@ -208,11 +161,6 @@ func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int) 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 { if a == nil && b == nil { return true @@ -223,9 +171,6 @@ func sameNode(a, b *int) bool { return *a == *b } -// baseInboundTag is the "in-" / "in-:" 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 { if isAnyListen(listen) { return fmt.Sprintf("in-%v", port) @@ -255,53 +200,13 @@ func nodeTagPrefix(nodeID *int) string { 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) +func composeInboundTag(listen string, port int, nodeID *int, bits transportBits) string { + return nodeTagPrefix(nodeID) + baseInboundTag(listen, port) + "-" + transportTagSuffix(bits) } -// composeInboundTag returns the canonical -// "[n-]inbound-[:]--" 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) -} - -// 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) { 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) if err != nil { 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) } -// 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-") 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) { if inbound.Tag != "" { taken, err := s.tagExists(inbound.Tag, ignoreId) diff --git a/web/service/port_conflict_test.go b/web/service/port_conflict_test.go index 06258325..2ef4cea2 100644 --- a/web/service/port_conflict_test.go +++ b/web/service/port_conflict_test.go @@ -285,13 +285,13 @@ func TestGenerateInboundTag_DisambiguatesByTransportOnSamePort(t *testing.T) { if err != nil { 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) { setupConflictDB(t) @@ -305,8 +305,8 @@ func TestGenerateInboundTag_KeepsBaseTagWhenFree(t *testing.T) { if err != nil { 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. func TestGenerateInboundTag_IgnoresSelfOnUpdate(t *testing.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 - 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) } @@ -326,7 +326,7 @@ func TestGenerateInboundTag_IgnoresSelfOnUpdate(t *testing.T) { if err != nil { 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) } } @@ -346,8 +346,8 @@ func TestGenerateInboundTag_SpecificListenSameDisambiguation(t *testing.T) { if err != nil { 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. func TestResolveInboundTag_RespectsCallerTagWhenFree(t *testing.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{} pushed := &model.Inbound{ @@ -436,8 +436,8 @@ func TestResolveInboundTag_GeneratesWhenTagEmpty(t *testing.T) { if err != nil { 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. func TestResolveInboundTag_RegeneratesOnCollision(t *testing.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{} pushed := &model.Inbound{ - Tag: "in-5000-vl-tcp", + Tag: "in-5000-tcp", Listen: "0.0.0.0", Port: 5000, Protocol: model.Hysteria, @@ -463,7 +463,7 @@ func TestResolveInboundTag_RegeneratesOnCollision(t *testing.T) { if err != nil { 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) } } @@ -486,8 +486,8 @@ func TestGenerateInboundTag_NodePrefix(t *testing.T) { if err != nil { 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. func TestGenerateInboundTag_NodePrefixedDoesNotCollideWithLocal(t *testing.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{} in := &model.Inbound{ @@ -508,8 +508,8 @@ func TestGenerateInboundTag_NodePrefixedDoesNotCollideWithLocal(t *testing.T) { if err != nil { 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) } }