From e40ce9042b6fa55273a9904bcf57de7485668070 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Tue, 10 Nov 2026 21:03:23 +0000 Subject: [PATCH 1/8] Add Marshaler and Unmarshaler for custom key values A type with MarshalINI or UnmarshalINI formats or parses its own key value in ReflectFrom, MapTo and StrictMapTo, as a field, a pointer field or a slice element. List, ValidList, StrictList, In and Range use UnmarshalINI too. An error leaves the field unchanged, and StrictMapTo, StrictList and ReflectFrom return it. A struct type that implements either interface maps to a key instead of a section, so types such as a host:port pair can be one value. --- README.md | 2 +- ini.go | 3 +- struct.go | 108 ++++++++++++++++++++++++++++++++++++--------- struct_test.go | 116 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 71d658a..4a45c67 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Package ini reads and writes INI files in Go. - **Several sources**: files, `[]byte` and `io.Reader`, with later sources overriding earlier ones. - **Rich INI syntax**: parent-child sections, auto-increment keys, repeated (shadow) keys, nested and multiline values, and `%(name)s` references. - **Typed values**: numbers, booleans, durations and times, with generic lists and candidates of any of them, and ranges of all but booleans. -- **Struct mapping**: fill a struct from a file, or a file from a struct, guided by `ini` struct tags. +- **Struct mapping**: fill a struct from a file, or a file from a struct, guided by `ini` struct tags. Types can format and parse their own values with `MarshalINI` and `UnmarshalINI`. - **Safe writes**: comments and order are kept, and what is written loads back the same, or `Write` returns an error. - **Precise errors**: parse errors give the line and column. - **Concurrency**: safe for concurrent use, or lock-free with `Locking(false)` for files that never change. diff --git a/ini.go b/ini.go index 4351161..cf5f779 100644 --- a/ini.go +++ b/ini.go @@ -18,7 +18,8 @@ // MapTo and StrictMapTo fill a struct from a data source or a File, and // File.ReflectFrom fills a File from a struct. Field names map to key and // section names unchanged unless NameMapping sets a mapper, and the ini, -// delim and comment struct tags adjust each field. +// delim and comment struct tags adjust each field. A type that implements +// Marshaler or Unmarshaler formats or parses its own values. // // File.Write and File.SaveTo write a File back with its comments and in its // order, and WriteOption values set the formatting. What Write writes loads diff --git a/struct.go b/struct.go index c73efe8..13c7e14 100644 --- a/struct.go +++ b/struct.go @@ -61,18 +61,57 @@ func parseDelim(actual string) string { return "," } -var timeType = reflect.TypeOf(time.Time{}) +// Marshaler is implemented by types that format themselves as a key value. +// ReflectFrom calls MarshalINI for fields, pointer fields and slice elements +// of such a type, whatever its kind. +type Marshaler interface { + MarshalINI() ([]byte, error) +} + +// Unmarshaler is implemented by types that parse themselves from a key value. +// MapTo, StrictMapTo, List, ValidList, StrictList, In and Range call +// UnmarshalINI with the value as Key.String returns it, or with one element +// of a list. An error leaves the field unchanged, and StrictMapTo and +// StrictList return it. +// +// A struct type that implements Marshaler or Unmarshaler maps to a key, not +// to a section. +type Unmarshaler interface { + UnmarshalINI([]byte) error +} + +var ( + timeType = reflect.TypeFor[time.Time]() + marshalerType = reflect.TypeFor[Marshaler]() + unmarshalerType = reflect.TypeFor[Unmarshaler]() +) + +// isMarshaler and isUnmarshaler report whether t, or a pointer to t, +// implements Marshaler or Unmarshaler. +func isMarshaler(t reflect.Type) bool { + return reflect.PointerTo(t).Implements(marshalerType) +} + +func isUnmarshaler(t reflect.Type) bool { + return reflect.PointerTo(t).Implements(unmarshalerType) +} + +// isCustomType reports whether t formats or parses itself, so that it maps +// to a key value whatever its kind. +func isCustomType(t reflect.Type) bool { + return isMarshaler(t) || isUnmarshaler(t) +} // isStructType reports whether t is a struct type that maps to a section, -// which is any struct type but time.Time. +// which is any struct type but time.Time and custom types. func isStructType(t reflect.Type) bool { - return t.Kind() == reflect.Struct && t != timeType + return t.Kind() == reflect.Struct && t != timeType && !isCustomType(t) } // isValueType reports whether a key value can be parsed as or formatted from // a value of type t. func isValueType(t reflect.Type) bool { - if t == timeType { + if t == timeType || isCustomType(t) { return true } switch t.Kind() { @@ -87,7 +126,7 @@ func isValueType(t reflect.Type) bool { // isSliceElemType reports whether slices of t are supported. func isSliceElemType(t reflect.Type) bool { - if t == timeType { + if t == timeType || isCustomType(t) { return true } switch t.Kind() { @@ -100,6 +139,10 @@ func isSliceElemType(t reflect.Type) bool { // parseValue parses str as a value of type t, for which isValueType is true. func parseValue(t reflect.Type, str string) (reflect.Value, error) { v := reflect.New(t).Elem() + if isUnmarshaler(t) { + err := v.Addr().Interface().(Unmarshaler).UnmarshalINI([]byte(str)) + return v, err + } if t == timeType { ti, err := time.Parse(time.RFC3339, str) if err != nil { @@ -158,23 +201,40 @@ func parseValue(t reflect.Type, str string) (reflect.Value, error) { } // formatValue does the opposite thing as parseValue. -func formatValue(v reflect.Value) string { - if v.Type() == timeType { - return v.Interface().(time.Time).Format(time.RFC3339Nano) +func formatValue(v reflect.Value) (string, error) { + t := v.Type() + if isMarshaler(t) { + var m Marshaler + switch { + case t.Implements(marshalerType): + m = v.Interface().(Marshaler) + case v.CanAddr(): + m = v.Addr().Interface().(Marshaler) + default: + // MarshalINI has a pointer receiver, so call it on a copy. + p := reflect.New(t) + p.Elem().Set(v) + m = p.Interface().(Marshaler) + } + b, err := m.MarshalINI() + return string(b), err + } + if t == timeType { + return v.Interface().(time.Time).Format(time.RFC3339Nano), nil } switch v.Kind() { case reflect.String: - return v.String() + return v.String(), nil case reflect.Bool: - return fmt.Sprint(v.Bool()) + return fmt.Sprint(v.Bool()), nil case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return fmt.Sprint(v.Int()) + return fmt.Sprint(v.Int()), nil case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return fmt.Sprint(v.Uint()) + return fmt.Sprint(v.Uint()), nil case reflect.Float32, reflect.Float64: - return fmt.Sprint(v.Float()) + return fmt.Sprint(v.Float()), nil } - return "" + return "", fmt.Errorf("unsupported type %q", t) } // defaultComment sets *c to comment if it is empty, under the file's lock. @@ -258,7 +318,7 @@ func wrapStrictError(err error, isStrict bool) error { // leaves the field as it is, so it keeps its default, and is an error only // when isStrict. An unsupported type is always an error. func decodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, allowShadow, isStrict bool) error { - if t.Kind() == reflect.Slice { + if t.Kind() == reflect.Slice && !isCustomType(t) { return decodeSlice(key, field, delim, allowShadow, isStrict) } @@ -272,8 +332,9 @@ func decodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, al } str := key.String() - // An empty string keeps the default value of a string field. - if vt.Kind() == reflect.String && len(str) == 0 && !isPtr { + // An empty string keeps the default value of a string field, unless + // the type parses itself. + if vt.Kind() == reflect.String && len(str) == 0 && !isPtr && !isUnmarshaler(vt) { return nil } val, err := parseValue(vt, str) @@ -515,7 +576,10 @@ func encodeSlice(key *Key, field reflect.Value, delim string, allowShadow bool) vals := make([]string, field.Len()) for i := range vals { - vals[i] = formatValue(field.Index(i)) + var err error + if vals[i], err = formatValue(field.Index(i)); err != nil { + return err + } } if allowShadow { @@ -576,14 +640,18 @@ func escapeSliceElem(s, delim string) string { // encodeValue does the opposite thing as decodeValue. func encodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, allowShadow bool) error { switch { - case t.Kind() == reflect.Slice: + case t.Kind() == reflect.Slice && !isCustomType(t): return encodeSlice(key, field, delim, allowShadow) case t.Kind() == reflect.Pointer: if !field.IsNil() { return encodeValue(t.Elem(), key, field.Elem(), delim, allowShadow) } case isValueType(t): - key.SetValue(formatValue(field)) + v, err := formatValue(field) + if err != nil { + return err + } + key.SetValue(v) default: return fmt.Errorf("unsupported type %q", t) } diff --git a/struct_test.go b/struct_test.go index 4a314d8..376a214 100644 --- a/struct_test.go +++ b/struct_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "slices" + "strconv" "strings" "testing" "time" @@ -1437,6 +1438,10 @@ type fzCfg struct { Named fzName PNamed *fzName + Perm testPerm + PPerm *testPerm + Perms []testPerm + Sub fzSub PSub *fzSub Shadow []string `ini:"shadow,,allowshadow"` @@ -1455,6 +1460,7 @@ func FuzzStructMapping(f *testing.F) { "S = s\nI = -3\nI8 = 127\nU = 4\nU8 = 255\nF32 = 1.5\nF = 2.5\nB = true\nD = 1s\nT = 2020-01-02T03:04:05.5Z\n"+ "PS = p\nPI = 7\nPT = 2020-01-02T03:04:05Z\nPD = 3s\nSS = a,b\\, c,d\nSI = 1,2\nSU = 3,4\nSF = 1.5,2\nSB = true,false\n"+ "ST = 2020-01-02T03:04:05Z\nSP = x|y\nNamed = n\nPNamed = m\nshadow = a\nshadow = b\nE = e\n"+ + "Perm = write\nPPerm = read\nPerms = read, x, write\n"+ "[Sub]\nA = 1\nB = b\n[PSub]\nA = 2\n[Peer]\nA = 3\n[Peer]\nA = 4\n[PPeer]\nB = q\n[Node]\nX = 1\n[Child]\nX = 2\n", "I8 = 300\nU8 = -1\nF32 = 1e300\nD = -5s\nSI = 1,x,3\nT = 2006-01-02T15:04:05.123456789+07:00\n", "a = %(b)s\nb = %(a)s\nS = %(S)s%(S)s\n", @@ -1577,3 +1583,113 @@ func normalizeFzValue(v reflect.Value) { } } } + +// testPerm is an enum that reads and writes itself by name. +type testPerm int + +var testPermNames = []string{"none", "read", "write"} + +func (p *testPerm) UnmarshalINI(b []byte) error { + for i, name := range testPermNames { + if string(b) == name { + *p = testPerm(i) + return nil + } + } + return fmt.Errorf("unknown permission %q", b) +} + +func (p testPerm) MarshalINI() ([]byte, error) { + if int(p) >= len(testPermNames) { + return nil, fmt.Errorf("invalid permission %d", int(p)) + } + return []byte(testPermNames[p]), nil +} + +// testHostPort is a struct that maps to one key, written as host:port. +type testHostPort struct { + Host string + Port int +} + +func (h *testHostPort) UnmarshalINI(b []byte) error { + host, port, ok := strings.Cut(string(b), ":") + if !ok { + return fmt.Errorf("missing port in %q", b) + } + n, err := strconv.Atoi(port) + if err != nil { + return err + } + h.Host, h.Port = host, n + return nil +} + +func (h *testHostPort) MarshalINI() ([]byte, error) { + return []byte(h.Host + ":" + strconv.Itoa(h.Port)), nil +} + +func TestMarshalerUnmarshaler(t *testing.T) { + type config struct { + Perm testPerm + Perms []testPerm + PPerm *testPerm + Addr testHostPort + Addrs []testHostPort `delim:"|"` + } + + t.Run("read", func(t *testing.T) { + data := []byte("Perm = write\nPerms = read, none\nPPerm = read\nAddr = example.com:80\nAddrs = a:1|b:2\n" + + "[Addr]\nHost = from a section\n") + var c config + require.NoError(t, StrictMapTo(&c, data)) + read := testPerm(1) + assert.Equal(t, config{ + Perm: 2, + Perms: []testPerm{1, 0}, + PPerm: &read, + // A struct that parses itself maps to its key, not to the + // section of the same name. + Addr: testHostPort{"example.com", 80}, + Addrs: []testHostPort{{"a", 1}, {"b", 2}}, + }, c) + }) + + t.Run("write and read back", func(t *testing.T) { + read := testPerm(1) + in := config{Perm: 2, Perms: []testPerm{1, 0}, PPerm: &read, Addr: testHostPort{"h", 8}, Addrs: []testHostPort{{"a", 1}}} + data, err := marshal(in) + require.NoError(t, err) + assert.Equal(t, "Perm = write\nPerms = read,none\nPPerm = read\nAddr = h:8\nAddrs = a:1\n", string(data)) + + var out config + require.NoError(t, unmarshal(data, &out)) + assert.Equal(t, in, out) + }) + + t.Run("errors", func(t *testing.T) { + c := config{Perm: 1} + require.NoError(t, MapTo(&c, []byte("Perm = admin\n"))) + assert.Equal(t, testPerm(1), c.Perm, "an invalid value leaves the field unchanged") + + err := StrictMapTo(&c, []byte("Perm = admin\n")) + assert.ErrorContains(t, err, `unknown permission "admin"`) + assert.ErrorContains(t, StrictMapTo(&c, []byte("Perms = read, admin\n")), "admin") + + _, err = marshal(config{Perm: 9}) + assert.ErrorContains(t, err, "invalid permission 9") + _, err = marshal(config{Perms: []testPerm{1, 9}}) + assert.ErrorContains(t, err, "invalid permission 9") + }) + + t.Run("generic accessors", func(t *testing.T) { + f, err := Load([]byte("perms = write, x, read\nperm = write\n")) + require.NoError(t, err) + sec := f.Section("") + assert.Equal(t, []testPerm{2, 0, 1}, List[testPerm](sec.Key("perms"), ",")) + assert.Equal(t, []testPerm{2, 1}, ValidList[testPerm](sec.Key("perms"), ",")) + _, err = StrictList[testPerm](sec.Key("perms"), ",") + assert.Error(t, err) + assert.Equal(t, testPerm(2), In(sec.Key("perm"), 0, []testPerm{1, 2})) + }) +} From 68f90c307d7a6904014b8624a7ac63f5a9656e5a Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 00:16:05 -0500 Subject: [PATCH 2/8] Require both methods for a struct or slice type to map to one key A slice type with only MarshalINI lost list reading, and one with only UnmarshalINI lost list writing. A struct type with only one of them could no longer be read or written as a section. Both now keep their usual mapping unless they implement both methods. Also correct the docs of errors in slice elements, the generic accessors, which take Scalar types, and Scalar types with UnmarshalINI. --- key.go | 3 ++- struct.go | 25 +++++++++++++++++-------- struct_test.go | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/key.go b/key.go index 40d3063..ea96a7c 100644 --- a/key.go +++ b/key.go @@ -510,7 +510,8 @@ func (k *Key) SetValue(v string) { // Scalar is a type a value can be parsed as, as struct mapping parses // fields: strings, booleans, integers and floats of any size, named types of // those, time.Duration (as "1s" or nanoseconds) and time.Time (RFC 3339). -// A number that doesn't fit the type doesn't parse. +// A number that doesn't fit the type doesn't parse. A type that implements +// Unmarshaler parses with UnmarshalINI instead. type Scalar interface { ~string | ~bool | ~int | ~int8 | ~int16 | ~int32 | ~int64 | diff --git a/struct.go b/struct.go index 13c7e14..4f26645 100644 --- a/struct.go +++ b/struct.go @@ -63,19 +63,23 @@ func parseDelim(actual string) string { // Marshaler is implemented by types that format themselves as a key value. // ReflectFrom calls MarshalINI for fields, pointer fields and slice elements -// of such a type, whatever its kind. +// of such a type, whatever its kind. A struct or slice type also needs +// Unmarshaler, as described there. type Marshaler interface { MarshalINI() ([]byte, error) } // Unmarshaler is implemented by types that parse themselves from a key value. -// MapTo, StrictMapTo, List, ValidList, StrictList, In and Range call -// UnmarshalINI with the value as Key.String returns it, or with one element -// of a list. An error leaves the field unchanged, and StrictMapTo and -// StrictList return it. +// MapTo and StrictMapTo call UnmarshalINI for fields, pointer fields and slice +// elements of such a type, with the value as Key.String returns it, or with +// one element of a list, and so do List, ValidList, StrictList, In and Range +// for a Scalar type. An error is handled as for a value that doesn't parse: +// MapTo leaves a field unchanged and makes an element of a slice field the +// zero value, and StrictMapTo and StrictList return it. // -// A struct type that implements Marshaler or Unmarshaler maps to a key, not -// to a section. +// A struct or slice type maps to one key, rather than to a section or a +// list, only when it implements both Marshaler and Unmarshaler, so that it +// maps the same way in both directions. type Unmarshaler interface { UnmarshalINI([]byte) error } @@ -97,8 +101,13 @@ func isUnmarshaler(t reflect.Type) bool { } // isCustomType reports whether t formats or parses itself, so that it maps -// to a key value whatever its kind. +// to a key value whatever its kind. A struct or slice type, which otherwise +// maps to a section or a list, needs both methods. func isCustomType(t reflect.Type) bool { + switch t.Kind() { + case reflect.Struct, reflect.Slice: + return isMarshaler(t) && isUnmarshaler(t) + } return isMarshaler(t) || isUnmarshaler(t) } diff --git a/struct_test.go b/struct_test.go index 376a214..5f21a1a 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1692,4 +1692,36 @@ func TestMarshalerUnmarshaler(t *testing.T) { assert.Error(t, err) assert.Equal(t, testPerm(2), In(sec.Key("perm"), 0, []testPerm{1, 2})) }) + + t.Run("one method on a slice or struct type", func(t *testing.T) { + // Without both methods, a slice type is still a list and a struct + // type a section, in both directions. + type oneWay struct { + Hosts testMarshalOnlyList + Tags testUnmarshalOnlyList + Peer testMarshalOnlyStruct + } + var c oneWay + require.NoError(t, StrictMapTo(&c, []byte("Hosts = a,b\nTags = x,y\n[Peer]\nName = p\n"))) + assert.Equal(t, oneWay{Hosts: testMarshalOnlyList{"a", "b"}, Tags: testUnmarshalOnlyList{"x", "y"}, Peer: testMarshalOnlyStruct{Name: "p"}}, c) + + data, err := marshal(c) + require.NoError(t, err) + assert.Equal(t, "Hosts = a,b\nTags = x,y\n\n[Peer]\nName = p\n", string(data)) + }) } + +// testMarshalOnlyList, testUnmarshalOnlyList and testMarshalOnlyStruct +// implement one of Marshaler and Unmarshaler, with methods that must not be +// called. +type ( + testMarshalOnlyList []string + testUnmarshalOnlyList []string + testMarshalOnlyStruct struct{ Name string } +) + +func (testMarshalOnlyList) MarshalINI() ([]byte, error) { panic("called") } + +func (*testUnmarshalOnlyList) UnmarshalINI([]byte) error { panic("called") } + +func (testMarshalOnlyStruct) MarshalINI() ([]byte, error) { panic("called") } From d679f6cabaad9933023374cd7064137af9a1ec57 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 00:25:25 -0500 Subject: [PATCH 3/8] Note that embedding a type with MarshalINI and UnmarshalINI makes a struct one key --- struct.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/struct.go b/struct.go index 4f26645..f51c35f 100644 --- a/struct.go +++ b/struct.go @@ -79,7 +79,8 @@ type Marshaler interface { // // A struct or slice type maps to one key, rather than to a section or a // list, only when it implements both Marshaler and Unmarshaler, so that it -// maps the same way in both directions. +// maps the same way in both directions. A struct that embeds such a type +// gets its methods, and so maps to one key too. type Unmarshaler interface { UnmarshalINI([]byte) error } From 346a07c90a1e65db11ecc3843176b61f77f1d4df Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 00:29:02 -0500 Subject: [PATCH 4/8] Require both methods for any type that isn't a string, boolean or number A map or array type with only MarshalINI was written by ReflectFrom but skipped by MapTo without an error. Only string, boolean and number types, whose kind handles the other direction, now map to a key with one method. --- struct.go | 25 +++++++++++++++---------- struct_test.go | 17 +++++++++++++---- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/struct.go b/struct.go index f51c35f..4e7df48 100644 --- a/struct.go +++ b/struct.go @@ -63,8 +63,8 @@ func parseDelim(actual string) string { // Marshaler is implemented by types that format themselves as a key value. // ReflectFrom calls MarshalINI for fields, pointer fields and slice elements -// of such a type, whatever its kind. A struct or slice type also needs -// Unmarshaler, as described there. +// of such a type, whatever its kind. A type that isn't a string, boolean or +// number also needs Unmarshaler, as described there. type Marshaler interface { MarshalINI() ([]byte, error) } @@ -77,9 +77,10 @@ type Marshaler interface { // MapTo leaves a field unchanged and makes an element of a slice field the // zero value, and StrictMapTo and StrictList return it. // -// A struct or slice type maps to one key, rather than to a section or a -// list, only when it implements both Marshaler and Unmarshaler, so that it -// maps the same way in both directions. A struct that embeds such a type +// A type that isn't a string, boolean or number, such as a struct, slice or +// map type, maps to one key only when it implements both Marshaler and +// Unmarshaler, so that it maps the same way in both directions. Otherwise a +// struct type still maps to a section and a slice type to a list. A struct that embeds such a type // gets its methods, and so maps to one key too. type Unmarshaler interface { UnmarshalINI([]byte) error @@ -102,14 +103,18 @@ func isUnmarshaler(t reflect.Type) bool { } // isCustomType reports whether t formats or parses itself, so that it maps -// to a key value whatever its kind. A struct or slice type, which otherwise -// maps to a section or a list, needs both methods. +// to a key value whatever its kind. A string, boolean or number type needs +// one method, as its kind handles the other direction. Any other type needs +// both. func isCustomType(t reflect.Type) bool { switch t.Kind() { - case reflect.Struct, reflect.Slice: - return isMarshaler(t) && isUnmarshaler(t) + case reflect.String, reflect.Bool, + reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, + reflect.Float32, reflect.Float64: + return isMarshaler(t) || isUnmarshaler(t) } - return isMarshaler(t) || isUnmarshaler(t) + return isMarshaler(t) && isUnmarshaler(t) } // isStructType reports whether t is a struct type that maps to a section, diff --git a/struct_test.go b/struct_test.go index 5f21a1a..90a6903 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1693,7 +1693,7 @@ func TestMarshalerUnmarshaler(t *testing.T) { assert.Equal(t, testPerm(2), In(sec.Key("perm"), 0, []testPerm{1, 2})) }) - t.Run("one method on a slice or struct type", func(t *testing.T) { + t.Run("one method on a type that isn't a string, boolean or number", func(t *testing.T) { // Without both methods, a slice type is still a list and a struct // type a section, in both directions. type oneWay struct { @@ -1708,16 +1708,23 @@ func TestMarshalerUnmarshaler(t *testing.T) { data, err := marshal(c) require.NoError(t, err) assert.Equal(t, "Hosts = a,b\nTags = x,y\n\n[Peer]\nName = p\n", string(data)) + + // A map type with one method stays unsupported both ways. + type withMap struct{ M testMarshalOnlyMap } + assert.Error(t, StrictMapTo(&withMap{}, []byte("M = a\n"))) + _, err = marshal(withMap{M: testMarshalOnlyMap{"a": 1}}) + assert.Error(t, err) }) } -// testMarshalOnlyList, testUnmarshalOnlyList and testMarshalOnlyStruct -// implement one of Marshaler and Unmarshaler, with methods that must not be -// called. +// testMarshalOnlyList, testUnmarshalOnlyList, testMarshalOnlyStruct and +// testMarshalOnlyMap implement one of Marshaler and Unmarshaler, with methods +// that must not be called. type ( testMarshalOnlyList []string testUnmarshalOnlyList []string testMarshalOnlyStruct struct{ Name string } + testMarshalOnlyMap map[string]int ) func (testMarshalOnlyList) MarshalINI() ([]byte, error) { panic("called") } @@ -1725,3 +1732,5 @@ func (testMarshalOnlyList) MarshalINI() ([]byte, error) { panic("called") } func (*testUnmarshalOnlyList) UnmarshalINI([]byte) error { panic("called") } func (testMarshalOnlyStruct) MarshalINI() ([]byte, error) { panic("called") } + +func (testMarshalOnlyMap) MarshalINI() ([]byte, error) { panic("called") } From f83838337381096305a2304b38cf7038b460e38b Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 00:37:23 -0500 Subject: [PATCH 5/8] Map custom struct types to sections where sections are asked for Section.ReflectFrom on a file with AllowNonUniqueSections rejected a pointer to a struct type with MarshalINI and UnmarshalINI, while MapTo accepted it. A nonunique field or a value passed for such a section now maps any struct type but time.Time to sections, in both directions. --- struct.go | 22 ++++++++++++++++------ struct_test.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/struct.go b/struct.go index 4e7df48..149906f 100644 --- a/struct.go +++ b/struct.go @@ -80,7 +80,9 @@ type Marshaler interface { // A type that isn't a string, boolean or number, such as a struct, slice or // map type, maps to one key only when it implements both Marshaler and // Unmarshaler, so that it maps the same way in both directions. Otherwise a -// struct type still maps to a section and a slice type to a list. A struct that embeds such a type +// struct type still maps to a section and a slice type to a list. A nonunique +// field, or a value passed for a section of a file with +// AllowNonUniqueSections, maps a struct type to sections either way. A struct that embeds such a type // gets its methods, and so maps to one key too. type Unmarshaler interface { UnmarshalINI([]byte) error @@ -120,7 +122,15 @@ func isCustomType(t reflect.Type) bool { // isStructType reports whether t is a struct type that maps to a section, // which is any struct type but time.Time and custom types. func isStructType(t reflect.Type) bool { - return t.Kind() == reflect.Struct && t != timeType && !isCustomType(t) + return isSectionsType(t) && !isCustomType(t) +} + +// isSectionsType reports whether t can map to sections where they are asked +// for explicitly, by a nonunique field or a value passed for a section of a +// file with AllowNonUniqueSections: any struct type but time.Time, custom +// types too. +func isSectionsType(t reflect.Type) bool { + return t.Kind() == reflect.Struct && t != timeType } // isValueType reports whether a key value can be parsed as or formatted from @@ -478,7 +488,7 @@ func (s *Section) decodeSections(secName string, val reflect.Value, isStrict boo if isPtr { structType = elemType.Elem() } - if !isStructType(structType) { + if !isSectionsType(structType) { return reflect.Value{}, fmt.Errorf("unsupported type %q for non-unique sections", val.Type()) } @@ -789,7 +799,7 @@ func (s *Section) encodeStruct(val reflect.Value, path []reflect.Type) error { if structType.Kind() == reflect.Pointer { structType = structType.Elem() } - if !isStructType(structType) { + if !isSectionsType(structType) { return fmt.Errorf("field %q is not a slice of pointer or struct", fieldName) } if tooDeep(path, structType) { @@ -853,10 +863,10 @@ func (s *Section) ReflectFrom(v any) error { if !s.f.isDefaultSection(s.name) && s.f.options.AllowNonUniqueSections && (typ.Kind() == reflect.Slice || typ.Kind() == reflect.Pointer) { - if typ.Kind() == reflect.Pointer && !isStructType(typ.Elem()) { + if typ.Kind() == reflect.Pointer && !isSectionsType(typ.Elem()) { return errors.New("not a pointer to a struct") } - if typ.Kind() == reflect.Slice && (typ.Elem().Kind() != reflect.Pointer || !isStructType(typ.Elem().Elem())) { + if typ.Kind() == reflect.Slice && (typ.Elem().Kind() != reflect.Pointer || !isSectionsType(typ.Elem().Elem())) { return errors.New("not a slice of pointers to structs") } diff --git a/struct_test.go b/struct_test.go index 90a6903..e0f881b 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1693,6 +1693,37 @@ func TestMarshalerUnmarshaler(t *testing.T) { assert.Equal(t, testPerm(2), In(sec.Key("perm"), 0, []testPerm{1, 2})) }) + t.Run("sections asked for explicitly", func(t *testing.T) { + // A nonunique field, or a value passed for a section of a file with + // AllowNonUniqueSections, maps a custom struct type to sections. + f, err := Load([]byte("[peer]\nHost = a\nPort = 1\n[peer]\nHost = b\nPort = 2\n"), AllowNonUniqueSections(true)) + require.NoError(t, err) + var hp testHostPort + require.NoError(t, f.Section("peer").MapTo(&hp)) + assert.Equal(t, testHostPort{"a", 1}, hp) + var hps []*testHostPort + require.NoError(t, f.Section("peer").MapTo(&hps)) + assert.Equal(t, []*testHostPort{{"a", 1}, {"b", 2}}, hps) + + require.NoError(t, f.Section("peer").ReflectFrom(&testHostPort{"c", 3})) + assert.Equal(t, "c", f.Section("peer").Key("Host").String()) + require.NoError(t, f.Section("peer").ReflectFrom([]*testHostPort{{"d", 4}, {"e", 5}})) + secs, err := f.SectionsByName("peer") + require.NoError(t, err) + assert.Len(t, secs, 2) + + type peers struct { + Peers []testHostPort `ini:"peer,,,nonunique"` + } + in := peers{Peers: []testHostPort{{"a", 1}, {"b", 2}}} + data, err := marshal(in) + require.NoError(t, err) + assert.Equal(t, "[peer]\nHost = a\nPort = 1\n\n[peer]\nHost = b\nPort = 2\n", string(data)) + var out peers + require.NoError(t, unmarshal(data, &out)) + assert.Equal(t, in, out) + }) + t.Run("one method on a type that isn't a string, boolean or number", func(t *testing.T) { // Without both methods, a slice type is still a list and a struct // type a section, in both directions. From 2a7d2c125abc805230fe3c426d935bfb5b08fdcc Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 00:55:10 -0500 Subject: [PATCH 6/8] Test every type and field shape of struct mapping in both directions TestStructMappingMatrix maps types of each kind that struct mapping treats differently, with no methods, either method, or both, in every field shape: plain, pointer, slice, allowshadow, delim, in a section, nonunique, embedded, extends, and passed for a section. Each must round trip through ReflectFrom, Write and MapTo, or be unsupported in both directions. Rows for types without both methods match v2. It found one more mismatch, which v2 already had: a pointer to a slice was written as the slice but couldn't be read. It now reads as one. FuzzStructMapping also gets custom struct, slice and map fields, a pointer to a slice, and custom nonunique sections. --- struct.go | 11 + struct_matrix_test.go | 466 ++++++++++++++++++ struct_test.go | 11 + .../fuzz/FuzzStructMapping/25e29f72c5b46d57 | 3 + 4 files changed, 491 insertions(+) create mode 100644 struct_matrix_test.go create mode 100644 testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 diff --git a/struct.go b/struct.go index 149906f..a5ce2dc 100644 --- a/struct.go +++ b/struct.go @@ -352,6 +352,17 @@ func decodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, al if isPtr { vt = t.Elem() } + if isPtr && vt.Kind() == reflect.Slice && !isCustomType(vt) { + // A pointer to a slice is written as the slice, so it reads as one. + // A nil pointer isn't written, so the key sets the pointer, as for + // any pointer field. + p := reflect.New(vt) + if err := decodeSlice(key, p.Elem(), delim, allowShadow, isStrict); err != nil { + return err + } + field.Set(p) + return nil + } if !isValueType(vt) { return fmt.Errorf("unsupported type %q", t) } diff --git a/struct_matrix_test.go b/struct_matrix_test.go new file mode 100644 index 0000000..d13015c --- /dev/null +++ b/struct_matrix_test.go @@ -0,0 +1,466 @@ +package ini + +import ( + "bytes" + "errors" + "fmt" + "reflect" + "sort" + "strconv" + "strings" + "testing" +) + +// The Mx types cover each kind that struct mapping treats differently, +// without methods, with only MarshalINI, with only UnmarshalINI, and with +// both. A method of a type that must not use it returns errMxCalled. The +// methods of the other types read what they write, in a format that differs +// from the kind's own where both are implemented, so that the test sees which +// one ran. +var errMxCalled = errors.New("method must not be called") + +type ( + MxIntNone int + MxIntM int + MxIntU int + MxIntMU int +) + +func (v MxIntM) MarshalINI() ([]byte, error) { return []byte(strconv.Itoa(int(v))), nil } + +func (v *MxIntU) UnmarshalINI(b []byte) error { + n, err := strconv.Atoi(string(b)) + *v = MxIntU(n) + return err +} + +func (v MxIntMU) MarshalINI() ([]byte, error) { return []byte("#" + strconv.Itoa(int(v))), nil } + +func (v *MxIntMU) UnmarshalINI(b []byte) error { + s, ok := strings.CutPrefix(string(b), "#") + if !ok { + return fmt.Errorf("missing # in %q", b) + } + n, err := strconv.Atoi(s) + *v = MxIntMU(n) + return err +} + +type ( + MxStructNone struct{ A string } + MxStructM struct{ A string } + MxStructU struct{ A string } + MxStructMU struct{ A string } +) + +func (MxStructM) MarshalINI() ([]byte, error) { return nil, errMxCalled } + +func (*MxStructU) UnmarshalINI([]byte) error { return errMxCalled } + +// MarshalINI has a pointer receiver, unlike the other types. +func (v *MxStructMU) MarshalINI() ([]byte, error) { return []byte("<" + v.A + ">"), nil } + +func (v *MxStructMU) UnmarshalINI(b []byte) error { + s, ok := strings.CutPrefix(string(b), "<") + if s, ok2 := strings.CutSuffix(s, ">"); ok && ok2 { + v.A = s + return nil + } + return fmt.Errorf("missing <> in %q", b) +} + +type ( + MxSliceNone []string + MxSliceM []string + MxSliceU []string + MxSliceMU []string +) + +func (MxSliceM) MarshalINI() ([]byte, error) { return nil, errMxCalled } + +func (*MxSliceU) UnmarshalINI([]byte) error { return errMxCalled } + +func (v MxSliceMU) MarshalINI() ([]byte, error) { return []byte(strings.Join(v, "+")), nil } + +func (v *MxSliceMU) UnmarshalINI(b []byte) error { + *v = nil + if len(b) > 0 { + *v = strings.Split(string(b), "+") + } + return nil +} + +type ( + MxMapNone map[string]string + MxMapM map[string]string + MxMapU map[string]string + MxMapMU map[string]string +) + +func (MxMapM) MarshalINI() ([]byte, error) { return nil, errMxCalled } + +func (*MxMapU) UnmarshalINI([]byte) error { return errMxCalled } + +func (v MxMapMU) MarshalINI() ([]byte, error) { + var pairs []string + for k, val := range v { + pairs = append(pairs, k+":"+val) + } + sort.Strings(pairs) + return []byte(strings.Join(pairs, "&")), nil +} + +func (v *MxMapMU) UnmarshalINI(b []byte) error { + *v = nil + if len(b) == 0 { + return nil + } + *v = MxMapMU{} + for _, pair := range strings.Split(string(b), "&") { + k, val, ok := strings.Cut(pair, ":") + if !ok { + return fmt.Errorf("missing : in %q", pair) + } + (*v)[k] = val + } + return nil +} + +// mxSamples holds a value of each Mx type that is not the zero value. +var mxSamples = []any{ + MxIntNone(5), MxIntM(5), MxIntU(5), MxIntMU(5), + MxStructNone{"a"}, MxStructM{"a"}, MxStructU{"a"}, MxStructMU{"a"}, + MxSliceNone{"a", "b"}, MxSliceM{"a", "b"}, MxSliceU{"a", "b"}, MxSliceMU{"a", "b"}, + MxMapNone{"k": "v"}, MxMapM{"k": "v"}, MxMapU{"k": "v"}, MxMapMU{"k": "v"}, +} + +// mxInputs are inputs that read a value into a field F, or into F in a +// section S, in each of the forms struct mapping reads. +var mxInputs = []string{ + "F = 5\n", "F = #5\n", "F = a,b\n", "F = a+b\n", "F = \n", "F = k:v\n", + "F = ,\n", "F = #5,#6\n", + "[F]\nA = a\n", "[F]\nA = a\n[F]\nA = b\n", + "[S]\nF = 5\n", "[S]\nF = #5\n", "[S]\nF = a,b\n", "[S]\nF = a+b\n", "[S]\nF = \n", "[S]\nF = k:v\n", + "[F]\nA = a\n[S]\nB = b\n", "[S]\nA = a\nB = b\n", +} + +// mxShape is a way to hold a value of a type in a struct. +type mxShape struct { + name string + // typ returns the struct type that holds t, or nil when t can't be + // held that way. + typ func(t reflect.Type) reflect.Type + // set stores the sample value v of type t in the struct s. + set func(s, v reflect.Value) + // opts are the options the file is loaded and written with. + opts []LoadOption +} + +var mxShapes = []mxShape{ + { + name: "allowshadow field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.SliceOf(t), Tag: `ini:"F,,allowshadow"`}}) + }, + set: func(s, v reflect.Value) { + s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), v, v)) + }, + opts: []LoadOption{AllowShadows(true), AllowDuplicateShadowValues(true)}, + }, + { + name: "delim field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.SliceOf(t), Tag: `delim:"|"`}}) + }, + set: func(s, v reflect.Value) { + s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), v, v)) + }, + }, + { + name: "field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: t}}) + }, + set: func(s, v reflect.Value) { s.Field(0).Set(v) }, + }, + { + name: "pointer field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.PointerTo(t)}}) + }, + set: func(s, v reflect.Value) { + p := reflect.New(v.Type()) + p.Elem().Set(v) + s.Field(0).Set(p) + }, + }, + { + name: "slice field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.SliceOf(t)}}) + }, + set: func(s, v reflect.Value) { + s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), v, v)) + }, + }, + { + name: "field in a section", + typ: func(t reflect.Type) reflect.Type { + inner := reflect.StructOf([]reflect.StructField{{Name: "F", Type: t}}) + return reflect.StructOf([]reflect.StructField{{Name: "S", Type: inner}}) + }, + set: func(s, v reflect.Value) { s.Field(0).Field(0).Set(v) }, + }, + { + name: "nonunique field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.SliceOf(t), Tag: `ini:"F,,,nonunique"`}}) + }, + set: func(s, v reflect.Value) { + s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), v, v)) + }, + opts: []LoadOption{AllowNonUniqueSections(true)}, + }, + { + name: "nonunique pointer field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.SliceOf(reflect.PointerTo(t)), Tag: `ini:"F,,,nonunique"`}}) + }, + set: func(s, v reflect.Value) { + p := reflect.New(v.Type()) + p.Elem().Set(v) + s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), p, p)) + }, + opts: []LoadOption{AllowNonUniqueSections(true)}, + }, +} + +// mxResult is what happened to one value: "ok" when it was written and read +// back the same, or the step that failed. +func mxRoundTrip(in reflect.Value, opts []LoadOption) string { + f := Empty(opts...) + if err := f.ReflectFrom(in.Addr().Interface()); err != nil { + return "write error" + } + var buf bytes.Buffer + if _, err := f.Write(&buf); err != nil { + return "write error" + } + out := reflect.New(in.Type()) + if err := StrictMapTo(out.Interface(), buf.Bytes(), opts...); err != nil { + return fmt.Sprintf("written %q but StrictMapTo failed: %v", buf.String(), err) + } + if !reflect.DeepEqual(out.Elem().Interface(), in.Interface()) { + return fmt.Sprintf("written %q but read back %+v", buf.String(), out.Elem().Interface()) + } + lax := reflect.New(in.Type()) + if err := MapTo(lax.Interface(), buf.Bytes(), opts...); err != nil || !reflect.DeepEqual(lax.Elem().Interface(), in.Interface()) { + return fmt.Sprintf("written %q but MapTo read back %+v, %v", buf.String(), lax.Elem().Interface(), err) + } + return "ok" +} + +// mxReads returns the first of mxInputs that StrictMapTo reads into a value +// of type t that is not the zero value, or "". +func mxReads(t reflect.Type, opts []LoadOption) string { + for _, in := range mxInputs { + out := reflect.New(t) + if err := StrictMapTo(out.Interface(), []byte(in), opts...); err == nil && !out.Elem().IsZero() { + return in + } + } + return "" +} + +// TestStructMappingMatrix checks every Mx type in every shape: a value is +// either written and read back the same by both MapTo and StrictMapTo, or +// can't be written, and then no input reads a value of its type either. +func TestStructMappingMatrix(t *testing.T) { + for _, shape := range mxShapes { + for _, sample := range mxSamples { + v := reflect.ValueOf(sample) + name := shape.name + "/" + v.Type().Name() + st := shape.typ(v.Type()) + opts := shape.opts + + in := reflect.New(st).Elem() + shape.set(in, v) + res := mxRoundTrip(in, opts) + if res == "write error" { + if read := mxReads(st, opts); read != "" { + res = fmt.Sprintf("can't be written, but %q reads", read) + } + } + t.Logf("%s: %s", name, res) + if res != "ok" && res != "write error" { + t.Errorf("%s: %s", name, res) + } + } + } +} + +// The MxEmbed and MxExtends types embed each Mx struct type, plainly and +// with the extends tag. +type ( + MxEmbedNone struct { + MxStructNone + B string + } + MxEmbedM struct { + MxStructM + B string + } + MxEmbedU struct { + MxStructU + B string + } + MxEmbedMU struct { + MxStructMU + B string + } + MxExtendsNone struct { + MxStructNone `ini:",,,,extends"` + B string + } + MxExtendsM struct { + MxStructM `ini:",,,,extends"` + B string + } + MxExtendsU struct { + MxStructU `ini:",,,,extends"` + B string + } + MxExtendsMU struct { + MxStructMU `ini:",,,,extends"` + B string + } +) + +// TestStructMappingMatrixEmbedded checks the same for embedded struct types, +// held directly and in a section. +func TestStructMappingMatrixEmbedded(t *testing.T) { + for _, v := range []any{ + MxEmbedNone{MxStructNone{"a"}, "b"}, MxEmbedM{MxStructM{"a"}, "b"}, + MxEmbedU{MxStructU{"a"}, "b"}, MxEmbedMU{MxStructMU{"a"}, "b"}, + MxExtendsNone{MxStructNone{"a"}, "b"}, MxExtendsM{MxStructM{"a"}, "b"}, + MxExtendsU{MxStructU{"a"}, "b"}, MxExtendsMU{MxStructMU{"a"}, "b"}, + } { + for _, shape := range mxShapes { + if shape.name != "field" { + continue + } + rv := reflect.ValueOf(v) + for _, held := range []struct { + name string + typ reflect.Type + set func(s reflect.Value) + }{ + {"top level", rv.Type(), func(s reflect.Value) { s.Set(rv) }}, + {shape.name, shape.typ(rv.Type()), func(s reflect.Value) { shape.set(s, rv) }}, + } { + name := held.name + "/" + rv.Type().Name() + in := reflect.New(held.typ).Elem() + held.set(in) + if held.name != "top level" && strings.HasSuffix(name, "MU") { + // The embedded type's methods are promoted, so the field + // maps to one key, as Unmarshaler says, and B isn't kept. + f := Empty() + if err := f.ReflectFrom(in.Addr().Interface()); err != nil { + t.Errorf("%s: %v", name, err) + } + var buf bytes.Buffer + _, _ = f.Write(&buf) + if buf.String() != "F = \n" { + t.Errorf("%s: written %q, want one key", name, buf.String()) + } + continue + } + res := mxRoundTrip(in, nil) + if res == "write error" { + if read := mxReads(held.typ, nil); read != "" { + res = fmt.Sprintf("can't be written, but %q reads", read) + } + } + t.Logf("%s: %s", name, res) + if res != "ok" && res != "write error" { + t.Errorf("%s: %s", name, res) + } + } + } + } +} + +// TestStructMappingMatrixSections checks the same for values passed for a +// section, as one struct and, with AllowNonUniqueSections, as several. +func TestStructMappingMatrixSections(t *testing.T) { + for _, sample := range mxSamples[4:8] { + v := reflect.ValueOf(sample) + for _, nonUnique := range []bool{false, true} { + var opts []LoadOption + if nonUnique { + opts = []LoadOption{AllowNonUniqueSections(true)} + } + cases := []struct { + name string + in reflect.Value + }{{"struct", v}} + if nonUnique { + p := reflect.New(v.Type()) + p.Elem().Set(v) + cases = append(cases, struct { + name string + in reflect.Value + }{"slice", reflect.Append(reflect.MakeSlice(reflect.SliceOf(p.Type()), 0, 2), p, p)}) + } + for _, c := range cases { + name := fmt.Sprintf("%s/%s/nonunique=%v", c.name, v.Type().Name(), nonUnique) + res := mxSectionRoundTrip(c.in, opts) + t.Logf("%s: %s", name, res) + if res != "ok" && res != "write and read error" { + t.Errorf("%s: %s", name, res) + } + } + } + } +} + +// mxSectionRoundTrip writes in, a struct or a slice of pointers to structs, +// to section S with Section.ReflectFrom and reads it back with +// Section.StrictMapTo. +func mxSectionRoundTrip(in reflect.Value, opts []LoadOption) string { + arg := in.Interface() + if in.Kind() == reflect.Struct { + p := reflect.New(in.Type()) + p.Elem().Set(in) + arg = p.Interface() + } + f := Empty(opts...) + werr := f.Section("S").ReflectFrom(arg) + var buf bytes.Buffer + if werr == nil { + _, werr = f.Write(&buf) + } + + read := func(data string) (reflect.Value, error) { + g, err := Load([]byte(data), opts...) + if err != nil { + return reflect.Value{}, err + } + out := reflect.New(in.Type()) + return out.Elem(), g.Section("S").StrictMapTo(out.Interface()) + } + if werr != nil { + if out, err := read("[S]\nA = a\n[S]\nA = b\n"); err == nil && !out.IsZero() { + return fmt.Sprintf("can't be written (%v), but reads %+v", werr, out.Interface()) + } + return "write and read error" + } + out, err := read(buf.String()) + if err != nil { + return fmt.Sprintf("written %q but StrictMapTo failed: %v", buf.String(), err) + } + if !reflect.DeepEqual(out.Interface(), in.Interface()) { + return fmt.Sprintf("written %q but read back %+v", buf.String(), out.Interface()) + } + return "ok" +} diff --git a/struct_test.go b/struct_test.go index e0f881b..81209f3 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1442,6 +1442,15 @@ type fzCfg struct { PPerm *testPerm Perms []testPerm + PSS *[]string + Addr MxStructMU + PAddr *MxStructMU + Addrs []MxStructMU + Tags MxSliceMU + KV MxMapMU + Hosts MxSliceM + MPeers []MxStructMU `ini:"MPeer,,,nonunique"` + Sub fzSub PSub *fzSub Shadow []string `ini:"shadow,,allowshadow"` @@ -1464,6 +1473,8 @@ func FuzzStructMapping(f *testing.F) { "[Sub]\nA = 1\nB = b\n[PSub]\nA = 2\n[Peer]\nA = 3\n[Peer]\nA = 4\n[PPeer]\nB = q\n[Node]\nX = 1\n[Child]\nX = 2\n", "I8 = 300\nU8 = -1\nF32 = 1e300\nD = -5s\nSI = 1,x,3\nT = 2006-01-02T15:04:05.123456789+07:00\n", "a = %(b)s\nb = %(a)s\nS = %(S)s%(S)s\n", + "PSS = p,q\nAddr = \nPAddr = \nAddrs = ,\nTags = a+b\nKV = k:v&l:w\nHosts = h1,h2\n"+ + "[MPeer]\nA = m\n[MPeer]\nA = n\n", ) for _, in := range inputs { for _, flags := range []uint32{0, 1 << 7, 1<<14 | 1<<7, 1<<0 | 1<<14, 1<<20 - 1} { diff --git a/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 b/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 new file mode 100644 index 0000000..afdb7c5 --- /dev/null +++ b/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 @@ -0,0 +1,3 @@ +go test fuzz v1 +[]byte("PSS = ,") +uint32(16403) From 9a4d4b86df55ac823f8fcddf40ad2ff6366b1c36 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 07:49:58 -0500 Subject: [PATCH 7/8] Read pointers to pointers, and test arrays and interface fields ReflectFrom wrote a **T field as the value it points to, but MapTo rejected it, as it already did on v2. MapTo now reads any depth of pointers, keeping how a single pointer reads: a value that doesn't parse leaves it as it is. The matrix test gets pointer to pointer, array and interface fields, and array types with and without the methods. --- struct.go | 53 +++++++++++++++++++++++------------------ struct_matrix_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++- struct_test.go | 3 ++- 3 files changed, 86 insertions(+), 25 deletions(-) diff --git a/struct.go b/struct.go index a5ce2dc..fc1fa18 100644 --- a/struct.go +++ b/struct.go @@ -343,49 +343,56 @@ func wrapStrictError(err error, isStrict bool) error { // leaves the field as it is, so it keeps its default, and is an error only // when isStrict. An unsupported type is always an error. func decodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, allowShadow, isStrict bool) error { - if t.Kind() == reflect.Slice && !isCustomType(t) { + switch { + case t.Kind() == reflect.Slice && !isCustomType(t): return decodeSlice(key, field, delim, allowShadow, isStrict) - } - vt := t - isPtr := t.Kind() == reflect.Pointer - if isPtr { - vt = t.Elem() - } - if isPtr && vt.Kind() == reflect.Slice && !isCustomType(vt) { - // A pointer to a slice is written as the slice, so it reads as one. - // A nil pointer isn't written, so the key sets the pointer, as for - // any pointer field. - p := reflect.New(vt) - if err := decodeSlice(key, p.Elem(), delim, allowShadow, isStrict); err != nil { - return err + case t.Kind() == reflect.Pointer: + // A pointer, to a pointer too, is written as the value it points + // to, so it reads as one. A nil pointer isn't written, so the key + // sets the pointer, unless the value doesn't parse. A slice keeps + // its elements that don't parse as zero values, as without the + // pointer. + if !isDecodable(t.Elem()) { + return fmt.Errorf("unsupported type %q", t) + } + p := reflect.New(t.Elem()) + elemStrict := isStrict || t.Elem().Kind() != reflect.Slice || isCustomType(t.Elem()) + if err := decodeValue(t.Elem(), key, p.Elem(), delim, allowShadow, elemStrict); err != nil { + return wrapStrictError(err, isStrict) } field.Set(p) return nil } - if !isValueType(vt) { + + if !isValueType(t) { return fmt.Errorf("unsupported type %q", t) } - str := key.String() // An empty string keeps the default value of a string field, unless // the type parses itself. - if vt.Kind() == reflect.String && len(str) == 0 && !isPtr && !isUnmarshaler(vt) { + if t.Kind() == reflect.String && len(str) == 0 && !isUnmarshaler(t) { return nil } - val, err := parseValue(vt, str) + val, err := parseValue(t, str) if err != nil { return wrapStrictError(err, isStrict) } - if isPtr { - ptr := reflect.New(vt) - ptr.Elem().Set(val) - val = ptr - } field.Set(val) return nil } +// isDecodable reports whether decodeValue supports t. +func isDecodable(t reflect.Type) bool { + switch { + case t.Kind() == reflect.Slice && !isCustomType(t): + return isSliceElemType(t.Elem()) + case t.Kind() == reflect.Pointer: + return isDecodable(t.Elem()) + } + return isValueType(t) +} + func parseTagOptions(tag string) (rawName string, omitEmpty bool, allowShadow bool, allowNonUnique bool, extends bool) { opts := strings.SplitN(tag, ",", 5) rawName = opts[0] diff --git a/struct_matrix_test.go b/struct_matrix_test.go index d13015c..2775882 100644 --- a/struct_matrix_test.go +++ b/struct_matrix_test.go @@ -126,19 +126,42 @@ func (v *MxMapMU) UnmarshalINI(b []byte) error { return nil } +type ( + MxArrayNone [2]string + MxArrayM [2]string + MxArrayU [2]string + MxArrayMU [2]string +) + +func (MxArrayM) MarshalINI() ([]byte, error) { return nil, errMxCalled } + +func (*MxArrayU) UnmarshalINI([]byte) error { return errMxCalled } + +func (v MxArrayMU) MarshalINI() ([]byte, error) { return []byte(v[0] + "~" + v[1]), nil } + +func (v *MxArrayMU) UnmarshalINI(b []byte) error { + a, c, ok := strings.Cut(string(b), "~") + if !ok { + return fmt.Errorf("missing ~ in %q", b) + } + *v = MxArrayMU{a, c} + return nil +} + // mxSamples holds a value of each Mx type that is not the zero value. var mxSamples = []any{ MxIntNone(5), MxIntM(5), MxIntU(5), MxIntMU(5), MxStructNone{"a"}, MxStructM{"a"}, MxStructU{"a"}, MxStructMU{"a"}, MxSliceNone{"a", "b"}, MxSliceM{"a", "b"}, MxSliceU{"a", "b"}, MxSliceMU{"a", "b"}, MxMapNone{"k": "v"}, MxMapM{"k": "v"}, MxMapU{"k": "v"}, MxMapMU{"k": "v"}, + MxArrayNone{"a", "b"}, MxArrayM{"a", "b"}, MxArrayU{"a", "b"}, MxArrayMU{"a", "b"}, } // mxInputs are inputs that read a value into a field F, or into F in a // section S, in each of the forms struct mapping reads. var mxInputs = []string{ "F = 5\n", "F = #5\n", "F = a,b\n", "F = a+b\n", "F = \n", "F = k:v\n", - "F = ,\n", "F = #5,#6\n", + "F = ,\n", "F = #5,#6\n", "F = a~b\n", "[F]\nA = a\n", "[F]\nA = a\n[F]\nA = b\n", "[S]\nF = 5\n", "[S]\nF = #5\n", "[S]\nF = a,b\n", "[S]\nF = a+b\n", "[S]\nF = \n", "[S]\nF = k:v\n", "[F]\nA = a\n[S]\nB = b\n", "[S]\nA = a\nB = b\n", @@ -203,6 +226,36 @@ var mxShapes = []mxShape{ s.Field(0).Set(reflect.Append(reflect.MakeSlice(s.Field(0).Type(), 0, 2), v, v)) }, }, + { + name: "double pointer field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.PointerTo(reflect.PointerTo(t))}}) + }, + set: func(s, v reflect.Value) { + p := reflect.New(v.Type()) + p.Elem().Set(v) + pp := reflect.New(p.Type()) + pp.Elem().Set(p) + s.Field(0).Set(pp) + }, + }, + { + name: "array field", + typ: func(t reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.ArrayOf(2, t)}}) + }, + set: func(s, v reflect.Value) { + s.Field(0).Index(0).Set(v) + s.Field(0).Index(1).Set(v) + }, + }, + { + name: "interface field", + typ: func(reflect.Type) reflect.Type { + return reflect.StructOf([]reflect.StructField{{Name: "F", Type: reflect.TypeFor[any]()}}) + }, + set: func(s, v reflect.Value) { s.Field(0).Set(v) }, + }, { name: "field in a section", typ: func(t reflect.Type) reflect.Type { diff --git a/struct_test.go b/struct_test.go index 81209f3..7f264ec 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1443,6 +1443,7 @@ type fzCfg struct { Perms []testPerm PSS *[]string + PPI **int Addr MxStructMU PAddr *MxStructMU Addrs []MxStructMU @@ -1473,7 +1474,7 @@ func FuzzStructMapping(f *testing.F) { "[Sub]\nA = 1\nB = b\n[PSub]\nA = 2\n[Peer]\nA = 3\n[Peer]\nA = 4\n[PPeer]\nB = q\n[Node]\nX = 1\n[Child]\nX = 2\n", "I8 = 300\nU8 = -1\nF32 = 1e300\nD = -5s\nSI = 1,x,3\nT = 2006-01-02T15:04:05.123456789+07:00\n", "a = %(b)s\nb = %(a)s\nS = %(S)s%(S)s\n", - "PSS = p,q\nAddr = \nPAddr = \nAddrs = ,\nTags = a+b\nKV = k:v&l:w\nHosts = h1,h2\n"+ + "PSS = p,q\nPPI = 3\nAddr = \nPAddr = \nAddrs = ,\nTags = a+b\nKV = k:v&l:w\nHosts = h1,h2\n"+ "[MPeer]\nA = m\n[MPeer]\nA = n\n", ) for _, in := range inputs { From c84e3b48a230e09b5129e28516d34f882a678f48 Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Wed, 11 Nov 2026 08:02:20 -0500 Subject: [PATCH 8/8] Drop reading pointers to slices and to pointers They were written but not read on v2 too, and nobody asked for them, so they stay that way. The matrix test expects exactly those cases to be written but not read, and the fuzzer drops their fields. --- struct.go | 48 ++++++------------- struct_matrix_test.go | 21 ++++++++ struct_test.go | 4 +- .../fuzz/FuzzStructMapping/25e29f72c5b46d57 | 3 -- 4 files changed, 37 insertions(+), 39 deletions(-) delete mode 100644 testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 diff --git a/struct.go b/struct.go index fc1fa18..149906f 100644 --- a/struct.go +++ b/struct.go @@ -343,56 +343,38 @@ func wrapStrictError(err error, isStrict bool) error { // leaves the field as it is, so it keeps its default, and is an error only // when isStrict. An unsupported type is always an error. func decodeValue(t reflect.Type, key *Key, field reflect.Value, delim string, allowShadow, isStrict bool) error { - switch { - case t.Kind() == reflect.Slice && !isCustomType(t): + if t.Kind() == reflect.Slice && !isCustomType(t) { return decodeSlice(key, field, delim, allowShadow, isStrict) - - case t.Kind() == reflect.Pointer: - // A pointer, to a pointer too, is written as the value it points - // to, so it reads as one. A nil pointer isn't written, so the key - // sets the pointer, unless the value doesn't parse. A slice keeps - // its elements that don't parse as zero values, as without the - // pointer. - if !isDecodable(t.Elem()) { - return fmt.Errorf("unsupported type %q", t) - } - p := reflect.New(t.Elem()) - elemStrict := isStrict || t.Elem().Kind() != reflect.Slice || isCustomType(t.Elem()) - if err := decodeValue(t.Elem(), key, p.Elem(), delim, allowShadow, elemStrict); err != nil { - return wrapStrictError(err, isStrict) - } - field.Set(p) - return nil } - if !isValueType(t) { + vt := t + isPtr := t.Kind() == reflect.Pointer + if isPtr { + vt = t.Elem() + } + if !isValueType(vt) { return fmt.Errorf("unsupported type %q", t) } + str := key.String() // An empty string keeps the default value of a string field, unless // the type parses itself. - if t.Kind() == reflect.String && len(str) == 0 && !isUnmarshaler(t) { + if vt.Kind() == reflect.String && len(str) == 0 && !isPtr && !isUnmarshaler(vt) { return nil } - val, err := parseValue(t, str) + val, err := parseValue(vt, str) if err != nil { return wrapStrictError(err, isStrict) } + if isPtr { + ptr := reflect.New(vt) + ptr.Elem().Set(val) + val = ptr + } field.Set(val) return nil } -// isDecodable reports whether decodeValue supports t. -func isDecodable(t reflect.Type) bool { - switch { - case t.Kind() == reflect.Slice && !isCustomType(t): - return isSliceElemType(t.Elem()) - case t.Kind() == reflect.Pointer: - return isDecodable(t.Elem()) - } - return isValueType(t) -} - func parseTagOptions(tag string) (rawName string, omitEmpty bool, allowShadow bool, allowNonUnique bool, extends bool) { opts := strings.SplitN(tag, ",", 5) rawName = opts[0] diff --git a/struct_matrix_test.go b/struct_matrix_test.go index 2775882..6bd4cac 100644 --- a/struct_matrix_test.go +++ b/struct_matrix_test.go @@ -313,6 +313,19 @@ func mxRoundTrip(in reflect.Value, opts []LoadOption) string { return "ok" } +// mxWrittenNotRead reports whether a value of type t in the shape is written +// but can't be read: a pointer to a pointer to a type that maps to a key, +// and a pointer to a slice that is a list. Both were so before custom types. +func mxWrittenNotRead(shape string, t reflect.Type) bool { + switch shape { + case "double pointer field": + return isCustomType(t) || isValueType(t) || t.Kind() == reflect.Slice + case "pointer field": + return t.Kind() == reflect.Slice && !isCustomType(t) + } + return false +} + // mxReads returns the first of mxInputs that StrictMapTo reads into a value // of type t that is not the zero value, or "". func mxReads(t reflect.Type, opts []LoadOption) string { @@ -339,6 +352,14 @@ func TestStructMappingMatrix(t *testing.T) { in := reflect.New(st).Elem() shape.set(in, v) res := mxRoundTrip(in, opts) + if mxWrittenNotRead(shape.name, v.Type()) { + // Kept as on v2: written as the value pointed to, but not + // read. + if !strings.HasPrefix(res, "written") { + t.Errorf("%s: %s, want written but not read", name, res) + } + continue + } if res == "write error" { if read := mxReads(st, opts); read != "" { res = fmt.Sprintf("can't be written, but %q reads", read) diff --git a/struct_test.go b/struct_test.go index 7f264ec..b7e00ff 100644 --- a/struct_test.go +++ b/struct_test.go @@ -1442,8 +1442,6 @@ type fzCfg struct { PPerm *testPerm Perms []testPerm - PSS *[]string - PPI **int Addr MxStructMU PAddr *MxStructMU Addrs []MxStructMU @@ -1474,7 +1472,7 @@ func FuzzStructMapping(f *testing.F) { "[Sub]\nA = 1\nB = b\n[PSub]\nA = 2\n[Peer]\nA = 3\n[Peer]\nA = 4\n[PPeer]\nB = q\n[Node]\nX = 1\n[Child]\nX = 2\n", "I8 = 300\nU8 = -1\nF32 = 1e300\nD = -5s\nSI = 1,x,3\nT = 2006-01-02T15:04:05.123456789+07:00\n", "a = %(b)s\nb = %(a)s\nS = %(S)s%(S)s\n", - "PSS = p,q\nPPI = 3\nAddr = \nPAddr = \nAddrs = ,\nTags = a+b\nKV = k:v&l:w\nHosts = h1,h2\n"+ + "Addr = \nPAddr = \nAddrs = ,\nTags = a+b\nKV = k:v&l:w\nHosts = h1,h2\n"+ "[MPeer]\nA = m\n[MPeer]\nA = n\n", ) for _, in := range inputs { diff --git a/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 b/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 deleted file mode 100644 index afdb7c5..0000000 --- a/testdata/fuzz/FuzzStructMapping/25e29f72c5b46d57 +++ /dev/null @@ -1,3 +0,0 @@ -go test fuzz v1 -[]byte("PSS = ,") -uint32(16403)