Browse Source

fix(sub): quote Clash scalars a YAML parser would read as numbers (#6104)

A REALITY short-id like 2351e1 is valid hex, but as a bare YAML scalar the
resolution rules read it as the float 23510. mihomo hex-decodes the resulting
five-digit string, fails with "invalid REALITY short ID", and the whole
provider loads zero nodes — one proxy takes the entire subscription down.

The encoder quotes the forms it recognises (plain integers, hex, booleans)
but not the exponent-float form, and its own parser reads that token back as
a string, so nothing in a round-trip through it reveals the problem. Check
the values against the resolution rules instead, and force quotes on any
plain scalar that would resolve to a non-string.

Applied to every string in the document rather than to short-id alone: the
panel's own short-id generator emits random hex, and passwords, obfs-
passwords and pre-shared keys reach the output the same way. Unambiguous
values are untouched, so the document is otherwise byte-identical.

The existing Clash tests assert on the config map, never on the serialized
text, which is why this survived; the new tests assert on the output.
Sanaei 9 hours ago
parent
commit
7fe9932d7b
3 changed files with 248 additions and 1 deletions
  1. 1 1
      internal/sub/clash_service.go
  2. 106 0
      internal/sub/clash_yaml.go
  3. 141 0
      internal/sub/clash_yaml_test.go

+ 1 - 1
internal/sub/clash_service.go

@@ -103,7 +103,7 @@ func (s *SubClashService) GetClash(subId string, host string) (string, string, e
 		}
 	}
 
-	finalYAML, err := yaml.Marshal(config)
+	finalYAML, err := marshalClashYAML(config)
 	if err != nil {
 		return "", "", err
 	}

+ 106 - 0
internal/sub/clash_yaml.go

