fix: ensure proxy stable IDs are unique for distinct configs#137
fix: ensure proxy stable IDs are unique for distinct configs#137yagee wants to merge 1 commit intokutovoys:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes proxy identity generation so distinct proxy configurations no longer collide on stableId, preventing duplicate keys/missing proxy cards in the dashboard.
Changes:
- Expands
ProxyConfig.GenerateStableID()to include additional transport/security fields. - Ensures runtime
StableIDuniqueness by suffixing duplicate base IDs inPrepareProxyConfigs. - Updates config equality checks to compare multiplicities (counts) instead of treating IDs as a set.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
xray/utils.go |
Reworks stable ID assignment to ensure uniqueness and updates config equality logic to be count-based. |
models/proxy_config.go |
Extends the stable ID hash inputs to cover more fields that define proxy identity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| idComponents = append(idComponents, pc.Server) | ||
| idComponents = append(idComponents, fmt.Sprintf("%d", pc.Port)) | ||
| idComponents = append(idComponents, pc.Name) |
There was a problem hiding this comment.
GenerateStableID() now incorporates pc.Name. Since Name is a user-facing label that can change without the underlying proxy transport/credentials changing (e.g., subscription providers often rename entries), this will change stable IDs and can trigger unnecessary config reloads / break any consumers that expect the ID to be stable across renames. Consider removing Name from the identity hash (or introducing a separate, explicitly "display" field) so identity is derived only from connection-defining fields.
| idComponents = append(idComponents, pc.Name) |
| if len(pc.ALPN) > 0 { | ||
| idComponents = append(idComponents, strings.Join(pc.ALPN, ",")) | ||
| } | ||
|
|
||
| idString := strings.Join(idComponents, "|") | ||
|
|
There was a problem hiding this comment.
idString is built by joining components with the literal "|", but several newly-added components (e.g., Name, Path, Host, ServiceName, potentially ALPN entries) can legally contain |. That makes the concatenation ambiguous and can reintroduce collisions where different component lists produce the same idString. Use an unambiguous encoding (e.g., length-prefix each component, or JSON encode a struct/array) before hashing.
| for _, cfg := range old { | ||
| if cfg.StableID == "" { | ||
| cfg.StableID = cfg.GenerateStableID() | ||
| } | ||
| oldMap[cfg.StableID] = true | ||
| oldCounts[cfg.GenerateStableID()]++ | ||
| } | ||
|
|
||
| for _, cfg := range new { | ||
| if cfg.StableID == "" { | ||
| cfg.StableID = cfg.GenerateStableID() | ||
| } | ||
| newMap[cfg.StableID] = true | ||
| newCounts[cfg.GenerateStableID()]++ |
There was a problem hiding this comment.
IsConfigsEqual now compares configs via GenerateStableID() counts. If GenerateStableID() includes mutable/non-identity fields (notably Name per this PR), a pure rename will be treated as a config change and will trigger a full reconfigure/restart. Consider comparing on a dedicated identity function that excludes display-only fields, or ensure GenerateStableID() is strictly connection-defining.
| idComponents = append(idComponents, pc.PublicKey) | ||
| } | ||
|
|
||
| if pc.Fingerprint != "" { | ||
| idComponents = append(idComponents, pc.Fingerprint) |
There was a problem hiding this comment.
GenerateStableID() is now used for config change detection (IsConfigsEqual) and for disambiguating entries, but it still omits some connection-impacting fields that are parsed and used in Xray config generation (e.g., AllowInsecure is set from subscription links and used in xray/config.go TLS stream settings). If AllowInsecure changes, IsConfigsEqual may incorrectly treat configs as unchanged and skip reconfiguration. Consider including AllowInsecure (and any other stream-settings fields that affect the generated Xray config) in the stable ID input.
|
Hi there! I've been investigating the same Cannot read properties of undefined (reading 'after') crash in Alpine.js caused by StableID collisions. While this PR improves things by adding more transport fields to the hash, it still fails when a subscription file (like the ones commonly found on GitHub) contains multiple entries for the same server with different names/fragments. Since Name isn't part of the identity in the current logic, they still receive duplicate IDs, which leads to the UI crash. I’ve found that the most robust solution is to: Save the original RawLink (the full URL string) inside the ProxyConfig model during parsing. |
Description of Changes
Fix proxy identity generation so distinct proxy configurations no longer receive the same
stableId.The previous
stableIdgeneration used only a subset of proxy fields, which caused collisions for configs that shared the sameprotocol/server/port/UUID/public key but differed in meaningful transport fields such as Reality fingerprint. This broke the
dashboard because Alpine uses
stableIdas thex-forkey, resulting in duplicate key warnings and missing server cards.This PR:
GenerateStableID()to include additional identity-defining fieldsstableIdvalues remain unique even if two entries are otherwise identicalType of Changes
Related Issues
Fixes #
Checklist