Просмотр исходного кода

xray-setting: pin api routing rule to index 0 on save (#4124)

when the admin adds a custom outbound (eg vless cascade to a second
server) and a routing rule sending all inbound traffic to it, that
catch-all gets evaluated before the existing api->api rule, so the
panel's internal stats inbound's traffic ends up on the cascade
outbound. the grpc stats query then can't see anything, GetTraffic
returns no inbound/user counters, and every client appears offline
with zero traffic even though the actual proxy path works fine.

before save, find the api rule and move it to the front of
routing.rules. if it's missing entirely, insert a default. other
rules keep their relative order.

closes #4113. probably also fixes the long-standing #2818 where the
documented workaround was "manually move the api rule to the top".
pwnnex 8 часов назад
Родитель
Сommit
22de983752
2 измененных файлов с 235 добавлено и 0 удалено
  1. 120 0
      web/service/xray_setting.go
  2. 115 0
      web/service/xray_setting_test.go

+ 120 - 0
web/service/xray_setting.go

@@ -24,6 +24,9 @@ func (s *XraySettingService) SaveXraySetting(newXraySettings string) error {
 	if err := s.CheckXrayConfig(newXraySettings); err != nil {
 		return err
 	}
+	if hoisted, err := EnsureStatsRouting(newXraySettings); err == nil {
+		newXraySettings = hoisted
+	}
 	return s.SettingService.saveSetting("xrayTemplateConfig", newXraySettings)
 }
 
@@ -83,3 +86,120 @@ func UnwrapXrayTemplateConfig(raw string) string {
 	}
 	return raw
 }
+
+// EnsureStatsRouting hoists the `api -> api` routing rule to the front
+// of routing.rules so the stats query path is never starved by a
+// catch-all rule the admin may have added or reordered above it.
+//
+// Why this matters (#4113, #2818): an admin who adds a cascade outbound
+// (e.g. vless to another server) and a routing rule sending all inbound
+// traffic to it ends up sending the internal stats inbound's traffic to
+// that cascade too, since rules are evaluated top-to-bottom and the
+// catch-all matches first. The panel's gRPC stats query then can't reach
+// the running xray instance, GetTraffic returns nothing, and every
+// client appears offline with zero traffic even though the actual proxy
+// path works fine.
+//
+// The api inbound is special-cased internal infrastructure for the
+// panel, not something the admin should ever route to a real outbound.
+// Keeping its rule pinned at index 0 is the only correct configuration.
+//
+// If the api rule is already at index 0 the input is returned unchanged.
+// If it exists somewhere else it is moved. If it is missing entirely a
+// default rule (`type=field, inboundTag=[api], outboundTag=api`) is
+// inserted at the front. Other routing entries keep their relative order.
+func EnsureStatsRouting(raw string) (string, error) {
+	var cfg map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		return raw, err
+	}
+
+	var routing map[string]json.RawMessage
+	if r, ok := cfg["routing"]; ok && len(r) > 0 {
+		if err := json.Unmarshal(r, &routing); err != nil {
+			return raw, err
+		}
+	}
+	if routing == nil {
+		routing = make(map[string]json.RawMessage)
+	}
+
+	var rules []map[string]any
+	if r, ok := routing["rules"]; ok && len(r) > 0 {
+		if err := json.Unmarshal(r, &rules); err != nil {
+			return raw, err
+		}
+	}
+
+	apiIdx := findApiRule(rules)
+	if apiIdx == 0 {
+		return raw, nil // already correct, don't churn the JSON
+	}
+
+	var apiRule map[string]any
+	if apiIdx > 0 {
+		apiRule = rules[apiIdx]
+		rules = append(rules[:apiIdx], rules[apiIdx+1:]...)
+	} else {
+		apiRule = map[string]any{
+			"type":        "field",
+			"inboundTag":  []string{"api"},
+			"outboundTag": "api",
+		}
+	}
+	rules = append([]map[string]any{apiRule}, rules...)
+
+	rulesJSON, err := json.Marshal(rules)
+	if err != nil {
+		return raw, err
+	}
+	routing["rules"] = rulesJSON
+
+	routingJSON, err := json.Marshal(routing)
+	if err != nil {
+		return raw, err
+	}
+	cfg["routing"] = routingJSON
+
+	out, err := json.Marshal(cfg)
+	if err != nil {
+		return raw, err
+	}
+	return string(out), nil
+}
+
+// findApiRule returns the index of the routing rule that targets the
+// internal api inbound (inboundTag contains "api" and outboundTag is
+// "api"), or -1 if no such rule exists.
+func findApiRule(rules []map[string]any) int {
+	for i, rule := range rules {
+		if outTag, _ := rule["outboundTag"].(string); outTag != "api" {
+			continue
+		}
+		raw, ok := rule["inboundTag"]
+		if !ok {
+			continue
+		}
+		// inboundTag is usually []string but can come as []any from a
+		// roundtrip through map[string]any. Accept both shapes.
+		switch tags := raw.(type) {
+		case []any:
+			for _, t := range tags {
+				if s, ok := t.(string); ok && s == "api" {
+					return i
+				}
+			}
+		case []string:
+			for _, s := range tags {
+				if s == "api" {
+					return i
+				}
+			}
+		case string:
+			if tags == "api" {
+				return i
+			}
+		}
+	}
+	return -1
+}