@@ -0,0 +1,106 @@
+package sub
+
+import (
+	"reflect"
+	"regexp"
+	"strings"
+
+	"github.com/goccy/go-yaml"
+)
+
+// yamlQuotedString is a string that must reach a YAML parser as a string.
+// Single-quoted style is used because it carries the value literally — no
+// escape processing — which suits hex ids, passwords and pre-shared keys.
+type yamlQuotedString string
+
+func (s yamlQuotedString) MarshalYAML() ([]byte, error) {
+	return []byte("'" + strings.ReplaceAll(string(s), "'", "''") + "'"), nil
+}
+
+// yamlPlainScalarNotString matches the plain scalars a YAML parser resolves to
+// something other than a string. It covers the YAML 1.2 core schema plus the
+// 1.1 forms go-yaml v3 — the library the Clash cores use — still resolves:
+// null, booleans (including y/yes/on and friends), integers in every base,
+// sexagesimals, floats, and dates.
+var yamlPlainScalarNotString = regexp.MustCompile(`^(?:` +
+	`~|[Nn]ull|NULL|` +
+	`[Tt]rue|TRUE|[Ff]alse|FALSE|[Yy]es|YES|[Nn]o|NO|[Oo]n|ON|[Oo]ff|OFF|[YyNn]|` +
+	`[-+]?[0-9]+|` +
+	`0[oO]?[0-7]+|0[xX][0-9a-fA-F]+|` +
+	`[-+]?[0-9][0-9_]*(?::[0-5]?[0-9])+|` +
+	`[-+]?(?:[0-9]*\.[0-9]+|[0-9]+\.?[0-9]*)(?:[eE][-+]?[0-9]+)?|` +
+	`[-+]?\.(?:inf|Inf|INF)|\.(?:nan|NaN|NAN)|` +
+	`[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}(?:[Tt ].*)?` +
+	`)$`)
+
+// yamlScalarIsAmbiguous reports whether s must be quoted to survive as a string.
+//
+// The encoder quotes most of these itself, but not the exponent-float form: a
+// REALITY short-id such as "2351e1" is valid hex and, emitted bare, is read as
+// 23510. A Clash core then hex-decodes a five-digit number, rejects the proxy
+// and drops the whole provider to zero nodes (#6104). goccy's own parser reads
+// that token back as a string, so the encoder never sees a problem and a
+// round-trip check through it cannot find one either — the resolution rules,
+// not the encoder, are the thing to test against. The same shape reaches
+// passwords, obfs-passwords and pre-shared keys, so every string in the
+// document is checked rather than an enumerated list of fields.
+func yamlScalarIsAmbiguous(s string) bool {
+	if s == "" {
+		return false
+	}
+	return yamlPlainScalarNotString.MatchString(s)
+}
+
+// quoteAmbiguousYAMLScalars rebuilds v with every ambiguous string wrapped so
+// the encoder is forced to quote it. Containers are rebuilt as map[string]any /
+// []any because a typed container cannot hold the wrapper; unambiguous values
+// are carried through untouched, so the emitted document is unchanged apart
+// from the quotes that were missing.
+func quoteAmbiguousYAMLScalars(v any) any {
+	if v == nil {
+		return nil
+	}
+	if s, ok := v.(string); ok {
+		if yamlScalarIsAmbiguous(s) {
+			return yamlQuotedString(s)
+		}
+		return s
+	}
+
+	rv := reflect.ValueOf(v)
+	switch rv.Kind() {
+	case reflect.Map:
+		out := make(map[string]any, rv.Len())
+		iter := rv.MapRange()
+		for iter.Next() {
+			key, ok := iter.Key().Interface().(string)
+			if !ok {
+				return v
+			}
+			out[key] = quoteAmbiguousYAMLScalars(iter.Value().Interface())
+		}
+		return out
+	case reflect.Slice, reflect.Array:
+		if rv.Kind() == reflect.Slice && rv.Type().Elem().Kind() == reflect.Uint8 {
+			return v
+		}
+		out := make([]any, rv.Len())
+		for i := range rv.Len() {
+			out[i] = quoteAmbiguousYAMLScalars(rv.Index(i).Interface())
+		}
+		return out
+	case reflect.Ptr, reflect.Interface:
+		if rv.IsNil() {
+			return v
+		}
+		return quoteAmbiguousYAMLScalars(rv.Elem().Interface())
+	default:
+		return v
+	}
+}
+
+// marshalClashYAML serializes a Clash config, keeping string values typed as
+// strings in the output.
+func marshalClashYAML(config any) ([]byte, error) {
+	return yaml.Marshal(quoteAmbiguousYAMLScalars(config))
+}

+ 141 - 0
internal/sub/clash_yaml_test.go

