Skip to content

Fix ValidatePrivateFields: UpperFirst corrupts unexported embedded struct field paths #331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
inhere merged 2 commits into from
Mar 25, 2026
Merged

Fix ValidatePrivateFields: UpperFirst corrupts unexported embedded struct field paths #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50933f4
Initial plan
Copilot Mar 24, 2026
24b965f
Fix ValidatePrivateFields: UpperFirst breaks private embedded struct …
Copilot Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions data_source.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,12 @@ func (d *StructData) Get(field string) (val any, exist bool) {

// TryGet value by field name. support get sub-value by path.
func (d *StructData) TryGet(field string) (val any, exist, zero bool) {
field = strutil.UpperFirst(field)
// Only uppercase if the field is not already registered with its original casing.
// This allows private/unexported embedded struct fields to be accessed correctly
// when ValidatePrivateFields is enabled (e.g., "foo.Field1" must not become "Foo.Field1").
if _, ok := d.fieldNames[field]; !ok {
field = strutil.UpperFirst(field)
}
// try read from cache
if fv, ok := d.fieldValues[field]; ok {
return fv.Interface(), true, fv.IsZero()
Expand Down Expand Up @@ -632,7 +637,11 @@ func (d *StructData) TryGet(field string) (val any, exist, zero bool) {
//
// Notice: `StructData.src` the incoming struct must be a pointer to set the value
func (d *StructData) Set(field string, val any) (newVal any, err error) {
field = strutil.UpperFirst(field)
// Only uppercase if the field is not already registered with its original casing.
// This mirrors the same fix in TryGet for private embedded struct fields.
if _, ok := d.fieldNames[field]; !ok {
field = strutil.UpperFirst(field)
}
if !d.HasField(field) { // field not found
Comment on lines +640 to 645
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set can now skip UpperFirst for registered embedded-field paths (e.g. foo.Field1), but the rest of Set does not handle fieldAtAnonymous (the switch case is empty). If d.fieldNames[field] is fieldAtAnonymous, fv remains an invalid reflect.Value and removeValuePtr(fv) will panic. Consider handling fieldAtAnonymous the same as fieldAtSubStruct (walk the path and select the nested field), or otherwise return a controlled error instead of leaving fv invalid.

Copilot uses AI. Check for mistakes.
return nil, ErrNoField
}
Expand Down
21 changes: 11 additions & 10 deletions data_source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,23 +270,24 @@ func TestValidatePrivateFieldsWhenTrue(t *testing.T) {
Field2 int `validate:"required|int" message:"Field2 outside of range"`
}

fooInt := 4
barInt := 25

myFoo := foo{Field1: fooInt}
barz := &bar{
foo: myFoo,
Field2: barInt,
}

Config(func(opt *GlobalOption) {
opt.ValidatePrivateFields = true
})

// Field1 = 4 violates min:5 — validation must fail for the right reason (rule violation)
barz := &bar{
foo: foo{Field1: 4},
Field2: 25,
}
v := Struct(barz)
v.Validate()

assert.Equal(t, v.hasError, true)

// Field1 = 5 satisfies all rules — validation must pass
barz.foo.Field1 = 5
v = Struct(barz)
v.Validate()
assert.Equal(t, v.hasError, false)
Comment on lines 283 to +290
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions pass actual then expected to assert.Equal, which is inconsistent with the surrounding tests that use assert.Equal(t, expected, actual) (e.g. data_source_test.go:260-261, issues_test.go:511). Swapping the argument order (or using assert.True/False) will make assertion failures much easier to interpret.

Copilot uses AI. Check for mistakes.
}

func TestValidatePrivateFieldsWhenFalse(t *testing.T) {
Expand Down
Loading