From f4f795a2a69ae3cc701e3ac3d65abde2b37b91fb Mon Sep 17 00:00:00 2001 From: Alessandro Ros Date: Sat, 24 Jan 2026 19:33:47 +0100 Subject: [PATCH] simplify configuration parsing (#5372) work around golang/go#21092 globally --- internal/conf/allowed_origins.go | 4 - internal/conf/auth_internal_users.go | 20 +--- internal/conf/conf.go | 101 ++++++++++---------- internal/conf/conf_test.go | 28 ------ internal/conf/jsonwrapper/unmarshal.go | 73 +++++++++++++- internal/conf/jsonwrapper/unmarshal_test.go | 74 ++++++++++++++ internal/conf/path.go | 1 - internal/conf/webrtc_ice_server.go | 12 +-- internal/conf/yamlwrapper/unmarshal.go | 4 + 9 files changed, 201 insertions(+), 116 deletions(-) delete mode 100644 internal/conf/allowed_origins.go create mode 100644 internal/conf/jsonwrapper/unmarshal_test.go diff --git a/internal/conf/allowed_origins.go b/internal/conf/allowed_origins.go deleted file mode 100644 index 77db4fba..00000000 --- a/internal/conf/allowed_origins.go +++ /dev/null @@ -1,4 +0,0 @@ -package conf - -// AllowedOrigins is a list of allowed CORS origins. -type AllowedOrigins []string diff --git a/internal/conf/auth_internal_users.go b/internal/conf/auth_internal_users.go index 720a132c..038cab5b 100644 --- a/internal/conf/auth_internal_users.go +++ b/internal/conf/auth_internal_users.go @@ -39,24 +39,8 @@ func (d *AuthInternalUser) UnmarshalJSON(b []byte) error { return nil } -// AuthInternalUsers is a list of AuthInternalUser +// AuthInternalUsers is a list of AuthInternalUser. type AuthInternalUsers []AuthInternalUser -// UnmarshalJSON implements json.Unmarshaler. -func (s *AuthInternalUsers) UnmarshalJSON(b []byte) error { - // remove default value before loading new value - // https://github.com/golang/go/issues/21092 - *s = nil - return jsonwrapper.Unmarshal(b, (*[]AuthInternalUser)(s)) -} - -// AuthInternalUserPermissions is a list of AuthInternalUserPermission +// AuthInternalUserPermissions is a list of AuthInternalUserPermission. type AuthInternalUserPermissions []AuthInternalUserPermission - -// UnmarshalJSON implements json.Unmarshaler. -func (s *AuthInternalUserPermissions) UnmarshalJSON(b []byte) error { - // remove default value before loading new value - // https://github.com/golang/go/issues/21092 - *s = nil - return jsonwrapper.Unmarshal(b, (*[]AuthInternalUserPermission)(s)) -} diff --git a/internal/conf/conf.go b/internal/conf/conf.go index 5ac60012..d015cc0a 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -147,7 +147,6 @@ var defaultAuthInternalUsers = AuthInternalUsers{ } // Conf is a configuration. -// WARNING: Avoid using slices directly due to https://github.com/golang/go/issues/21092 type Conf struct { // General LogLevel LogLevel `json:"logLevel"` @@ -178,44 +177,44 @@ type Conf struct { AuthJWTInHTTPQuery bool `json:"authJWTInHTTPQuery"` // Control API - API bool `json:"api"` - APIAddress string `json:"apiAddress"` - APIEncryption bool `json:"apiEncryption"` - APIServerKey string `json:"apiServerKey"` - APIServerCert string `json:"apiServerCert"` - APIAllowOrigin *string `json:"apiAllowOrigin,omitempty"` // deprecated - APIAllowOrigins AllowedOrigins `json:"apiAllowOrigins"` - APITrustedProxies IPNetworks `json:"apiTrustedProxies"` + API bool `json:"api"` + APIAddress string `json:"apiAddress"` + APIEncryption bool `json:"apiEncryption"` + APIServerKey string `json:"apiServerKey"` + APIServerCert string `json:"apiServerCert"` + APIAllowOrigin *string `json:"apiAllowOrigin,omitempty"` // deprecated + APIAllowOrigins []string `json:"apiAllowOrigins"` + APITrustedProxies IPNetworks `json:"apiTrustedProxies"` // Metrics - Metrics bool `json:"metrics"` - MetricsAddress string `json:"metricsAddress"` - MetricsEncryption bool `json:"metricsEncryption"` - MetricsServerKey string `json:"metricsServerKey"` - MetricsServerCert string `json:"metricsServerCert"` - MetricsAllowOrigin *string `json:"metricsAllowOrigin,omitempty"` // deprecated - MetricsAllowOrigins AllowedOrigins `json:"metricsAllowOrigins"` - MetricsTrustedProxies IPNetworks `json:"metricsTrustedProxies"` + Metrics bool `json:"metrics"` + MetricsAddress string `json:"metricsAddress"` + MetricsEncryption bool `json:"metricsEncryption"` + MetricsServerKey string `json:"metricsServerKey"` + MetricsServerCert string `json:"metricsServerCert"` + MetricsAllowOrigin *string `json:"metricsAllowOrigin,omitempty"` // deprecated + MetricsAllowOrigins []string `json:"metricsAllowOrigins"` + MetricsTrustedProxies IPNetworks `json:"metricsTrustedProxies"` // PPROF - PPROF bool `json:"pprof"` - PPROFAddress string `json:"pprofAddress"` - PPROFEncryption bool `json:"pprofEncryption"` - PPROFServerKey string `json:"pprofServerKey"` - PPROFServerCert string `json:"pprofServerCert"` - PPROFAllowOrigin *string `json:"pprofAllowOrigin,omitempty"` // deprecated - PPROFAllowOrigins AllowedOrigins `json:"pprofAllowOrigins"` - PPROFTrustedProxies IPNetworks `json:"pprofTrustedProxies"` + PPROF bool `json:"pprof"` + PPROFAddress string `json:"pprofAddress"` + PPROFEncryption bool `json:"pprofEncryption"` + PPROFServerKey string `json:"pprofServerKey"` + PPROFServerCert string `json:"pprofServerCert"` + PPROFAllowOrigin *string `json:"pprofAllowOrigin,omitempty"` // deprecated + PPROFAllowOrigins []string `json:"pprofAllowOrigins"` + PPROFTrustedProxies IPNetworks `json:"pprofTrustedProxies"` // Playback - Playback bool `json:"playback"` - PlaybackAddress string `json:"playbackAddress"` - PlaybackEncryption bool `json:"playbackEncryption"` - PlaybackServerKey string `json:"playbackServerKey"` - PlaybackServerCert string `json:"playbackServerCert"` - PlaybackAllowOrigin *string `json:"playbackAllowOrigin,omitempty"` // deprecated - PlaybackAllowOrigins AllowedOrigins `json:"playbackAllowOrigins"` - PlaybackTrustedProxies IPNetworks `json:"playbackTrustedProxies"` + Playback bool `json:"playback"` + PlaybackAddress string `json:"playbackAddress"` + PlaybackEncryption bool `json:"playbackEncryption"` + PlaybackServerKey string `json:"playbackServerKey"` + PlaybackServerCert string `json:"playbackServerCert"` + PlaybackAllowOrigin *string `json:"playbackAllowOrigin,omitempty"` // deprecated + PlaybackAllowOrigins []string `json:"playbackAllowOrigins"` + PlaybackTrustedProxies IPNetworks `json:"playbackTrustedProxies"` // RTSP server RTSP bool `json:"rtsp"` @@ -253,23 +252,23 @@ type Conf struct { RTMPServerCert string `json:"rtmpServerCert"` // HLS server - HLS bool `json:"hls"` - HLSDisable *bool `json:"hlsDisable,omitempty"` // deprecated - HLSAddress string `json:"hlsAddress"` - HLSEncryption bool `json:"hlsEncryption"` - HLSServerKey string `json:"hlsServerKey"` - HLSServerCert string `json:"hlsServerCert"` - HLSAllowOrigin *string `json:"hlsAllowOrigin,omitempty"` // deprecated - HLSAllowOrigins AllowedOrigins `json:"hlsAllowOrigins"` - HLSTrustedProxies IPNetworks `json:"hlsTrustedProxies"` - HLSAlwaysRemux bool `json:"hlsAlwaysRemux"` - HLSVariant HLSVariant `json:"hlsVariant"` - HLSSegmentCount int `json:"hlsSegmentCount"` - HLSSegmentDuration Duration `json:"hlsSegmentDuration"` - HLSPartDuration Duration `json:"hlsPartDuration"` - HLSSegmentMaxSize StringSize `json:"hlsSegmentMaxSize"` - HLSDirectory string `json:"hlsDirectory"` - HLSMuxerCloseAfter Duration `json:"hlsMuxerCloseAfter"` + HLS bool `json:"hls"` + HLSDisable *bool `json:"hlsDisable,omitempty"` // deprecated + HLSAddress string `json:"hlsAddress"` + HLSEncryption bool `json:"hlsEncryption"` + HLSServerKey string `json:"hlsServerKey"` + HLSServerCert string `json:"hlsServerCert"` + HLSAllowOrigin *string `json:"hlsAllowOrigin,omitempty"` // deprecated + HLSAllowOrigins []string `json:"hlsAllowOrigins"` + HLSTrustedProxies IPNetworks `json:"hlsTrustedProxies"` + HLSAlwaysRemux bool `json:"hlsAlwaysRemux"` + HLSVariant HLSVariant `json:"hlsVariant"` + HLSSegmentCount int `json:"hlsSegmentCount"` + HLSSegmentDuration Duration `json:"hlsSegmentDuration"` + HLSPartDuration Duration `json:"hlsPartDuration"` + HLSSegmentMaxSize StringSize `json:"hlsSegmentMaxSize"` + HLSDirectory string `json:"hlsDirectory"` + HLSMuxerCloseAfter Duration `json:"hlsMuxerCloseAfter"` // WebRTC server WebRTC bool `json:"webrtc"` @@ -279,7 +278,7 @@ type Conf struct { WebRTCServerKey string `json:"webrtcServerKey"` WebRTCServerCert string `json:"webrtcServerCert"` WebRTCAllowOrigin *string `json:"webrtcAllowOrigin,omitempty"` // deprecated - WebRTCAllowOrigins AllowedOrigins `json:"webrtcAllowOrigins"` + WebRTCAllowOrigins []string `json:"webrtcAllowOrigins"` WebRTCTrustedProxies IPNetworks `json:"webrtcTrustedProxies"` WebRTCLocalUDPAddress string `json:"webrtcLocalUDPAddress"` WebRTCLocalTCPAddress string `json:"webrtcLocalTCPAddress"` diff --git a/internal/conf/conf_test.go b/internal/conf/conf_test.go index 1ae06eab..8d1d8a3d 100644 --- a/internal/conf/conf_test.go +++ b/internal/conf/conf_test.go @@ -770,31 +770,3 @@ func TestSampleConfFile(t *testing.T) { require.Equal(t, conf1.Paths, conf2.Paths) }() } - -// needed due to https://github.com/golang/go/issues/21092 -func TestConfOverrideDefaultSlices(t *testing.T) { - tmpf, err := createTempFile([]byte( - "authInternalUsers:\n" + - " - user: user1\n" + - " - user: user2\n" + - "authHTTPExclude:\n" + - " - path: ''\n")) - require.NoError(t, err) - defer os.Remove(tmpf) - - conf, _, err := Load(tmpf, nil, nil) - require.NoError(t, err) - - require.Equal(t, AuthInternalUsers{ - { - User: "user1", - }, - { - User: "user2", - }, - }, conf.AuthInternalUsers) - - require.Equal(t, AuthInternalUserPermissions{ - {}, - }, conf.AuthHTTPExclude) -} diff --git a/internal/conf/jsonwrapper/unmarshal.go b/internal/conf/jsonwrapper/unmarshal.go index 62f5826c..27b14b20 100644 --- a/internal/conf/jsonwrapper/unmarshal.go +++ b/internal/conf/jsonwrapper/unmarshal.go @@ -5,18 +5,85 @@ import ( "bytes" "encoding/json" "io" + "reflect" + "strings" ) +// differences with respect to the standard package: +// - unknown fields cause an error +// - using existing elements of slices is prevented, fixing https://github.com/golang/go/issues/21092 + // Unmarshal decodes JSON. -// It returns an error if a non-existing field is found. func Unmarshal(buf []byte, dest any) error { return Decode(bytes.NewReader(buf), dest) } // Decode decodes JSON. -// It returns an error if a non-existing field is found. func Decode(r io.Reader, dest any) error { - d := json.NewDecoder(r) + buf, err := io.ReadAll(r) + if err != nil { + return err + } + + var raw any + err = json.Unmarshal(buf, &raw) + if err != nil { + return err + } + + nilExistingSlices(reflect.ValueOf(dest), raw) + + d := json.NewDecoder(bytes.NewReader(buf)) d.DisallowUnknownFields() return d.Decode(dest) } + +// nilExistingSlices recursively nils slices that are present in the JSON data. +func nilExistingSlices(v reflect.Value, jsonData any) { + if !v.IsValid() || jsonData == nil { + return + } + + for v.Kind() == reflect.Pointer { + if v.IsNil() { + return + } + v = v.Elem() + } + + switch v.Kind() { + case reflect.Slice: + if _, ok := jsonData.([]any); ok { + if !v.IsNil() { + v.Set(reflect.Zero(v.Type())) + } + return + } + + case reflect.Struct: + jsonMap, ok := jsonData.(map[string]any) + if !ok { + return + } + + vType := v.Type() + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + fieldType := vType.Field(i) + + jsonKey := fieldType.Tag.Get("json") + if jsonKey == "" || jsonKey == "-" { + continue + } + jsonKey = strings.Split(jsonKey, ",")[0] + + if jsonValue, exists := jsonMap[jsonKey]; exists { + if field.Kind() == reflect.Slice && !field.IsNil() { + field.Set(reflect.Zero(field.Type())) + } + + nilExistingSlices(field, jsonValue) + } + } + } +} diff --git a/internal/conf/jsonwrapper/unmarshal_test.go b/internal/conf/jsonwrapper/unmarshal_test.go new file mode 100644 index 00000000..713c8f78 --- /dev/null +++ b/internal/conf/jsonwrapper/unmarshal_test.go @@ -0,0 +1,74 @@ +package jsonwrapper + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +type testStruct struct { + Field1 string `json:"field1"` + Field2 int `json:"field2"` +} + +func TestUnmarshalDisallowUnknownFields(t *testing.T) { + input := strings.NewReader(`{"field1": "test", "unknownField": "value", "field2": 456}`) + var result testStruct + err := Decode(input, &result) + require.Error(t, err) + require.Contains(t, err.Error(), "unknown field") +} + +func TestUnmarshalPreventSliceReuse(t *testing.T) { + t.Run("a", func(t *testing.T) { + type Person struct { + Name string `json:"name"` + Age int `json:"age"` + } + + slice := []Person{ + {Name: "John", Age: 30}, + {Name: "Jane", Age: 25}, + } + + json := []byte(`[{"name": "Bob"}]`) + err := Unmarshal(json, &slice) + require.NoError(t, err) + + require.Equal(t, []Person{{ + Name: "Bob", + Age: 0, + }}, slice) + }) + + t.Run("b", func(t *testing.T) { + type Config struct { + Name string `json:"name"` + Enabled bool `json:"enabled"` + Port int `json:"port"` + } + type Settings struct { + Configs []Config `json:"configs"` + } + + settings := Settings{ + Configs: []Config{ + {Name: "old1", Enabled: true, Port: 8080}, + {Name: "old2", Enabled: false, Port: 9090}, + {Name: "old3", Enabled: true, Port: 7070}, + }, + } + + json := []byte(`{"configs": [{"name": "new1"}, {"name": "new2"}]}`) + err := Unmarshal(json, &settings) + require.NoError(t, err) + + require.Equal(t, Settings{ + Configs: []Config{ + {Name: "new1", Enabled: false, Port: 0}, + {Name: "new2", Enabled: false, Port: 0}, + }, + }, settings) + }) +} diff --git a/internal/conf/path.go b/internal/conf/path.go index fec6c86b..5b388bf2 100644 --- a/internal/conf/path.go +++ b/internal/conf/path.go @@ -106,7 +106,6 @@ func FindPathConf(pathConfs map[string]*Path, name string) (*Path, []string, err } // Path is a path configuration. -// WARNING: Avoid using slices directly due to https://github.com/golang/go/issues/21092 type Path struct { Regexp *regexp.Regexp `json:"-"` // filled by Check() Name string `json:"name"` // filled by Check() diff --git a/internal/conf/webrtc_ice_server.go b/internal/conf/webrtc_ice_server.go index 36af1fb0..64424ef3 100644 --- a/internal/conf/webrtc_ice_server.go +++ b/internal/conf/webrtc_ice_server.go @@ -1,7 +1,5 @@ package conf -import "github.com/bluenviron/mediamtx/internal/conf/jsonwrapper" - // WebRTCICEServer is a WebRTC ICE Server. type WebRTCICEServer struct { URL string `json:"url"` @@ -10,13 +8,5 @@ type WebRTCICEServer struct { ClientOnly bool `json:"clientOnly"` } -// WebRTCICEServers is a list of WebRTCICEServer +// WebRTCICEServers is a list of WebRTCICEServer. type WebRTCICEServers []WebRTCICEServer - -// UnmarshalJSON implements json.Unmarshaler. -func (s *WebRTCICEServers) UnmarshalJSON(b []byte) error { - // remove default value before loading new value - // https://github.com/golang/go/issues/21092 - *s = nil - return jsonwrapper.Unmarshal(b, (*[]WebRTCICEServer)(s)) -} diff --git a/internal/conf/yamlwrapper/unmarshal.go b/internal/conf/yamlwrapper/unmarshal.go index 69c15fdb..a516d012 100644 --- a/internal/conf/yamlwrapper/unmarshal.go +++ b/internal/conf/yamlwrapper/unmarshal.go @@ -8,6 +8,10 @@ import ( "gopkg.in/yaml.v2" ) +// differences with respect to the standard package: +// - unknown fields cause an error +// - integer map keys cause an error + func convertKeys(i any) (any, error) { switch x := i.(type) { case map[any]any: