fix: Unmarshal struct decoding when passing in a pointer to a pointer

Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>

.github/workflows/ci.yaml

@@ -26,7 +26,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
         with:
-          go-version: "1.21"
+          go-version: '1.21'
 
       - name: Build
         run: go build .
@@ -44,8 +44,8 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        go: ["1.19", "1.20", "1.21"]
-        tags: ["", "finder", "viper_bind_struct"]
+        go: ['1.19', '1.20', '1.21']
+        tags: ['', 'finder']
 
     steps:
       - name: Checkout repository
@@ -75,7 +75,7 @@ jobs:
       - name: Set up Go
         uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
         with:
-          go-version: "1.21"
+          go-version: '1.21'
 
       - name: Lint
         uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3.7.0

.github/workflows/codeql-analysis.yaml

@@ -43,7 +43,7 @@ jobs:
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@b374143c1149a9115d881581d29b8390bbcbb59c # v3.22.11
+      uses: github/codeql-action/init@c0d1daa7f7e14667747d73a7dbbe8c074bc8bfe2 # v2.22.9
       with:
         languages: ${{ matrix.language }}
         # If you wish to specify custom queries, you can do so here or in a config file.
@@ -54,7 +54,7 @@ jobs:
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
-      uses: github/codeql-action/autobuild@b374143c1149a9115d881581d29b8390bbcbb59c # v3.22.11
+      uses: github/codeql-action/autobuild@c0d1daa7f7e14667747d73a7dbbe8c074bc8bfe2 # v2.22.9
 
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 https://git.io/JvXDl
@@ -68,5 +68,5 @@ jobs:
     #   make release
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@b374143c1149a9115d881581d29b8390bbcbb59c # v3.22.11
+      uses: github/codeql-action/analyze@c0d1daa7f7e14667747d73a7dbbe8c074bc8bfe2 # v2.22.9
 

README.md

