Browse Source

fix(limit-ip): leave a reverse client out of the temporary disconnect (#6553)

* fix(limit-ip): leave a reverse client out of the temporary disconnect

The LIMIT_IP cycle removes the client and adds it back 100 ms later. For a vless
client carrying a reverse config that is not reversible: RemoveUser calls
RemoveReverse and deletes the client's outbound handler, while the account added
back is built without the reverse field, so the tunnel stays down until Xray
restarts and the core's forward-proxy guard for that client no longer fires
(proxy/vless/inbound/inbound.go:245 and :542-544 at the pinned core). The cycle
now skips such a client and says so, instead of trading a limit violation for a
tunnel that needs a restart to come back.

TestDisconnectClientTemporarilySkipsReverseClient fails without this -- watched
red, the client is removed and re-added -- and asserts the skip is logged rather
than silent.

* style(limit-ip): keep the reverse-client comment within the 2-line cap

The block explaining why a reverse client is skipped was three lines, against
the rule this repo sets for committed Go comments; the same why fits in two.
BlindMaster24 20 hours ago
parent
commit
d9c7c76fb0

+ 11 - 2
internal/web/job/check_client_ip_job.go

@@ -627,11 +627,13 @@ func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, c
 
 
 	// Find the client config
 	// Find the client config
 	var clientConfig map[string]any
 	var clientConfig map[string]any
+	var reverseClient bool
 	for _, client := range clients {
 	for _, client := range clients {
 		if client.Email == clientEmail {
 		if client.Email == clientEmail {
 			// Convert client to map for API
 			// Convert client to map for API
 			clientBytes, _ := json.Marshal(client)
 			clientBytes, _ := json.Marshal(client)
 			_ = json.Unmarshal(clientBytes, &clientConfig)
 			_ = json.Unmarshal(clientBytes, &clientConfig)
+			reverseClient = client.Reverse != nil
 			break
 			break
 		}
 		}
 	}
 	}
@@ -651,6 +653,13 @@ func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, c
 		return
 		return
 	}
 	}
 
 
+	// RemoveUser drops a reverse client's outbound handler and the re-add below
+	// cannot restore it, so its tunnel would stay down until Xray restarts.
+	if reverseClient {
+		logger.Warningf("[LIMIT_IP] Not disconnecting %s: its reverse proxy config does not survive a temporary removal", clientEmail)
+		return
+	}
+
 	// For Shadowsocks, ensure the required "cipher" field is present by
 	// For Shadowsocks, ensure the required "cipher" field is present by
 	// reading it from the inbound settings (e.g., settings["method"]).
 	// reading it from the inbound settings (e.g., settings["method"]).
 	if string(inbound.Protocol) == "shadowsocks" {
 	if string(inbound.Protocol) == "shadowsocks" {
@@ -664,8 +673,8 @@ func (j *CheckClientIpJob) disconnectClientTemporarily(inbound *model.Inbound, c
 		}
 		}
 	}
 	}
 
 
-	// The core's RemoveUser clears its validator, so a session already up keeps
-	// running -- except vless, where it also drops a reverse handler.
+	// The core's RemoveUser clears its validator: a session already up keeps
+	// running, except a reverse vless client, which is skipped above.
 	err = xrayAPI.RemoveUser(inbound.Tag, clientEmail)
 	err = xrayAPI.RemoveUser(inbound.Tag, clientEmail)
 	if err != nil {
 	if err != nil {
 		logger.Warningf("[LIMIT_IP] Failed to remove user %s: %v", clientEmail, err)
 		logger.Warningf("[LIMIT_IP] Failed to remove user %s: %v", clientEmail, err)

+ 33 - 0
internal/web/job/limit_ip_disconnect_test.go

@@ -8,6 +8,39 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
 	"github.com/mhsanaei/3x-ui/v3/internal/logger"
 )
 )
 
 
+// A reverse client has to be left alone: the cycle drops its reverse outbound
+// handler and re-adds an account without a reverse, which no retry restores.
+func TestDisconnectClientTemporarilySkipsReverseClient(t *testing.T) {
+	setupIntegrationDB(t)
+
+	const email = "rev-limit-probe"
+	inbound := &model.Inbound{
+		Id:       1,
+		Protocol: model.VLESS,
+		Tag:      "rev-limit-probe-tag",
+		Settings: `{"clients":[]}`,
+	}
+	clients := []model.Client{{Email: email, ID: "11111111-1111-1111-1111-111111111111", Reverse: &model.ClientReverse{Tag: "rev-out"}}}
+
+	(&CheckClientIpJob{}).disconnectClientTemporarily(inbound, email, clients)
+
+	var skipped, attempted bool
+	for _, line := range logger.GetLogs(500, "warning") {
+		if strings.Contains(line, "Not disconnecting "+email) {
+			skipped = true
+		}
+		if strings.Contains(line, "Failed to remove user "+email) {
+			attempted = true
+		}
+	}
+	if attempted {
+		t.Fatal("a reverse client must not be removed and re-added")
+	}
+	if !skipped {
+		t.Fatal("the skip must be reported, not silent")
+	}
+}
+
 // The protocol gate must let hysteria through: XrayAPI supports it, and the
 // The protocol gate must let hysteria through: XrayAPI supports it, and the
 // skip left over-limit Hysteria2 sessions alive until the fail2ban ban caught up.
 // skip left over-limit Hysteria2 sessions alive until the fail2ban ban caught up.
 func TestDisconnectClientTemporarilyAllowsHysteria(t *testing.T) {
 func TestDisconnectClientTemporarilyAllowsHysteria(t *testing.T) {