mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-08-29 06:27:14 +00:00
style: drop the line comments added with the triage fixes
CLAUDE.md rules out // line comments in committed Go. The rationale they carried is in the commit messages for each fix; doc comments that already existed are kept, updated where the code they describe changed. Also replaces reflect.Ptr with reflect.Pointer and rewrites the YAML keyword alternation as a lookup table, both flagged by golangci-lint.
This commit is contained in:
+14
-31
@@ -8,23 +8,24 @@ import (
|
|||||||
"github.com/goccy/go-yaml"
|
"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
|
type yamlQuotedString string
|
||||||
|
|
||||||
func (s yamlQuotedString) MarshalYAML() ([]byte, error) {
|
func (s yamlQuotedString) MarshalYAML() ([]byte, error) {
|
||||||
return []byte("'" + strings.ReplaceAll(string(s), "'", "''") + "'"), nil
|
return []byte("'" + strings.ReplaceAll(string(s), "'", "''") + "'"), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// yamlPlainScalarNotString matches the plain scalars a YAML parser resolves to
|
var yamlNonStringWords = map[string]bool{
|
||||||
// something other than a string. It covers the YAML 1.2 core schema plus the
|
"~": true, "null": true, "Null": true, "NULL": true,
|
||||||
// 1.1 forms go-yaml v3 — the library the Clash cores use — still resolves:
|
"true": true, "True": true, "TRUE": true,
|
||||||
// null, booleans (including y/yes/on and friends), integers in every base,
|
"false": true, "False": true, "FALSE": true,
|
||||||
// sexagesimals, floats, and dates.
|
"yes": true, "Yes": true, "YES": true,
|
||||||
var yamlPlainScalarNotString = regexp.MustCompile(`^(?:` +
|
"no": true, "No": true, "NO": true,
|
||||||
`~|[Nn]ull|NULL|` +
|
"on": true, "On": true, "ON": true,
|
||||||
`[Tt]rue|TRUE|[Ff]alse|FALSE|[Yy]es|YES|[Nn]o|NO|[Oo]n|ON|[Oo]ff|OFF|[YyNn]|` +
|
"off": true, "Off": true, "OFF": true,
|
||||||
|
"y": true, "Y": true, "n": true, "N": true,
|
||||||
|
}
|
||||||
|
|
||||||
|
var yamlNonStringNumber = regexp.MustCompile(`^(?:` +
|
||||||
`[-+]?[0-9]+|` +
|
`[-+]?[0-9]+|` +
|
||||||
`0[oO]?[0-7]+|0[xX][0-9a-fA-F]+|` +
|
`0[oO]?[0-7]+|0[xX][0-9a-fA-F]+|` +
|
||||||
`[-+]?[0-9][0-9_]*(?::[0-5]?[0-9])+|` +
|
`[-+]?[0-9][0-9_]*(?::[0-5]?[0-9])+|` +
|
||||||
@@ -33,29 +34,13 @@ var yamlPlainScalarNotString = regexp.MustCompile(`^(?:` +
|
|||||||
`[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}(?:[Tt ].*)?` +
|
`[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 {
|
func yamlScalarIsAmbiguous(s string) bool {
|
||||||
if s == "" {
|
if s == "" {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
return yamlPlainScalarNotString.MatchString(s)
|
return yamlNonStringWords[s] || yamlNonStringNumber.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 {
|
func quoteAmbiguousYAMLScalars(v any) any {
|
||||||
if v == nil {
|
if v == nil {
|
||||||
return nil
|
return nil
|
||||||
@@ -89,7 +74,7 @@ func quoteAmbiguousYAMLScalars(v any) any {
|
|||||||
out[i] = quoteAmbiguousYAMLScalars(rv.Index(i).Interface())
|
out[i] = quoteAmbiguousYAMLScalars(rv.Index(i).Interface())
|
||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
case reflect.Ptr, reflect.Interface:
|
case reflect.Pointer, reflect.Interface:
|
||||||
if rv.IsNil() {
|
if rv.IsNil() {
|
||||||
return v
|
return v
|
||||||
}
|
}
|
||||||
@@ -99,8 +84,6 @@ func quoteAmbiguousYAMLScalars(v any) any {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// marshalClashYAML serializes a Clash config, keeping string values typed as
|
|
||||||
// strings in the output.
|
|
||||||
func marshalClashYAML(config any) ([]byte, error) {
|
func marshalClashYAML(config any) ([]byte, error) {
|
||||||
return yaml.Marshal(quoteAmbiguousYAMLScalars(config))
|
return yaml.Marshal(quoteAmbiguousYAMLScalars(config))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,10 +7,6 @@ import (
|
|||||||
"github.com/goccy/go-yaml"
|
"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) {
|
func TestAmbiguousScalarsAreQuoted(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -54,7 +50,6 @@ func TestAmbiguousScalarsAreQuoted(t *testing.T) {
|
|||||||
if !tt.mustQuote && quoted {
|
if !tt.mustQuote && quoted {
|
||||||
t.Errorf("%q needs no quoting, got %q", tt.value, got)
|
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
|
var decoded map[string]any
|
||||||
if err := yaml.Unmarshal(out, &decoded); err != nil {
|
if err := yaml.Unmarshal(out, &decoded); err != nil {
|
||||||
t.Fatalf("output must stay parseable: %v\n%s", err, out)
|
t.Fatalf("output must stay parseable: %v\n%s", err, out)
|
||||||
@@ -63,8 +58,6 @@ func TestAmbiguousScalarsAreQuoted(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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) {
|
func TestClashShortIDIsQuotedInOutput(t *testing.T) {
|
||||||
out, err := marshalClashYAML(map[string]any{
|
out, err := marshalClashYAML(map[string]any{
|
||||||
"proxies": []any{map[string]any{
|
"proxies": []any{map[string]any{
|
||||||
@@ -80,8 +73,6 @@ func TestClashShortIDIsQuotedInOutput(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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) {
|
func TestAmbiguousPasswordIsQuoted(t *testing.T) {
|
||||||
out, err := marshalClashYAML(map[string]any{
|
out, err := marshalClashYAML(map[string]any{
|
||||||
"proxies": []any{map[string]any{
|
"proxies": []any{map[string]any{
|
||||||
@@ -101,8 +92,6 @@ func TestAmbiguousPasswordIsQuoted(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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) {
|
func TestUnambiguousScalarsAreUnchanged(t *testing.T) {
|
||||||
config := map[string]any{
|
config := map[string]any{
|
||||||
"port": 7890,
|
"port": 7890,
|
||||||
@@ -125,7 +114,6 @@ func TestUnambiguousScalarsAreUnchanged(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// A quote inside the value must not terminate the scalar.
|
|
||||||
func TestQuotedScalarEscapesQuotes(t *testing.T) {
|
func TestQuotedScalarEscapesQuotes(t *testing.T) {
|
||||||
out, err := marshalClashYAML(map[string]any{"password": "1e2'3"})
|
out, err := marshalClashYAML(map[string]any{"password": "1e2'3"})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -624,9 +624,6 @@ func TestUsageOnFirstLinkOnly_SingleBracket(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Every link of a subscription carries the client's identity, because that is
|
|
||||||
// what tells one imported profile from another in the client app. Only the
|
|
||||||
// usage block, which is identical on all of them, is first-link-only (#6098).
|
|
||||||
func TestEmailOnEveryLink(t *testing.T) {
|
func TestEmailOnEveryLink(t *testing.T) {
|
||||||
s := &SubService{
|
s := &SubService{
|
||||||
remarkTemplate: "{{INBOUND}} {{EMAIL}}|📊{{TRAFFIC_LEFT}}",
|
remarkTemplate: "{{INBOUND}} {{EMAIL}}|📊{{TRAFFIC_LEFT}}",
|
||||||
|
|||||||
@@ -10,9 +10,6 @@ import (
|
|||||||
"github.com/valyala/fasthttp"
|
"github.com/valyala/fasthttp"
|
||||||
)
|
)
|
||||||
|
|
||||||
// stallingListener accepts connections, reads whatever is sent and then never
|
|
||||||
// answers and never closes — the receiver shape that used to hold the traffic
|
|
||||||
// job open indefinitely (#6115).
|
|
||||||
func stallingListener(t *testing.T) (addr string, release func()) {
|
func stallingListener(t *testing.T) (addr string, release func()) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
||||||
@@ -65,7 +62,6 @@ func TestExternalInformClient_StalledReceiverTimesOut(t *testing.T) {
|
|||||||
if !errors.Is(err, fasthttp.ErrTimeout) {
|
if !errors.Is(err, fasthttp.ErrTimeout) {
|
||||||
t.Errorf("want a timeout error, got %v", err)
|
t.Errorf("want a timeout error, got %v", err)
|
||||||
}
|
}
|
||||||
// The whole point is that the call cannot outlast the 5s poll cadence.
|
|
||||||
if elapsed > externalInformTimeout+2*time.Second {
|
if elapsed > externalInformTimeout+2*time.Second {
|
||||||
t.Errorf("call took %v, must be bounded by %v", elapsed, externalInformTimeout)
|
t.Errorf("call took %v, must be bounded by %v", elapsed, externalInformTimeout)
|
||||||
}
|
}
|
||||||
@@ -80,8 +76,6 @@ func TestExternalInformClient_HasDeadlines(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// An idle panel posts nothing, which keeps a misbehaving receiver out of the
|
|
||||||
// job's path entirely for most ticks.
|
|
||||||
func TestInformSkippedWhenNothingToReport(t *testing.T) {
|
func TestInformSkippedWhenNothingToReport(t *testing.T) {
|
||||||
j := &XrayTrafficJob{}
|
j := &XrayTrafficJob{}
|
||||||
done := make(chan struct{})
|
done := make(chan struct{})
|
||||||
|
|||||||
@@ -29,19 +29,8 @@ type XrayTrafficJob struct {
|
|||||||
// refetch for the rest.
|
// refetch for the rest.
|
||||||
const clientStatsSnapshotMaxClients = 5000
|
const clientStatsSnapshotMaxClients = 5000
|
||||||
|
|
||||||
// externalInformTimeout bounds the traffic-notify POST. Run() is scheduled
|
|
||||||
// every 5s under cron.SkipIfStillRunning, so an unbounded call to a receiver
|
|
||||||
// that accepts the connection and then stalls does not merely delay one
|
|
||||||
// notification: it holds the job, and every following tick is skipped for as
|
|
||||||
// long as the stall lasts. AddTraffic — quota enforcement, auto-renew — and
|
|
||||||
// the online/websocket work all sit in that same tick (#6115).
|
|
||||||
const externalInformTimeout = 3 * time.Second
|
const externalInformTimeout = 3 * time.Second
|
||||||
|
|
||||||
// externalInformClient is kept separate from fasthttp's shared default client
|
|
||||||
// so this endpoint's timeouts and connection handling cannot be influenced by,
|
|
||||||
// or influence, any other caller. Idempotent-call retries stay off on purpose:
|
|
||||||
// the payload carries per-tick deltas, so an attempt that reached the receiver
|
|
||||||
// but failed on the response leg would be counted twice if it were resent.
|
|
||||||
var externalInformClient = &fasthttp.Client{
|
var externalInformClient = &fasthttp.Client{
|
||||||
ReadTimeout: externalInformTimeout,
|
ReadTimeout: externalInformTimeout,
|
||||||
WriteTimeout: externalInformTimeout,
|
WriteTimeout: externalInformTimeout,
|
||||||
|
|||||||
@@ -91,9 +91,6 @@ func TestDepletedCond_ProbeGuard(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// A master that stopped pushing leaves its last snapshot behind forever. Those
|
|
||||||
// frozen counters must not keep enforcing quota, or a client well inside its
|
|
||||||
// limit is disabled on every traffic poll with no way back (#6113).
|
|
||||||
func TestStaleGlobalTraffic_Ignored(t *testing.T) {
|
func TestStaleGlobalTraffic_Ignored(t *testing.T) {
|
||||||
db := initTrafficTestDB(t)
|
db := initTrafficTestDB(t)
|
||||||
svc := &InboundService{}
|
svc := &InboundService{}
|
||||||
@@ -112,7 +109,6 @@ func TestStaleGlobalTraffic_Ignored(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
t.Run("stale row alone neither enforces nor selects the cross-panel predicate", func(t *testing.T) {
|
t.Run("stale row alone neither enforces nor selects the cross-panel predicate", func(t *testing.T) {
|
||||||
// 200 of 1000 used locally, 1900 reported by a master gone for a day.
|
|
||||||
seedClientRow(t, "cap", 1, 100, 100, 1000)
|
seedClientRow(t, "cap", 1, 100, 100, 1000)
|
||||||
seedStaleGlobal(t, "dead-master", "cap", 1000, 900)
|
seedStaleGlobal(t, "dead-master", "cap", 1000, 900)
|
||||||
|
|
||||||
@@ -138,8 +134,6 @@ func TestStaleGlobalTraffic_Ignored(t *testing.T) {
|
|||||||
})
|
})
|
||||||
|
|
||||||
t.Run("a live master still enforces alongside the stale row", func(t *testing.T) {
|
t.Run("a live master still enforces alongside the stale row", func(t *testing.T) {
|
||||||
// This is the #6113 shape: one dead master frozen over quota, one live
|
|
||||||
// master well under it. Only the live one may decide.
|
|
||||||
if err := svc.AcceptGlobalTraffic("live-master", []*xray.ClientTraffic{{Email: "cap", Up: 1, Down: 1}}); err != nil {
|
if err := svc.AcceptGlobalTraffic("live-master", []*xray.ClientTraffic{{Email: "cap", Up: 1, Down: 1}}); err != nil {
|
||||||
t.Fatalf("AcceptGlobalTraffic: %v", err)
|
t.Fatalf("AcceptGlobalTraffic: %v", err)
|
||||||
}
|
}
|
||||||
@@ -152,7 +146,6 @@ func TestStaleGlobalTraffic_Ignored(t *testing.T) {
|
|||||||
t.Fatalf("the live master reports usage well under quota, disabled %d", count)
|
t.Fatalf("the live master reports usage well under quota, disabled %d", count)
|
||||||
}
|
}
|
||||||
|
|
||||||
// And once the live master reports real depletion, it takes effect.
|
|
||||||
if err := svc.AcceptGlobalTraffic("live-master", []*xray.ClientTraffic{{Email: "cap", Up: 600, Down: 500}}); err != nil {
|
if err := svc.AcceptGlobalTraffic("live-master", []*xray.ClientTraffic{{Email: "cap", Up: 600, Down: 500}}); err != nil {
|
||||||
t.Fatalf("AcceptGlobalTraffic: %v", err)
|
t.Fatalf("AcceptGlobalTraffic: %v", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -49,15 +49,6 @@ func (s *InboundService) disableInvalidInbounds(tx *gorm.DB) (bool, int64, error
|
|||||||
return needRestart, count, err
|
return needRestart, count, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// globalTrafficFreshWindow bounds how long a pushed client_global_traffics row
|
|
||||||
// stays authoritative. Masters refresh their rows every nodeGlobalPushInterval
|
|
||||||
// (30s), so a row older than this belongs to a master that stopped pushing —
|
|
||||||
// decommissioned, reinstalled under a new GUID, or detached from this node.
|
|
||||||
// Such a row keeps its last-seen counters forever, and without this bound a
|
|
||||||
// long-dead master's numbers permanently trip the cross-panel quota check and
|
|
||||||
// disable clients that are nowhere near their limit (#6113). The window is far
|
|
||||||
// wider than any real push gap, so a master that is merely unreachable for a
|
|
||||||
// while keeps enforcing.
|
|
||||||
const globalTrafficFreshWindow = 24 * time.Hour
|
const globalTrafficFreshWindow = 24 * time.Hour
|
||||||
|
|
||||||
func globalTrafficFreshSince() int64 {
|
func globalTrafficFreshSince() int64 {
|
||||||
|
|||||||
@@ -28,9 +28,6 @@ func seedVlessInbound(t *testing.T, tag string, port int, clients []model.Client
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ClientRecord.Enable carries gorm:"default:true", so a false on insert is
|
|
||||||
// replaced by the default. Production disables through an UPDATE
|
|
||||||
// (disableInvalidClients); do the same here.
|
|
||||||
func disableClients(t *testing.T, emails ...string) {
|
func disableClients(t *testing.T, emails ...string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
if err := database.GetDB().Model(&model.ClientRecord{}).
|
if err := database.GetDB().Model(&model.ClientRecord{}).
|
||||||
@@ -62,9 +59,6 @@ func emittedClients(t *testing.T, tag string) (any, bool) {
|
|||||||
return nil, false
|
return nil, false
|
||||||
}
|
}
|
||||||
|
|
||||||
// An inbound whose clients are all filtered out must hand xray-core an empty
|
|
||||||
// array. It used to emit "clients": null, which the panel treats as invalid
|
|
||||||
// data elsewhere and coerces to [] at startup (#6117).
|
|
||||||
func TestGetXrayConfig_EmptyClientListIsArrayNotNull(t *testing.T) {
|
func TestGetXrayConfig_EmptyClientListIsArrayNotNull(t *testing.T) {
|
||||||
seedVlessInbound(t, "vless-empty", 43101, []model.Client{
|
seedVlessInbound(t, "vless-empty", 43101, []model.Client{
|
||||||
{Email: "gone@x", ID: "11111111-1111-1111-1111-111111111111", Enable: true},
|
{Email: "gone@x", ID: "11111111-1111-1111-1111-111111111111", Enable: true},
|
||||||
@@ -87,8 +81,6 @@ func TestGetXrayConfig_EmptyClientListIsArrayNotNull(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// The disabled/depleted filter must keep working — an empty array is only
|
|
||||||
// correct because those clients are genuinely excluded.
|
|
||||||
func TestGetXrayConfig_EnabledClientsStillEmitted(t *testing.T) {
|
func TestGetXrayConfig_EnabledClientsStillEmitted(t *testing.T) {
|
||||||
seedVlessInbound(t, "vless-mixed", 43102, []model.Client{
|
seedVlessInbound(t, "vless-mixed", 43102, []model.Client{
|
||||||
{Email: "live@x", ID: "22222222-2222-2222-2222-222222222222", Enable: true},
|
{Email: "live@x", ID: "22222222-2222-2222-2222-222222222222", Enable: true},
|
||||||
|
|||||||
Reference in New Issue
Block a user