From 16eee1b89c835dd7be44f12c24ce7f303bda8a9f Mon Sep 17 00:00:00 2001 From: RPRX <63339210+RPRX@users.noreply.github.com> Date: Tue, 4 Mar 2025 11:01:03 +0000 Subject: [PATCH] Mux server: Clone the session.Content attached on ctx as well https://github.com/XTLS/Xray-core/pull/3908#issuecomment-2696911994 Fixes https://github.com/XTLS/Xray-core/pull/3904 for good --- app/proxyman/inbound/worker.go | 1 + common/mux/server.go | 2 +- common/protocol/http/sniff.go | 2 +- common/session/context.go | 12 +++++++++-- common/session/session.go | 39 ++-------------------------------- 5 files changed, 15 insertions(+), 41 deletions(-) diff --git a/app/proxyman/inbound/worker.go b/app/proxyman/inbound/worker.go index a14a338f..12e29876 100644 --- a/app/proxyman/inbound/worker.go +++ b/app/proxyman/inbound/worker.go @@ -324,6 +324,7 @@ func (w *udpWorker) callback(b *buf.Buffer, source net.Destination, originalDest if w.sniffingConfig != nil { content.SniffingRequest.Enabled = w.sniffingConfig.Enabled content.SniffingRequest.OverrideDestinationForProtocol = w.sniffingConfig.DestinationOverride + content.SniffingRequest.ExcludeForDomain = w.sniffingConfig.DomainsExcluded content.SniffingRequest.MetadataOnly = w.sniffingConfig.MetadataOnly content.SniffingRequest.RouteOnly = w.sniffingConfig.RouteOnly } diff --git a/common/mux/server.go b/common/mux/server.go index 480175ba..8fc0ae09 100644 --- a/common/mux/server.go +++ b/common/mux/server.go @@ -120,7 +120,7 @@ func (w *ServerWorker) handleStatusKeepAlive(meta *FrameMetadata, reader *buf.Bu func (w *ServerWorker) handleStatusNew(ctx context.Context, meta *FrameMetadata, reader *buf.BufferedReader) error { // deep-clone outbounds because it is going to be mutated concurrently // (Target and OriginalTarget) - ctx = session.ContextCloneOutbounds(ctx) + ctx = session.ContextCloneOutboundsAndContent(ctx) errors.LogInfo(ctx, "received request for ", meta.Target) { msg := &log.AccessMessage{ diff --git a/common/protocol/http/sniff.go b/common/protocol/http/sniff.go index 40968294..e85a0792 100644 --- a/common/protocol/http/sniff.go +++ b/common/protocol/http/sniff.go @@ -63,7 +63,7 @@ func SniffHTTP(b []byte, c context.Context) (*SniffHeader, error) { ShouldSniffAttr := true // If content.Attributes have information, that means it comes from HTTP inbound PlainHTTP mode. // It will set attributes, so skip it. - if content == nil || content.AttributeLen() != 0 { + if content == nil || len(content.Attributes) != 0 { ShouldSniffAttr = false } if err := beginWithHTTPMethod(b); err != nil { diff --git a/common/session/context.go b/common/session/context.go index f2c8de7f..0e2558c2 100644 --- a/common/session/context.go +++ b/common/session/context.go @@ -42,7 +42,7 @@ func ContextWithOutbounds(ctx context.Context, outbounds []*Outbound) context.Co return context.WithValue(ctx, outboundSessionKey, outbounds) } -func ContextCloneOutbounds(ctx context.Context) context.Context { +func ContextCloneOutboundsAndContent(ctx context.Context) context.Context { outbounds := OutboundsFromContext(ctx) newOutbounds := make([]*Outbound, len(outbounds)) for i, ob := range outbounds { @@ -55,7 +55,15 @@ func ContextCloneOutbounds(ctx context.Context) context.Context { newOutbounds[i] = &v } - return ContextWithOutbounds(ctx, newOutbounds) + content := ContentFromContext(ctx) + newContent := Content{} + if content != nil { + newContent = *content + if content.Attributes != nil { + panic("content.Attributes != nil") + } + } + return ContextWithContent(ContextWithOutbounds(ctx, newOutbounds), &newContent) } func OutboundsFromContext(ctx context.Context) []*Outbound { diff --git a/common/session/session.go b/common/session/session.go index cad15400..4de97374 100644 --- a/common/session/session.go +++ b/common/session/session.go @@ -4,7 +4,6 @@ package session // import "github.com/xtls/xray-core/common/session" import ( "context" "math/rand" - "sync" c "github.com/xtls/xray-core/common/ctx" "github.com/xtls/xray-core/common/errors" @@ -75,8 +74,8 @@ type Outbound struct { // SniffingRequest controls the behavior of content sniffing. type SniffingRequest struct { - ExcludeForDomain []string - OverrideDestinationForProtocol []string + ExcludeForDomain []string // read-only once set + OverrideDestinationForProtocol []string // read-only once set Enabled bool MetadataOnly bool RouteOnly bool @@ -92,10 +91,6 @@ type Content struct { Attributes map[string]string SkipDNSResolve bool - - mu sync.Mutex - - isLocked bool } // Sockopt is the settings for socket connection. @@ -104,22 +99,8 @@ type Sockopt struct { Mark int32 } -// Some how when using mux, there will be a same ctx between different requests -// This will cause problem as it's designed for single request, like concurrent map writes -// Add a Mutex as a temp solution - // SetAttribute attaches additional string attributes to content. func (c *Content) SetAttribute(name string, value string) { - if c.isLocked { - errors.LogError(context.Background(), "Multiple goroutines are tring to access one routing content, tring to write ", name, ":", value) - } - c.mu.Lock() - c.isLocked = true - defer func() { - c.isLocked = false - c.mu.Unlock() - }() - if c.Attributes == nil { c.Attributes = make(map[string]string) } @@ -128,24 +109,8 @@ func (c *Content) SetAttribute(name string, value string) { // Attribute retrieves additional string attributes from content. func (c *Content) Attribute(name string) string { - c.mu.Lock() - c.isLocked = true - defer func() { - c.isLocked = false - c.mu.Unlock() - }() if c.Attributes == nil { return "" } return c.Attributes[name] } - -func (c *Content) AttributeLen() int { - c.mu.Lock() - c.isLocked = true - defer func() { - c.isLocked = false - c.mu.Unlock() - }() - return len(c.Attributes) -}