Explorar el Código

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=<base64> 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 <[email protected]>
Co-authored-by: MHSanaei <[email protected]>
sdhfsl hace 13 horas
padre
commit
d42e2133c7
Se han modificado 3 ficheros con 60 adiciones y 13 borrados
  1. 1 1
      internal/sub/external_config.go
  2. 40 0
      internal/sub/external_remark_test.go
  3. 19 12
      internal/sub/service.go

+ 1 - 1
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 {

+ 40 - 0
internal/sub/external_remark_test.go

@@ -0,0 +1,40 @@
+package sub
+
+import (
+	"strings"
+	"testing"
+)
+
+// #6575: a trailing ?serverDescription=<base64> must stay literal in the
+// fragment so Happ renders its subtitle; only the display name is escaped.
+func TestApplyRemarkKeepsServerDescription(t *testing.T) {
+	link := "vless://[email protected]: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://[email protected]: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)
+	}
+}

+ 19 - 12
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=<base64>
+// 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.