From d42e2133c78bda639097ba874854da566a3b8064 Mon Sep 17 00:00:00 2001 From: sdhfsl Date: Sun, 27 Sep 2026 04:00:09 +0800 Subject: [PATCH] fix(sub): keep serverDescription literal in external link fragments (#6580) * fix(panel): accept 2FA codes from adjacent TOTP windows CheckUser compared only gotp.Now(), so a code submitted at the end of its 30s window (or with slight client/server clock drift) failed with 'invalid 2fa code', while the immediate retry in the next window succeeded. Accept current +/-1 window, the standard TOTP skew tolerance. Fixes MHSanaei/3x-ui#6535 * fix(panel): share TOTP skew tolerance with VerifyTwoFactorCode Move the +/-1 window helper to internal/util/totp so both 2FA acceptance points use it: login (CheckUser) and disable/rebind plus username/password changes (VerifyTwoFactorCode). Also shrink comments to the 2-line house rule and anchor the unit test mid-window to avoid a step-boundary flake. Addresses review on #6546 (MEDIUM + 2 LOWs). * fix(sub): keep serverDescription literal in external link fragments Client external links escaped the whole remark, turning ?serverDescription= into %3F...%2F... so Happ lost its subtitle. Split on ?serverDescription= like appendQueryAndFragment (#6488): escape only the display name, keep a clean base64 tail literal, fall back to full escaping otherwise. Fixes MHSanaei/3x-ui#6575 * refactor(sub): share one serverDescription fragment split across link paths #6488 fixed the split in appendQueryAndFragment and #6575 was the same bug on the external-link path, which had its own copy. Both now call escapeLinkFragment with their own escaper, so a later change to the tail check cannot reach one path and miss the other. --------- Co-authored-by: sdhfsl Co-authored-by: MHSanaei --- internal/sub/external_config.go | 2 +- internal/sub/external_remark_test.go | 40 ++++++++++++++++++++++++++++ internal/sub/service.go | 31 ++++++++++++--------- 3 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 internal/sub/external_remark_test.go diff --git a/internal/sub/external_config.go b/internal/sub/external_config.go index 4fa88e07f..ed60be12f 100644 --- a/internal/sub/external_config.go +++ b/internal/sub/external_config.go @@ -163,7 +163,7 @@ func applyRemarkToLink(rawLink, remark string) string { if i := strings.IndexByte(rawLink, '#'); i >= 0 { rawLink = rawLink[:i] } - return rawLink + "#" + url.PathEscape(remark) + return rawLink + "#" + escapeLinkFragment(remark, url.PathEscape) } func applyVmessRemark(rawLink, remark string) string { diff --git a/internal/sub/external_remark_test.go b/internal/sub/external_remark_test.go new file mode 100644 index 000000000..963f0b9cd --- /dev/null +++ b/internal/sub/external_remark_test.go @@ -0,0 +1,40 @@ +package sub + +import ( + "strings" + "testing" +) + +// #6575: a trailing ?serverDescription= must stay literal in the +// fragment so Happ renders its subtitle; only the display name is escaped. +func TestApplyRemarkKeepsServerDescription(t *testing.T) { + link := "vless://00000000-0000-0000-0000-000000000000@example.com:443?type=tcp&security=reality&pbk=XXX&fp=chrome&sni=example.org&sid=00&flow=xtls-rprx-vision&encryption=none" + remark := "🇵🇱 Warsaw ⚡️?serverDescription=0JTQu9GPIExURSAo0LHQtdC70YvQtSDRgdC/0LjRgdC60Lgp" + + out := applyRemarkToLink(link, remark) + frag := out[strings.IndexByte(out, '#')+1:] + if !strings.Contains(frag, "?serverDescription=") { + t.Fatalf("serverDescription escaped: %s", out) + } + if strings.Contains(frag, "%3F") || strings.Contains(frag, "%2F") { + t.Fatalf("fragment over-escaped: %s", out) + } + tail := frag[strings.Index(frag, "?serverDescription=")+len("?serverDescription="):] + if strings.ContainsAny(tail, " \r\n\t#&") { + t.Fatalf("tail not clean base64: %q", tail) + } + if !strings.HasPrefix(out, link+"#") { + t.Fatalf("link body altered: %s", out) + } +} + +func TestApplyRemarkMalformedServerDescriptionFallsBack(t *testing.T) { + link := "vless://uuid@example.com:443?security=reality#old" + out := applyRemarkToLink(link, "name?serverDescription=not base64!!") + if strings.Contains(out, "?serverDescription=") { + t.Fatalf("malformed tail kept literal: %s", out) + } + if !strings.HasPrefix(out, link[:strings.IndexByte(link, '#')]+"#") { + t.Fatalf("link body altered: %s", out) + } +} diff --git a/internal/sub/service.go b/internal/sub/service.go index 3e14efdac..576e1d7b9 100644 --- a/internal/sub/service.go +++ b/internal/sub/service.go @@ -2330,22 +2330,29 @@ func appendQueryAndFragment(link string, params map[string]string, fragment, sec if fragment != "" { sb.WriteByte('#') - if before, after, ok := strings.Cut(fragment, "?serverDescription="); ok { - if _, err := base64.StdEncoding.DecodeString(after); err == nil && len(after) > 0 && !strings.ContainsAny(after, " \r\n\t#&") { - sb.WriteString(strings.ReplaceAll(url.QueryEscape(before), "+", "%20")) - sb.WriteString("?serverDescription=") - sb.WriteString(after) - } else { - sb.WriteString(strings.ReplaceAll(url.QueryEscape(fragment), "+", "%20")) - } - } else { - // Match the frontend's encodeURIComponent(remark): spaces become %20. - sb.WriteString(strings.ReplaceAll(url.QueryEscape(fragment), "+", "%20")) - } + sb.WriteString(escapeLinkFragment(fragment, encodeURIComponent)) } return sb.String() } +// encodeURIComponent matches the frontend's escaping of a remark: spaces become %20. +func encodeURIComponent(s string) string { + return strings.ReplaceAll(url.QueryEscape(s), "+", "%20") +} + +// escapeLinkFragment escapes a remark but keeps a valid ?serverDescription= +// tail literal, which Happ reads as the subtitle (#6488, #6575). +func escapeLinkFragment(fragment string, escape func(string) string) string { + before, after, ok := strings.Cut(fragment, "?serverDescription=") + if !ok || after == "" || strings.ContainsAny(after, " \r\n\t#&") { + return escape(fragment) + } + if _, err := base64.StdEncoding.DecodeString(after); err != nil { + return escape(fragment) + } + return escape(before) + "?serverDescription=" + after +} + // buildExternalProxyURLLinks is a thin adapter: it maps the legacy externalProxy // entries to []ShareEndpoint and renders them through the unified endpoint path. // Kept so the genVless/genTrojan/genShadowsocks call sites are unchanged.