+ 115 - 0
web/service/xray_setting_test.go

@@ -88,3 +88,118 @@ func equalJSON(t *testing.T, a, b string) bool {
 	jb, _ := json.Marshal(vb)
 	return string(ja) == string(jb)
 }
+
+// firstRuleOutbound parses the (post-hoisted) config and returns
+// routing.rules[0].outboundTag, or "" if anything is missing.
+func firstRuleOutbound(t *testing.T, raw string) string {
+	t.Helper()
+	var cfg map[string]any
+	if err := json.Unmarshal([]byte(raw), &cfg); err != nil {
+		t.Fatalf("unmarshal cfg: %v", err)
+	}
+	routing, _ := cfg["routing"].(map[string]any)
+	rules, _ := routing["rules"].([]any)
+	if len(rules) == 0 {
+		return ""
+	}
+	first, _ := rules[0].(map[string]any)
+	tag, _ := first["outboundTag"].(string)
+	return tag
+}
+
+func TestEnsureStatsRouting_HoistsApiRuleFromMiddle(t *testing.T) {
+	// #4113 repro shape: admin added a cascade outbound and put a
+	// catch-all routing rule above the api rule. stats query path
+	// gets starved by the catch-all unless we hoist the api rule.
+	in := `{
+		"routing": {
+			"rules": [
+				{"type":"field","inboundTag":["inbound-vless"],"outboundTag":"vless-cascade"},
+				{"type":"field","inboundTag":["api"],"outboundTag":"api"},
+				{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}
+			]
+		}
+	}`
+	out, err := EnsureStatsRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if got := firstRuleOutbound(t, out); got != "api" {
+		t.Fatalf("api rule should be at index 0 after hoist, got first outboundTag = %q\nfull: %s", got, out)
+	}
+}
+
+func TestEnsureStatsRouting_NoOpWhenAlreadyFirst(t *testing.T) {
+	// Don't churn the JSON when nothing needs fixing — same string in,
+	// same string out. Lets the diff in the panel UI stay quiet for
+	// well-formed configs.
+	in := `{"routing":{"rules":[{"type":"field","inboundTag":["api"],"outboundTag":"api"},{"type":"field","outboundTag":"blocked","ip":["geoip:private"]}]}}`
+	out, err := EnsureStatsRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if out != in {
+		t.Fatalf("expected unchanged input, got: %s", out)
+	}
+}
+
+func TestEnsureStatsRouting_InsertsDefaultWhenMissing(t *testing.T) {
+	// Some admins delete the api rule by accident. Re-add a default
+	// at the front so stats keep working after the next save.
+	in := `{"routing":{"rules":[{"type":"field","outboundTag":"vless-cascade","inboundTag":["inbound-vless"]}]}}`
+	out, err := EnsureStatsRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if got := firstRuleOutbound(t, out); got != "api" {
+		t.Fatalf("default api rule should be inserted at index 0, got %q\nfull: %s", got, out)
+	}
+	// The original rule should still be there, just shifted.
+	var cfg map[string]any
+	json.Unmarshal([]byte(out), &cfg)
+	rules := cfg["routing"].(map[string]any)["rules"].([]any)
+	if len(rules) != 2 {
+		t.Fatalf("expected 2 rules after insert, got %d: %v", len(rules), rules)
+	}
+}
+
+func TestEnsureStatsRouting_NoRoutingBlock(t *testing.T) {
+	// Pathological but possible: empty config or one without a routing
+	// section. Don't crash, and create the section with the api rule.
+	in := `{"log":{}}`
+	out, err := EnsureStatsRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if got := firstRuleOutbound(t, out); got != "api" {
+		t.Fatalf("api rule should be created when routing was missing, got %q\nfull: %s", got, out)
+	}
+}
+
+func TestEnsureStatsRouting_InvalidJsonReturnsAsIs(t *testing.T) {
+	// SaveXraySetting calls CheckXrayConfig before this helper, so
+	// invalid JSON shouldn't reach us in practice — but be defensive
+	// about garbage in (return same garbage out plus an error) so the
+	// caller can choose to skip the hoist instead of corrupting input.
+	in := "definitely not json"
+	out, err := EnsureStatsRouting(in)
+	if err == nil {
+		t.Fatalf("expected error for invalid json, got none")
+	}
+	if out != in {
+		t.Fatalf("expected raw passthrough on error, got %q", out)
+	}
+}
+
+func TestEnsureStatsRouting_AcceptsInboundTagAsString(t *testing.T) {
+	// Some manually-edited configs use a single string instead of an
+	// array for inboundTag. Make sure we still recognize the api rule.
+	in := `{"routing":{"rules":[{"type":"field","inboundTag":["other"],"outboundTag":"vless-cascade"},{"type":"field","inboundTag":"api","outboundTag":"api"}]}}`
+	out, err := EnsureStatsRouting(in)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if got := firstRuleOutbound(t, out); got != "api" {
+		t.Fatalf("api rule with string-form inboundTag should hoist to front, got %q\nfull: %s", got, out)
+	}
+}