Przeglądaj źródła

feat(port-conflict): include offending inbound + L4 in the error, cover quic and tunnel.allowedNetwork

checkPortConflict used to return a bare bool, which the API layer
translated into "Port already exists: 443" with no hint about which
existing inbound owned the port, what listen address it used, or
which L4 transport actually clashed. On a panel with dozens of
inbounds the admin had to scan the list by hand to figure out the
collision.

Return a portConflictDetail{InboundID, Remark, Tag, Listen, Port,
Transports} instead; a String() method formats it as
"port 443 (tcp) already used by inbound 'my-vless' (#7) on *" so the
existing common.NewError wrapping carries the full context up to the
UI without a second round-trip.

Two predicate gaps fixed at the same time:
- streamSettings.network="quic" rides on UDP the same way "kcp" does,
  so it now joins KCP in the UDP branch instead of falling through to
  the TCP default (a QUIC inbound used to silently allow a UDP
  neighbour on the same port).
- Tunnel reads settings.allowedNetwork ("tcp" / "udp" / "tcp,udp"),
  not settings.network — 3x-ui's dokodemo-door wrapper renames the
  field, and treating it as Shadowsocks-shaped left every Tunnel
  inbound looking like plain TCP regardless of what the admin
  configured.

Tests: TCP/UDP coexist + same-transport collision matrix already
covered the happy path; added QUICTreatedAsUDP, TunnelAllowedNetwork,
and DetailMessage to lock in the new behaviour. Dropped the unused
transportBits.conflicts() helper now that the call site composes the
mask itself to populate the detail.
MHSanaei 9 godzin temu
rodzic
commit
980511bcad

Plik diff jest za duży
+ 586 - 586
frontend/src/pages/inbounds/InboundFormModal.tsx


+ 22 - 22
frontend/src/pages/xray/OutboundFormModal.tsx

@@ -98,7 +98,7 @@ function isMuxAllowed(protocol: string, flow: string, network: string): boolean
 }
 
 const NETWORK_OPTIONS: { value: string; label: string }[] = [
-  { value: 'tcp', label: 'TCP (RAW)' },
+  { value: 'tcp', label: 'RAW' },
   { value: 'kcp', label: 'mKCP' },
   { value: 'ws', label: 'WebSocket' },
   { value: 'grpc', label: 'gRPC' },
@@ -684,11 +684,11 @@ export default function OutboundFormModal({
                                         ['settings', 'fragment'],
                                         checked
                                           ? {
-                                              packets: 'tlshello',
-                                              length: '100-200',
-                                              interval: '10-20',
-                                              maxSplit: '300-400',
-                                            }
+                                            packets: 'tlshello',
+                                            length: '100-200',
+                                            interval: '10-20',
+                                            maxSplit: '300-400',
+                                          }
                                           : { packets: '', length: '', interval: '', maxSplit: '' },
                                       );
                                     }}
@@ -1143,20 +1143,20 @@ export default function OutboundFormModal({
                                           ['streamSettings', 'tcpSettings', 'header'],
                                           checked
                                             ? {
-                                                type: 'http',
-                                                request: {
-                                                  version: '1.1',
-                                                  method: 'GET',
-                                                  path: ['/'],
-                                                  headers: {},
-                                                },
-                                                response: {
-                                                  version: '1.1',
-                                                  status: '200',
-                                                  reason: 'OK',
-                                                  headers: {},
-                                                },
-                                              }
+                                              type: 'http',
+                                              request: {
+                                                version: '1.1',
+                                                method: 'GET',
+                                                path: ['/'],
+                                                headers: {},
+                                              },
+                                              response: {
+                                                version: '1.1',
+                                                status: '200',
+                                                reason: 'OK',
+                                                headers: {},
+                                              },
+                                            }
                                             : { type: 'none' },
                                         )
                                       }