@@ -802,7 +802,7 @@ if err != nil {
 }
 ```
 
-Viper uses [github.com/go-viper/mapstructure](https://github.com/go-viper/mapstructure) under the hood for unmarshaling values which uses `mapstructure` tags by default.
+Viper uses [github.com/mitchellh/mapstructure](https://github.com/mitchellh/mapstructure) under the hood for unmarshaling values which uses `mapstructure` tags by default.
 
 ### Decoding custom formats
 

go.mod

@@ -4,10 +4,10 @@ go 1.18
 
 require (
 	github.com/fsnotify/fsnotify v1.7.0
-	github.com/go-viper/mapstructure/v2 v2.0.0-alpha.1
 	github.com/hashicorp/hcl v1.0.0
 	github.com/magiconair/properties v1.8.7
-	github.com/pelletier/go-toml/v2 v2.1.1
+	github.com/mitchellh/mapstructure v1.5.0
+	github.com/pelletier/go-toml/v2 v2.1.0
 	github.com/sagikazarmark/crypt v0.17.0
 	github.com/sagikazarmark/locafero v0.4.0
 	github.com/sagikazarmark/slog-shim v0.1.0
@@ -49,7 +49,6 @@ require (
 	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/mattn/go-isatty v0.0.17 // indirect
 	github.com/mitchellh/go-homedir v1.1.0 // indirect
-	github.com/mitchellh/mapstructure v1.5.0 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/nats-io/nats.go v1.31.0 // indirect

go.sum

@@ -58,8 +58,6 @@ github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2
 github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
-github.com/go-viper/mapstructure/v2 v2.0.0-alpha.1 h1:TQcrn6Wq+sKGkpyPvppOz99zsMBaUOKXq6HSv655U1c=
-github.com/go-viper/mapstructure/v2 v2.0.0-alpha.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
 github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
@@ -199,8 +197,8 @@ github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OS
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
 github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
-github.com/pelletier/go-toml/v2 v2.1.1 h1:LWAJwfNvjQZCFIDKWYQaM62NcYeYViCmWIwmOStowAI=
-github.com/pelletier/go-toml/v2 v2.1.1/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
+github.com/pelletier/go-toml/v2 v2.1.0 h1:FnwAJ4oYMvbT/34k9zzHuZNrhlz48GB3/s6at6/MHO4=
+github.com/pelletier/go-toml/v2 v2.1.0/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=

internal/features/bind_struct.go

@@ -1,5 +0,0 @@
-//go:build viper_bind_struct
-
-package features
-
-const BindStruct = true

internal/features/bind_struct_default.go

@@ -1,5 +0,0 @@
-//go:build !viper_bind_struct
-
-package features
-
-const BindStruct = false

viper.go

@@ -34,7 +34,7 @@ import (
 	"time"
 
 	"github.com/fsnotify/fsnotify"
-	"github.com/go-viper/mapstructure/v2"
+	"github.com/mitchellh/mapstructure"
 	slog "github.com/sagikazarmark/slog-shim"
 	"github.com/spf13/afero"
 	"github.com/spf13/cast"
@@ -48,7 +48,6 @@ import (
 	"github.com/spf13/viper/internal/encoding/json"
 	"github.com/spf13/viper/internal/encoding/toml"
 	"github.com/spf13/viper/internal/encoding/yaml"
-	"github.com/spf13/viper/internal/features"
 )
 
 // ConfigMarshalError happens when failing to marshal the configuration.
@@ -1115,25 +1114,26 @@ func Unmarshal(rawVal any, opts ...DecoderConfigOption) error {
 }
 
 func (v *Viper) Unmarshal(rawVal any, opts ...DecoderConfigOption) error {
-	keys := v.AllKeys()
-
-	if features.BindStruct {
-		// TODO: make this optional?
-		structKeys, err := v.decodeStructKeys(rawVal, opts...)
-		if err != nil {
-			return err
-		}
-
-		keys = append(keys, structKeys...)
+	// TODO: make this optional?
+	structKeys, err := v.decodeStructKeys(rawVal, opts...)
+	if err != nil {
+		return err
 	}
 
 	// TODO: struct keys should be enough?
-	return decode(v.getSettings(keys), defaultDecoderConfig(rawVal, opts...))
+	return decode(v.getSettings(append(v.AllKeys(), structKeys...)), defaultDecoderConfig(rawVal, opts...))
 }
 
 func (v *Viper) decodeStructKeys(input any, opts ...DecoderConfigOption) ([]string, error) {
 	var structKeyMap map[string]any
 
+	rv := reflect.ValueOf(input)
+	for rv.Kind() == reflect.Ptr {
+		rv = reflect.Indirect(rv)
+	}
+
+	input = rv.Interface()
+
 	err := decode(input, defaultDecoderConfig(&structKeyMap, opts...))
 	if err != nil {
 		return nil, err
@@ -1158,8 +1158,7 @@ func defaultDecoderConfig(output any, opts ...DecoderConfigOption) *mapstructure
 		WeaklyTypedInput: true,
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
 			mapstructure.StringToTimeDurationHookFunc(),
-			// mapstructure.StringToSliceHookFunc(","),
-			stringToWeakSliceHookFunc(","),
+			mapstructure.StringToSliceHookFunc(","),
 		),
 	}
 	for _, opt := range opts {
@@ -1168,28 +1167,6 @@ func defaultDecoderConfig(output any, opts ...DecoderConfigOption) *mapstructure
 	return c
 }
 
-// As of mapstructure v2.0.0 StringToSliceHookFunc checks if the return type is a string slice.
-// This function removes that check.
-// TODO: implement a function that checks if the value can be converted to the return type and use it instead.
-func stringToWeakSliceHookFunc(sep string) mapstructure.DecodeHookFunc {
-	return func(
-		f reflect.Type,
-		t reflect.Type,
-		data interface{},
-	) (interface{}, error) {
-		if f.Kind() != reflect.String || t.Kind() != reflect.Slice {
-			return data, nil
-		}
-
-		raw := data.(string)
-		if raw == "" {
-			return []string{}, nil
-		}
-
-		return strings.Split(raw, sep), nil
-	}
-}
-
 // decode is a wrapper around mapstructure.Decode that mimics the WeakDecode functionality.
 func decode(input any, config *mapstructure.DecoderConfig) error {
 	decoder, err := mapstructure.NewDecoder(config)
@@ -1209,20 +1186,14 @@ func (v *Viper) UnmarshalExact(rawVal any, opts ...DecoderConfigOption) error {
 	config := defaultDecoderConfig(rawVal, opts...)
 	config.ErrorUnused = true
 
-	keys := v.AllKeys()
-
-	if features.BindStruct {
-		// TODO: make this optional?
-		structKeys, err := v.decodeStructKeys(rawVal, opts...)
-		if err != nil {
-			return err
-		}
-
-		keys = append(keys, structKeys...)
+	// TODO: make this optional?
+	structKeys, err := v.decodeStructKeys(rawVal, opts...)
+	if err != nil {
+		return err
 	}
 
 	// TODO: struct keys should be enough?
-	return decode(v.getSettings(keys), config)
+	return decode(v.getSettings(append(v.AllKeys(), structKeys...)), config)
 }
 
 // BindPFlags binds a full flag set to the configuration, using each flag's long

viper_test.go

@@ -21,14 +21,13 @@ import (
 	"time"
 
 	"github.com/fsnotify/fsnotify"
-	"github.com/go-viper/mapstructure/v2"
+	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/afero"
 	"github.com/spf13/cast"
 	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/spf13/viper/internal/features"
 	"github.com/spf13/viper/internal/testutil"
 )
 
@@ -957,10 +956,6 @@ func TestUnmarshalWithDecoderOptions(t *testing.T) {
 }
 
 func TestUnmarshalWithAutomaticEnv(t *testing.T) {
-	if !features.BindStruct {
-		t.Skip("binding struct is not enabled")
-	}
-
 	t.Setenv("PORT", "1313")
 	t.Setenv("NAME", "Steve")
 	t.Setenv("DURATION", "1s1ms")
@@ -1085,6 +1080,48 @@ func TestUnmarshalWithAutomaticEnv(t *testing.T) {
 	})
 }
 
+func TestUnmarshalIndirection(t *testing.T) {
+	v := New()
+
+	v.Set("foo", "bar")
+
+	type config struct {
+		Foo string
+	}
+
+	var C config
+
+	CC := &C
+	CCC := &CC
+
+	err := v.Unmarshal(&CCC)
+	require.NoError(t, err)
+
+	assert.Equal(t, config{"bar"}, C)
+}
+
+func TestUnmarshalInterface(t *testing.T) {
+	v := New()
+
+	v.Set("foo", "bar")
+
+	type someInterface interface {
+		Foo() string
+	}
+
+	type config struct {
+		Foo   string
+		Fooer someInterface
+	}
+
+	var C config
+
+	err := v.Unmarshal(&C)
+	require.NoError(t, err)
+
+	assert.Equal(t, config{Foo: "bar"}, C)
+}
+
 func TestBindPFlags(t *testing.T) {
 	v := New() // create independent Viper object
 	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)