fix(nodes): explicit reference check, trim lookup, deep-copy global definitions

IsReference no longer uses reflect.DeepEqual; ResolveConfig trims the name before registry lookup; cloneConfig deep-copies all nested fields so hop parsing never mutates shared globals. Also documents the intentional fail-fast vs warn-and-skip error semantics.
This commit is contained in:
ginuerzh
2026-10-07 01:36:45 +00:00
parent 8f6a725e7e
commit 17909c4f95
4 changed files with 237 additions and 36 deletions
+4
View File
@@ -91,6 +91,10 @@ func ParseHop(cfg *config.HopConfig, log logger.Logger) (hop.Hop, error) {
var err error
v, err = node_parser.ResolveConfig(v)
if err != nil {
// Fail the whole hop: static config is validated at startup,
// so a dangling reference is a hard error. (Contrast with
// chainHop.parseNode, which warns and skips bad nodes to
// keep hot-reloaded hops resilient.)
return nil, err
}
+106 -11
View File
@@ -3,7 +3,6 @@ package node
import (
"fmt"
"net"
"reflect"
"regexp"
"strconv"
"strings"
@@ -40,11 +39,20 @@ const MaxMatcherBodySize = 10 << 20 // 10MB
// IsReference reports whether cfg contains only a non-empty name. Such a
// config references a definition from the top-level nodes section.
// Any additional field, even a zero-valued one set explicitly (e.g. an
// empty but non-nil connector), makes the config inline.
// NOTE: keep this in sync with NodeConfig — a new field must be added here,
// otherwise a config setting only that field would misresolve as a reference.
func IsReference(cfg *config.NodeConfig) bool {
if cfg == nil || strings.TrimSpace(cfg.Name) == "" {
return false
}
return reflect.DeepEqual(cfg, &config.NodeConfig{Name: cfg.Name})
return cfg.Addr == "" && cfg.Network == "" && cfg.Bypass == "" &&
len(cfg.Bypasses) == 0 && cfg.Resolver == "" && cfg.Hosts == "" &&
cfg.Connector == nil && cfg.Dialer == nil &&
cfg.Interface == "" && cfg.Netns == "" && cfg.SockOpts == nil &&
cfg.Filter == nil && cfg.Matcher == nil && cfg.HTTP == nil &&
cfg.TLS == nil && cfg.Probe == nil && len(cfg.Metadata) == 0
}
// ResolveConfig resolves a name-only config through the global node registry
@@ -58,7 +66,7 @@ func ResolveConfig(cfg *config.NodeConfig) (*config.NodeConfig, error) {
return cfg, nil
}
name := cfg.Name
name := strings.TrimSpace(cfg.Name)
cfg = registry.NodeRegistry().Get(name)
if cfg == nil {
return nil, fmt.Errorf("node %q not found", name)
@@ -66,8 +74,11 @@ func ResolveConfig(cfg *config.NodeConfig) (*config.NodeConfig, error) {
return cloneConfig(cfg), nil
}
// cloneConfig copies the fields that parsing may default or inherit. Other
// nested values are read-only during parsing and can safely remain shared.
// cloneConfig returns a deep copy of cfg so that parsing (hop-level
// inheritance, connector/dialer type defaults, TLS ServerName defaulting)
// only ever mutates the copy, never the shared global definition.
// NOTE: keep this in sync with NodeConfig — a new reference-type field
// must be cloned here, otherwise hops would share it with the registry.
func cloneConfig(cfg *config.NodeConfig) *config.NodeConfig {
c := *cfg
c.Bypasses = append([]string(nil), cfg.Bypasses...)
@@ -79,23 +90,107 @@ func cloneConfig(cfg *config.NodeConfig) *config.NodeConfig {
}
if cfg.Connector != nil {
connector := *cfg.Connector
if cfg.Connector.TLS != nil {
tls := *cfg.Connector.TLS
connector.TLS = &tls
connector.Auth = cloneAuth(cfg.Connector.Auth)
connector.TLS = cloneTLSConfig(cfg.Connector.TLS)
if cfg.Connector.Metadata != nil {
connector.Metadata = make(map[string]any, len(cfg.Connector.Metadata))
for k, v := range cfg.Connector.Metadata {
connector.Metadata[k] = v
}
}
c.Connector = &connector
}
if cfg.Dialer != nil {
dialer := *cfg.Dialer
if cfg.Dialer.TLS != nil {
tls := *cfg.Dialer.TLS
dialer.TLS = &tls
dialer.Auth = cloneAuth(cfg.Dialer.Auth)
dialer.TLS = cloneTLSConfig(cfg.Dialer.TLS)
if cfg.Dialer.Metadata != nil {
dialer.Metadata = make(map[string]any, len(cfg.Dialer.Metadata))
for k, v := range cfg.Dialer.Metadata {
dialer.Metadata[k] = v
}
}
c.Dialer = &dialer
}
if cfg.SockOpts != nil {
sockOpts := *cfg.SockOpts
c.SockOpts = &sockOpts
}
if cfg.Filter != nil {
filter := *cfg.Filter
c.Filter = &filter
}
if cfg.Matcher != nil {
matcher := *cfg.Matcher
c.Matcher = &matcher
}
if cfg.HTTP != nil {
http := *cfg.HTTP
http.Header = cloneStringMap(cfg.HTTP.Header)
http.RequestHeader = cloneStringMap(cfg.HTTP.RequestHeader)
http.ResponseHeader = cloneStringMap(cfg.HTTP.ResponseHeader)
http.Rewrite = append([]config.HTTPURLRewriteConfig(nil), cfg.HTTP.Rewrite...)
http.RewriteURL = append([]config.HTTPURLRewriteConfig(nil), cfg.HTTP.RewriteURL...)
http.RewriteBody = append([]config.HTTPBodyRewriteConfig(nil), cfg.HTTP.RewriteBody...)
http.RewriteRequestBody = append([]config.HTTPBodyRewriteConfig(nil), cfg.HTTP.RewriteRequestBody...)
http.RewriteResponseBody = append([]config.HTTPBodyRewriteConfig(nil), cfg.HTTP.RewriteResponseBody...)
http.RewriteRequestHeader = append([]config.HTTPHeaderRewriteConfig(nil), cfg.HTTP.RewriteRequestHeader...)
http.RewriteResponseHeader = append([]config.HTTPHeaderRewriteConfig(nil), cfg.HTTP.RewriteResponseHeader...)
http.Auth = cloneAuth(cfg.HTTP.Auth)
c.HTTP = &http
}
if cfg.TLS != nil {
tls := *cfg.TLS
if cfg.TLS.Options != nil {
options := *cfg.TLS.Options
options.CipherSuites = append([]string(nil), cfg.TLS.Options.CipherSuites...)
options.ALPN = append([]string(nil), cfg.TLS.Options.ALPN...)
tls.Options = &options
}
c.TLS = &tls
}
if cfg.Probe != nil {
probe := *cfg.Probe
probe.HTTPHeaders = cloneStringMap(cfg.Probe.HTTPHeaders)
c.Probe = &probe
}
return &c
}
func cloneAuth(cfg *config.AuthConfig) *config.AuthConfig {
if cfg == nil {
return nil
}
auth := *cfg
return &auth
}
func cloneTLSConfig(cfg *config.TLSConfig) *config.TLSConfig {
if cfg == nil {
return nil
}
tls := *cfg
tls.ServerNames = append([]string(nil), cfg.ServerNames...)
if cfg.Options != nil {
options := *cfg.Options
options.CipherSuites = append([]string(nil), cfg.Options.CipherSuites...)
options.ALPN = append([]string(nil), cfg.Options.ALPN...)
tls.Options = &options
}
return &tls
}
func cloneStringMap(m map[string]string) map[string]string {
if m == nil {
return nil
}
c := make(map[string]string, len(m))
for k, v := range m {
c[k] = v
}
return c
}
// filterToMatcherRule converts a deprecated NodeFilterConfig (host/protocol/
// path) into an equivalent matcher DSL rule. Empty fields are omitted; an
// empty result means the node is unconditional. Path maps to PathPrefix
+99
View File
@@ -8,6 +8,7 @@ import (
"github.com/go-gost/x/config"
"github.com/go-gost/x/config/parsing"
xlogger "github.com/go-gost/x/logger"
"github.com/go-gost/x/registry"
// Register connector and dialer implementations needed for node parsing.
_ "github.com/go-gost/x/connector/http"
@@ -574,3 +575,101 @@ func TestFilterToMatcherRule(t *testing.T) {
})
}
}
func TestIsReference(t *testing.T) {
tests := []struct {
name string
cfg *config.NodeConfig
want bool
}{
{"nil", nil, false},
{"empty name", &config.NodeConfig{}, false},
{"blank name", &config.NodeConfig{Name: " "}, false},
{"name only", &config.NodeConfig{Name: "shared"}, true},
{"name and addr", &config.NodeConfig{Name: "shared", Addr: "example.com:80"}, false},
{"name and network", &config.NodeConfig{Name: "shared", Network: "tcp"}, false},
{"name and empty-but-non-nil connector", &config.NodeConfig{Name: "shared", Connector: &config.ConnectorConfig{}}, false},
{"name and empty bypasses slice", &config.NodeConfig{Name: "shared", Bypasses: []string{}}, true},
{"name and matcher", &config.NodeConfig{Name: "shared", Matcher: &config.NodeMatcherConfig{Rule: "Host(`x`)"}}, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := IsReference(tt.cfg); got != tt.want {
t.Errorf("IsReference(%+v) = %v, want %v", tt.cfg, got, tt.want)
}
})
}
}
func TestResolveConfig_TrimsName(t *testing.T) {
const name = "spaced-global-node"
registry.NodeRegistry().Register(name, &config.NodeConfig{Name: name, Addr: "example.com:8080"})
t.Cleanup(func() { registry.NodeRegistry().Unregister(name) })
got, err := ResolveConfig(&config.NodeConfig{Name: " " + name + " "})
if err != nil {
t.Fatalf("ResolveConfig: %v", err)
}
if got == nil || got.Addr != "example.com:8080" {
t.Fatalf("resolved = %+v, want addr %q", got, "example.com:8080")
}
}
func TestResolveConfig_DeepCopyIsolation(t *testing.T) {
const name = "rich-global-node"
global := &config.NodeConfig{
Name: name,
Addr: "example.com:8080",
Metadata: map[string]any{"k": "v"},
Connector: &config.ConnectorConfig{
Type: "http",
Auth: &config.AuthConfig{Username: "u"},
Metadata: map[string]any{"ck": "cv"},
TLS: &config.TLSConfig{ServerName: "sni.example", Options: &config.TLSOptions{ALPN: []string{"h2"}}},
},
Dialer: &config.DialerConfig{
Type: "tcp",
Metadata: map[string]any{"dk": "dv"},
},
Matcher: &config.NodeMatcherConfig{Rule: "Host(`x`)"},
HTTP: &config.HTTPNodeConfig{
Host: "h",
RequestHeader: map[string]string{"A": "b"},
Auth: &config.AuthConfig{Username: "hu"},
},
TLS: &config.TLSNodeConfig{ServerName: "tls.example", Options: &config.TLSOptions{MinVersion: "1.2"}},
Probe: &config.ProbeConfig{Type: "tcp", Addr: "example.com:80", HTTPHeaders: map[string]string{"H": "v"}},
}
registry.NodeRegistry().Register(name, global)
t.Cleanup(func() { registry.NodeRegistry().Unregister(name) })
got, err := ResolveConfig(&config.NodeConfig{Name: name})
if err != nil {
t.Fatalf("ResolveConfig: %v", err)
}
// Mutate every nested value on the copy; the global must not change.
got.Metadata["k"] = "mut"
got.Connector.Auth.Username = "mut"
got.Connector.Metadata["ck"] = "mut"
got.Connector.TLS.Options.ALPN[0] = "mut"
got.Dialer.Metadata["dk"] = "mut"
got.Matcher.Rule = "mut"
got.HTTP.RequestHeader["A"] = "mut"
got.HTTP.Auth.Username = "mut"
got.TLS.Options.MinVersion = "mut"
got.Probe.HTTPHeaders["H"] = "mut"
if global.Metadata["k"] != "v" ||
global.Connector.Auth.Username != "u" ||
global.Connector.Metadata["ck"] != "cv" ||
global.Connector.TLS.Options.ALPN[0] != "h2" ||
global.Dialer.Metadata["dk"] != "dv" ||
global.Matcher.Rule != "Host(`x`)" ||
global.HTTP.RequestHeader["A"] != "b" ||
global.HTTP.Auth.Username != "hu" ||
global.TLS.Options.MinVersion != "1.2" ||
global.Probe.HTTPHeaders["H"] != "v" {
t.Fatalf("global definition was mutated via the resolved copy: %+v", global)
}
}
+5 -2
View File
@@ -20,11 +20,11 @@ import (
"github.com/go-gost/core/routing"
"github.com/go-gost/core/selector"
"github.com/go-gost/x/config"
xs "github.com/go-gost/x/selector"
mdutil "github.com/go-gost/x/metadata/util"
node_parser "github.com/go-gost/x/config/parsing/node"
"github.com/go-gost/x/internal/loader"
xlogger "github.com/go-gost/x/logger"
mdutil "github.com/go-gost/x/metadata/util"
xs "github.com/go-gost/x/selector"
)
type options struct {
@@ -487,6 +487,9 @@ func (p *chainHop) parseNode(r io.Reader) ([]*chain.Node, error) {
name := nc.Name
nc, err := node_parser.ResolveConfig(nc)
if err != nil {
// Warn and skip: file/Redis/HTTP reloads must stay resilient
// to a single bad entry. (Contrast with ParseHop, which
// fails the whole hop on a dangling reference at startup.)
p.logger.Warnf("skip node %s: %v", name, err)
errs = append(errs, err)
continue