@@ -1771,8 +1771,8 @@ export default function OutboundFormModal({
                               normalize={(v: unknown) =>
                                 Array.isArray(v)
                                   ? v
-                                      .map((x) => Number(x))
-                                      .filter((n) => Number.isInteger(n) && n > 0)
+                                    .map((x) => Number(x))
+                                    .filter((n) => Number.isInteger(n) && n > 0)
                                   : []
                               }
                             >

+ 6 - 6
web/service/inbound.go

@@ -467,12 +467,12 @@ func (s *InboundService) AddInbound(inbound *model.Inbound) (*model.Inbound, boo
 	// Normalize streamSettings based on protocol
 	s.normalizeStreamSettings(inbound)
 
-	exist, err := s.checkPortConflict(inbound, 0)
+	conflict, err := s.checkPortConflict(inbound, 0)
 	if err != nil {
 		return inbound, false, err
 	}
-	if exist {
-		return inbound, false, common.NewError("Port already exists:", inbound.Port)
+	if conflict != nil {
+		return inbound, false, common.NewError(conflict.String())
 	}
 
 	inbound.Tag, err = s.resolveInboundTag(inbound, 0)
@@ -735,12 +735,12 @@ func (s *InboundService) UpdateInbound(inbound *model.Inbound) (*model.Inbound,
 	// Normalize streamSettings based on protocol
 	s.normalizeStreamSettings(inbound)
 
-	exist, err := s.checkPortConflict(inbound, inbound.Id)
+	conflict, err := s.checkPortConflict(inbound, inbound.Id)
 	if err != nil {
 		return inbound, false, err
 	}
-	if exist {
-		return inbound, false, common.NewError("Port already exists:", inbound.Port)
+	if conflict != nil {
+		return inbound, false, common.NewError(conflict.String())
 	}
 
 	oldInbound, err := s.GetInbound(inbound.Id)

+ 80 - 25
web/service/port_conflict.go

@@ -20,17 +20,17 @@ const (
 	transportUDP
 )
 
-// conflicts is true when the two masks share any L4 transport.
-func (b transportBits) conflicts(o transportBits) bool { return b&o != 0 }
-
 // inboundTransports returns the L4 transports the given inbound listens on.
 // always returns at least one bit (falls back to tcp on parse errors), so
-// the validator never gets looser than the old port-only check.
+// no parse failure can silently let a real socket collision through.
 //
 // the rules:
 //   - hysteria, wireguard: udp regardless of streamSettings
-//   - streamSettings.network=kcp: udp
-//   - shadowsocks: whatever settings.network says ("tcp" / "udp" / "tcp,udp")
+//   - 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 {
@@ -42,7 +42,7 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string)
 
 	var bits transportBits
 
-	// peek at streamSettings.network to spot udp transports like kcp.
+	// peek at streamSettings.network to spot udp-based transports.
 	// parse errors are non-fatal: missing or weird streamSettings just
 	// keeps the default tcp bit below.
 	network := ""
@@ -54,23 +54,31 @@ func inboundTransports(protocol model.Protocol, streamSettings, settings string)
 			}
 		}
 	}
-	if network == "kcp" {
+	switch network {
+	case "kcp", "quic":
 		bits |= transportUDP
-	} else {
+	default:
 		bits |= transportTCP
 	}
 
-	// some protocols also listen on udp on the same port via their own
-	// settings json. parse and merge.
+	// a few protocols carry their L4 choice in settings instead of (or in
+	// addition to) streamSettings: SS / Tunnel via a CSV field that wins
+	// outright, Mixed via an additive udp boolean.
 	if settings != "" {
 		var st map[string]any
 		if json.Unmarshal([]byte(settings), &st) == nil {
 			switch protocol {
-			case model.Shadowsocks:
-				// shadowsocks settings.network controls both tcp and udp,
-				// independently of streamSettings. the field takes "tcp",
-				// "udp", or "tcp,udp". if it's set, it wins outright.
-				if n, ok := st["network"].(string); ok && n != "" {
+			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"
+				}
+				if n, ok := st[key].(string); ok && n != "" {
 					bits = 0
 					for part := range strings.SplitSeq(n, ",") {
 						switch strings.TrimSpace(part) {
@@ -113,10 +121,47 @@ func isAnyListen(s string) bool {
 	return s == "" || s == "0.0.0.0" || s == "::" || s == "::0"
 }
 
-// checkPortConflict reports whether adding/updating an inbound on
-// (listen, port) would clash with an existing inbound. unlike the old
-// port-only check, this one understands that tcp/443 and udp/443 are
-// independent sockets in linux and may coexist on the same address.
+// 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
+	Tag        string
+	Listen     string
+	Port       int
+	Transports transportBits
+}
+
+// String renders the detail as a single-line, user-facing summary.
+func (d *portConflictDetail) String() string {
+	name := d.Remark
+	if name == "" {
+		name = d.Tag
+	}
+	if name == "" {
+		name = fmt.Sprintf("#%d", d.InboundID)
+	} else {
+		name = fmt.Sprintf("'%s' (#%d)", name, d.InboundID)
+	}
+	listen := d.Listen
+	if isAnyListen(listen) {
+		listen = "*"
+	}
+	return fmt.Sprintf("port %d (%s) already used by inbound %s on %s",
+		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.
+//
+// unlike the old port-only check, this one understands that tcp/443 and
+// udp/443 are independent sockets in linux and may coexist on the same
+// address.
 //
 // node scope: inbounds with different NodeID run on different physical
 // machines (local panel xray vs a remote node, or two remote nodes),
@@ -125,7 +170,7 @@ func isAnyListen(s string) bool {
 //
 // the listen-overlap rule (specific addr conflicts with any-addr on the
 // same port, both directions) is preserved from the previous check.
-func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int) (bool, error) {
+func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int) (*portConflictDetail, error) {
 	db := database.GetDB()
 
 	var candidates []*model.Inbound
@@ -134,7 +179,7 @@ func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int)
 		q = q.Where("id != ?", ignoreId)
 	}
 	if err := q.Find(&candidates).Error; err != nil {
-		return false, err
+		return nil, err
 	}
 
 	newBits := inboundTransports(inbound.Protocol, inbound.StreamSettings, inbound.Settings)
@@ -145,11 +190,21 @@ func (s *InboundService) checkPortConflict(inbound *model.Inbound, ignoreId int)
 		if !listenOverlaps(c.Listen, inbound.Listen) {
 			continue
 		}
-		if inboundTransports(c.Protocol, c.StreamSettings, c.Settings).conflicts(newBits) {
-			return true, nil
+		existingBits := inboundTransports(c.Protocol, c.StreamSettings, c.Settings)
+		shared := existingBits & newBits
+		if shared == 0 {
+			continue
 		}
+		return &portConflictDetail{
+			InboundID:  c.Id,
+			Remark:     c.Remark,
+			Tag:        c.Tag,
+			Listen:     c.Listen,
+			Port:       c.Port,
+			Transports: shared,
+		}, nil
 	}
-	return false, nil
+	return nil, nil
 }
 
 // sameNode reports whether two NodeID pointers refer to the same xray

+ 118 - 11
web/service/port_conflict_test.go

@@ -2,6 +2,7 @@ package service
 
 import (
 	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 
@@ -137,7 +138,7 @@ func TestCheckPortConflict_TCPandUDPCoexistOnSamePort(t *testing.T) {
 	if err != nil {
 		t.Fatalf("checkPortConflict: %v", err)
 	}
-	if exist {
+	if exist != nil {
 		t.Fatalf("vless/tcp and hysteria2/udp on the same port must be allowed to coexist")
 	}
 }
@@ -159,7 +160,7 @@ func TestCheckPortConflict_TCPCollidesWithTCP(t *testing.T) {
 	if err != nil {
 		t.Fatalf("checkPortConflict: %v", err)
 	}
-	if !exist {
+	if exist == nil {
 		t.Fatalf("two tcp inbounds on the same port must still conflict")
 	}
 }
@@ -181,7 +182,7 @@ func TestCheckPortConflict_UDPCollidesWithUDP(t *testing.T) {
 	if err != nil {
 		t.Fatalf("checkPortConflict: %v", err)
 	}
-	if !exist {
+	if exist == nil {
 		t.Fatalf("two udp inbounds on the same port must conflict")
 	}
 }
@@ -201,7 +202,7 @@ func TestCheckPortConflict_ShadowsocksDualListenBlocksBoth(t *testing.T) {
 		Protocol:       model.VLESS,
 		StreamSettings: `{"network":"tcp"}`,
 	}
-	if exist, err := svc.checkPortConflict(tcpClash, 0); err != nil || !exist {
+	if exist, err := svc.checkPortConflict(tcpClash, 0); err != nil || exist == nil {
 		t.Fatalf("tcp inbound should clash with shadowsocks tcp,udp; exist=%v err=%v", exist, err)
 	}
 
@@ -211,7 +212,7 @@ func TestCheckPortConflict_ShadowsocksDualListenBlocksBoth(t *testing.T) {
 		Port:     443,
 		Protocol: model.Hysteria,
 	}
-	if exist, err := svc.checkPortConflict(udpClash, 0); err != nil || !exist {
+	if exist, err := svc.checkPortConflict(udpClash, 0); err != nil || exist == nil {
 		t.Fatalf("udp inbound should clash with shadowsocks tcp,udp; exist=%v err=%v", exist, err)
 	}
 }
@@ -229,7 +230,7 @@ func TestCheckPortConflict_DifferentPortNeverConflicts(t *testing.T) {
 		Protocol:       model.VLESS,
 		StreamSettings: `{"network":"tcp"}`,
 	}
-	if exist, err := svc.checkPortConflict(other, 0); err != nil || exist {
+	if exist, err := svc.checkPortConflict(other, 0); err != nil || exist != nil {
 		t.Fatalf("different port must not conflict; exist=%v err=%v", exist, err)
 	}
 }
@@ -251,7 +252,7 @@ func TestCheckPortConflict_ListenOverlapPreserved(t *testing.T) {
 		Protocol:       model.VLESS,
 		StreamSettings: `{"network":"tcp"}`,
 	}
-	if exist, err := svc.checkPortConflict(other, 0); err != nil || exist {
+	if exist, err := svc.checkPortConflict(other, 0); err != nil || exist != nil {
 		t.Fatalf("different specific listen must not conflict; exist=%v err=%v", exist, err)
 	}
 
@@ -263,7 +264,7 @@ func TestCheckPortConflict_ListenOverlapPreserved(t *testing.T) {
 		Protocol:       model.VLESS,
 		StreamSettings: `{"network":"tcp"}`,
 	}
-	if exist, err := svc.checkPortConflict(anyAddr, 0); err != nil || !exist {
+	if exist, err := svc.checkPortConflict(anyAddr, 0); err != nil || exist == nil {
 		t.Fatalf("any-addr on same port+transport must conflict with specific; exist=%v err=%v", exist, err)
 	}
 }
@@ -386,8 +387,8 @@ func TestCheckPortConflict_NodeScope(t *testing.T) {
 			if err != nil {
 				t.Fatalf("checkPortConflict: %v", err)
 			}
-			if got != c.want {
-				t.Fatalf("got conflict=%v, want %v", got, c.want)
+			if (got != nil) != c.want {
+				t.Fatalf("got conflict=%v, want %v", got != nil, c.want)
 			}
 		})
 	}
@@ -482,7 +483,113 @@ func TestCheckPortConflict_IgnoreSelfOnUpdate(t *testing.T) {
 	}
 
 	svc := &InboundService{}
-	if exist, err := svc.checkPortConflict(&existing, existing.Id); err != nil || exist {
+	if exist, err := svc.checkPortConflict(&existing, existing.Id); err != nil || exist != nil {
 		t.Fatalf("self-update must not be flagged as conflict; exist=%v err=%v", exist, err)
 	}
 }
+
+// streamSettings.network=quic rides on UDP at L4, so a QUIC inbound must
+// conflict with a UDP-only neighbour (hysteria) on the same port but not
+// with a TCP-only one. covers the gap left by the original kcp-only check.
+func TestCheckPortConflict_QUICTreatedAsUDP(t *testing.T) {
+	quic := &model.Inbound{
+		Tag:            "vless-quic-443",
+		Listen:         "0.0.0.0",
+		Port:           443,
+		Protocol:       model.VLESS,
+		StreamSettings: `{"network":"quic"}`,
+	}
+
+	t.Run("conflicts with hysteria/udp", func(t *testing.T) {
+		setupConflictDB(t)
+		seedInboundConflict(t, "hyst-443", "0.0.0.0", 443, model.Hysteria, ``, ``)
+		svc := &InboundService{}
+		if exist, err := svc.checkPortConflict(quic, 0); err != nil || exist == nil {
+			t.Fatalf("quic on same port as hysteria must conflict; exist=%v err=%v", exist, err)
+		}
+	})
+
+	t.Run("coexists with vless/tcp", func(t *testing.T) {
+		setupConflictDB(t)
+		seedInboundConflict(t, "vless-tcp-443", "0.0.0.0", 443, model.VLESS, `{"network":"tcp"}`, `{}`)
+		svc := &InboundService{}
+		if exist, err := svc.checkPortConflict(quic, 0); err != nil || exist != nil {
+			t.Fatalf("quic and tcp on same port must coexist; exist=%v err=%v", exist, err)
+		}
+	})
+}
+
+// tunnel (dokodemo-door) carries its L4 transport list in
+// settings.allowedNetwork, not settings.network. verify the predicate
+// picks the right field for each protocol.
+func TestCheckPortConflict_TunnelAllowedNetwork(t *testing.T) {
+	setupConflictDB(t)
+	seedInboundConflict(t, "tunnel-udp-443", "0.0.0.0", 443, model.Tunnel, ``, `{"allowedNetwork":"udp"}`)
+
+	svc := &InboundService{}
+
+	// tcp inbound on same port should coexist with udp-only tunnel.
+	tcpNeighbour := &model.Inbound{
+		Tag:            "vless-443",
+		Listen:         "0.0.0.0",
+		Port:           443,
+		Protocol:       model.VLESS,
+		StreamSettings: `{"network":"tcp"}`,
+	}
+	if exist, err := svc.checkPortConflict(tcpNeighbour, 0); err != nil || exist != nil {
+		t.Fatalf("tunnel/udp and vless/tcp on same port must coexist; exist=%v err=%v", exist, err)
+	}
+
+	// udp neighbour (hysteria) on same port must conflict.
+	udpNeighbour := &model.Inbound{
+		Tag:      "hyst-443",
+		Listen:   "0.0.0.0",
+		Port:     443,
+		Protocol: model.Hysteria,
+	}
+	if exist, err := svc.checkPortConflict(udpNeighbour, 0); err != nil || exist == nil {
+		t.Fatalf("tunnel/udp and hysteria on same port must conflict; exist=%v err=%v", exist, err)
+	}
+}
+
+// the rich conflict detail surfaced to the user must name the offending
+// inbound (by remark when available) and the shared L4 transport(s).
+func TestCheckPortConflict_DetailMessage(t *testing.T) {
+	setupConflictDB(t)
+	seeded := &model.Inbound{
+		Tag:            "vless-443",
+		Remark:         "my-vless",
+		Enable:         true,
+		Listen:         "0.0.0.0",
+		Port:           443,
+		Protocol:       model.VLESS,
+		StreamSettings: `{"network":"tcp"}`,
+		Settings:       `{}`,
+	}
+	if err := database.GetDB().Create(seeded).Error; err != nil {
+		t.Fatalf("seed inbound: %v", err)
+	}
+
+	svc := &InboundService{}
+	candidate := &model.Inbound{
+		Tag:            "trojan-443",
+		Listen:         "0.0.0.0",
+		Port:           443,
+		Protocol:       model.Trojan,
+		StreamSettings: `{"network":"ws"}`,
+	}
+	got, err := svc.checkPortConflict(candidate, 0)
+	if err != nil || got == nil {
+		t.Fatalf("expected conflict, got=%v err=%v", got, err)
+	}
+	msg := got.String()
+	if !strings.Contains(msg, "my-vless") {
+		t.Fatalf("message should mention the conflicting inbound's remark; got %q", msg)
+	}
+	if !strings.Contains(msg, "tcp") {
+		t.Fatalf("message should mention the shared L4 transport; got %q", msg)
+	}
+	if !strings.Contains(msg, "443") {
+		t.Fatalf("message should mention the port; got %q", msg)
+	}
+}

Niektóre pliki nie zostały wyświetlone z powodu dużej ilości zmienionych plików