@@ -0,0 +1,141 @@
+package sub
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/goccy/go-yaml"
+)
+
+// The encoder's own parser reads every one of these back as a string, so a
+// round-trip through it proves nothing. What matters is the emitted text: a
+// plain scalar that the YAML resolution rules turn into a number, bool, null
+// or date is what breaks a Clash core, so those must carry quotes.
+func TestAmbiguousScalarsAreQuoted(t *testing.T) {
+	tests := []struct {
+		name      string
+		value     string
+		mustQuote bool
+	}{
+		{"reality short-id read as a float", "2351e1", true},
+		{"short exponent form", "0e1", true},
+		{"long exponent form", "12e34", true},
+		{"all digits", "123456", true},
+		{"leading zeros read as octal", "0177", true},
+		{"hex form", "0x1f", true},
+		{"decimal point", "1.5", true},
+		{"boolean word", "true", true},
+		{"single letter bool", "y", true},
+		{"null word", "null", true},
+		{"tilde", "~", true},
+		{"date", "2026-07-27", true},
+		{"sexagesimal", "12:30", true},
+		{"plain hex with letters", "6ba7b8", false},
+		{"letters only", "abcdef", false},
+		{"hostname", "example.com", false},
+		{"dotted quad", "1.2.3.4", false},
+		{"uuid", "6ba7b810-9dad-11d1-80b4-00c04fd430c8", false},
+		{"alpn token", "h2", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			out, err := marshalClashYAML(map[string]any{
+				"reality-opts": map[string]any{"short-id": tt.value},
+			})
+			if err != nil {
+				t.Fatalf("marshalClashYAML: %v", err)
+			}
+			got := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(string(out)), "reality-opts:"))
+			quoted := strings.Contains(got, "'"+tt.value+"'") || strings.Contains(got, `"`+tt.value+`"`)
+			if tt.mustQuote && !quoted {
+				t.Errorf("%q must be emitted quoted, got %q", tt.value, got)
+			}
+			if !tt.mustQuote && quoted {
+				t.Errorf("%q needs no quoting, got %q", tt.value, got)
+			}
+			// Whatever the quoting decision, the document must still parse.
+			var decoded map[string]any
+			if err := yaml.Unmarshal(out, &decoded); err != nil {
+				t.Fatalf("output must stay parseable: %v\n%s", err, out)
+			}
+		})
+	}
+}
+
+// The value in the report: unquoted, YAML reads 2351e1 as 23510, mihomo hex-
+// decodes an odd-length "23510" and the provider drops to zero nodes (#6104).
+func TestClashShortIDIsQuotedInOutput(t *testing.T) {
+	out, err := marshalClashYAML(map[string]any{
+		"proxies": []any{map[string]any{
+			"name":         "de-1",
+			"reality-opts": map[string]any{"short-id": "2351e1"},
+		}},
+	})
+	if err != nil {
+		t.Fatalf("marshalClashYAML: %v", err)
+	}
+	if !strings.Contains(string(out), `'2351e1'`) {
+		t.Errorf("short-id must be emitted as a quoted scalar, got:\n%s", out)
+	}
+}
+
+// A password or pre-shared key of the same shape reaches the output the same
+// way, so the guard is not specific to short-id.
+func TestAmbiguousPasswordIsQuoted(t *testing.T) {
+	out, err := marshalClashYAML(map[string]any{
+		"proxies": []any{map[string]any{
+			"name":           "de-1",
+			"password":       "80e12",
+			"obfs-password":  "12345",
+			"pre-shared-key": "9e9",
+		}},
+	})
+	if err != nil {
+		t.Fatalf("marshalClashYAML: %v", err)
+	}
+	for _, want := range []string{`'80e12'`, `'12345'`, `'9e9'`} {
+		if !strings.Contains(string(out), want) {
+			t.Errorf("expected %s in output:\n%s", want, out)
+		}
+	}
+}
+
+// Values that are unambiguous must not pick up noise quoting, so the emitted
+// document stays byte-identical to what the encoder produced before.
+func TestUnambiguousScalarsAreUnchanged(t *testing.T) {
+	config := map[string]any{
+		"port":     7890,
+		"mode":     "rule",
+		"proxies":  []any{map[string]any{"name": "de-1", "udp": true, "port": 443}},
+		"rules":    []string{"MATCH,PROXY"},
+		"alpn":     []string{"h2", "http/1.1"},
+		"nonempty": "example.com",
+	}
+	quoted, err := marshalClashYAML(config)
+	if err != nil {
+		t.Fatalf("marshalClashYAML: %v", err)
+	}
+	plain, err := yaml.Marshal(config)
+	if err != nil {
+		t.Fatalf("yaml.Marshal: %v", err)
+	}
+	if string(quoted) != string(plain) {
+		t.Errorf("unambiguous document changed:\n--- got ---\n%s\n--- want ---\n%s", quoted, plain)
+	}
+}
+
+// A quote inside the value must not terminate the scalar.
+func TestQuotedScalarEscapesQuotes(t *testing.T) {
+	out, err := marshalClashYAML(map[string]any{"password": "1e2'3"})
+	if err != nil {
+		t.Fatalf("marshalClashYAML: %v", err)
+	}
+	var decoded map[string]any
+	if err := yaml.Unmarshal(out, &decoded); err != nil {
+		t.Fatalf("output must stay parseable: %v\n%s", err, out)
+	}
+	if got := decoded["password"]; got != any("1e2'3") {
+		t.Errorf("password round-tripped to %#v\n%s", got, out)
+	}
+}