From 38b9a9ac1980ca2e446495316922b9b01967607a Mon Sep 17 00:00:00 2001 From: nikolay Date: Thu, 22 Feb 2024 19:32:47 +0700 Subject: [PATCH 1/4] Resolving Validating unexported fields #417 https://github.com/go-playground/validator/issues/417 --- Makefile | 2 +- cache.go | 4 ---- validator.go | 35 ++++++++++++++++++++++++++++++++--- validator_test.go | 9 ++++++--- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 8bf4f28a..e097dfaf 100644 --- a/Makefile +++ b/Makefile @@ -15,4 +15,4 @@ test: bench: $(GOCMD) test -run=NONE -bench=. -benchmem ./... -.PHONY: test lint linters-install \ No newline at end of file +.PHONY: test lint linters-install diff --git a/cache.go b/cache.go index 0f4fa6b5..e1eac704 100644 --- a/cache.go +++ b/cache.go @@ -126,10 +126,6 @@ func (v *Validate) extractStructCache(current reflect.Value, sName string) *cStr fld = typ.Field(i) - if !fld.Anonymous && len(fld.PkgPath) > 0 { - continue - } - if rtag, ok := rules[fld.Name]; ok { tag = rtag } else { diff --git a/validator.go b/validator.go index a072d39c..ce531fb1 100644 --- a/validator.go +++ b/validator.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "strconv" + "unsafe" ) // per validate construct @@ -410,7 +411,7 @@ OUTER: structNs: v.str2, fieldLen: uint8(len(cf.altName)), structfieldLen: uint8(len(cf.name)), - value: current.Interface(), + value: getValue(current), param: ct.param, kind: kind, typ: typ, @@ -430,7 +431,7 @@ OUTER: structNs: v.str2, fieldLen: uint8(len(cf.altName)), structfieldLen: uint8(len(cf.name)), - value: current.Interface(), + value: getValue(current), param: ct.param, kind: kind, typ: typ, @@ -470,7 +471,7 @@ OUTER: structNs: v.str2, fieldLen: uint8(len(cf.altName)), structfieldLen: uint8(len(cf.name)), - value: current.Interface(), + value: getValue(current), param: ct.param, kind: kind, typ: typ, @@ -484,3 +485,31 @@ OUTER: } } + +func getValue(val reflect.Value) any { + if val.CanInterface() { + return val.Interface() + } + + if val.CanAddr() { + return reflect.NewAt(val.Type(), unsafe.Pointer(val.UnsafeAddr())).Elem().Interface() + } + + if val.CanInt() { + return val.Int() + } + + if val.CanUint() { + return val.Uint() + } + + if val.CanComplex() { + return val.Complex() + } + + if val.CanFloat() { + return val.Float() + } + + return val.String() +} diff --git a/validator_test.go b/validator_test.go index 2826ae70..c2f8ae26 100644 --- a/validator_test.go +++ b/validator_test.go @@ -470,8 +470,9 @@ func TestAnonymous(t *testing.T) { errs := err.(ValidationErrors) - Equal(t, len(errs), 1) + Equal(t, len(errs), 2) AssertError(t, errs, "Test.AnonymousB.BEE", "Test.AnonymousB.B", "BEE", "B", "required") + AssertError(t, errs, "Test.anonymousC.c", "Test.anonymousC.c", "c", "c", "required") fe := getError(errs, "Test.AnonymousB.BEE", "Test.AnonymousB.B") NotEqual(t, fe, nil) @@ -485,7 +486,8 @@ func TestAnonymous(t *testing.T) { } err = validate.Struct(s) - Equal(t, err, nil) + NotEqual(t, err, nil) + AssertError(t, err, "c", "c", "c", "c", "required") } func TestAnonymousSameStructDifferentTags(t *testing.T) { @@ -7449,7 +7451,8 @@ func TestUnexposedStruct(t *testing.T) { Equal(t, s.unexposed.A, "") errs := validate.Struct(s) - Equal(t, errs, nil) + NotEqual(t, errs, nil) + AssertError(t, errs, "Test.unexposed.A", "Test.unexposed.A", "A", "A", "required") } func TestBadParams(t *testing.T) { From 1d6c9b700eec3affd8b58f0c68356c593d8f06c5 Mon Sep 17 00:00:00 2001 From: nikolay Date: Thu, 22 Feb 2024 19:59:15 +0700 Subject: [PATCH 2/4] add backward compatibility with 1.17 --- validator.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/validator.go b/validator.go index ce531fb1..014e4784 100644 --- a/validator.go +++ b/validator.go @@ -486,7 +486,7 @@ OUTER: } -func getValue(val reflect.Value) any { +func getValue(val reflect.Value) interface{} { if val.CanInterface() { return val.Interface() } @@ -495,21 +495,16 @@ func getValue(val reflect.Value) any { return reflect.NewAt(val.Type(), unsafe.Pointer(val.UnsafeAddr())).Elem().Interface() } - if val.CanInt() { + switch val.Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: return val.Int() - } - - if val.CanUint() { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: return val.Uint() - } - - if val.CanComplex() { + case reflect.Complex64, reflect.Complex128: return val.Complex() - } - - if val.CanFloat() { + case reflect.Float32, reflect.Float64: return val.Float() + default: + return val.String() } - - return val.String() } From adbe200cb46a232344c56093bdf13bcdf47057a7 Mon Sep 17 00:00:00 2001 From: nikolay Date: Thu, 22 Feb 2024 20:51:13 +0700 Subject: [PATCH 3/4] up test coverage --- validator.go | 2 +- validator_test.go | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/validator.go b/validator.go index 014e4784..901e7b50 100644 --- a/validator.go +++ b/validator.go @@ -157,7 +157,7 @@ func (v *validate) traverseField(ctx context.Context, parent reflect.Value, curr structNs: v.str2, fieldLen: uint8(len(cf.altName)), structfieldLen: uint8(len(cf.name)), - value: current.Interface(), + value: getValue(current), param: ct.param, kind: kind, typ: current.Type(), diff --git a/validator_test.go b/validator_test.go index c2f8ae26..ffbf1f3d 100644 --- a/validator_test.go +++ b/validator_test.go @@ -490,6 +490,108 @@ func TestAnonymous(t *testing.T) { AssertError(t, err, "c", "c", "c", "c", "required") } +func TestPrivateFieldsStruct(t *testing.T) { + type tc struct { + stct interface{} + errorNum int + } + + tcs := []tc{ + { + stct: &struct { + f1 int8 `validate:"required"` + f2 int16 `validate:"required"` + f3 int32 `validate:"required"` + f4 int64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: &struct { + f1 uint8 `validate:"required"` + f2 uint16 `validate:"required"` + f3 uint32 `validate:"required"` + f4 uint64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: &struct { + f1 complex64 `validate:"required"` + f2 complex128 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: &struct { + f1 float32 `validate:"required"` + f2 float64 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 int8 `validate:"required"` + f2 int16 `validate:"required"` + f3 int32 `validate:"required"` + f4 int64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: struct { + f1 uint8 `validate:"required"` + f2 uint16 `validate:"required"` + f3 uint32 `validate:"required"` + f4 uint64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: struct { + f1 complex64 `validate:"required"` + f2 complex128 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 float32 `validate:"required"` + f2 float64 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 *int `validate:"required"` + f2 struct { + f3 int `validate:"required"` + } + }{}, + errorNum: 2, + }, + { + stct: &struct { + f1 *int `validate:"required"` + f2 struct { + f3 int `validate:"required"` + } + }{}, + errorNum: 2, + }, + } + + validate := New() + + for _, tc := range tcs { + err := validate.Struct(tc.stct) + NotEqual(t, err, nil) + + errs := err.(ValidationErrors) + Equal(t, len(errs), tc.errorNum) + } +} + func TestAnonymousSameStructDifferentTags(t *testing.T) { validate := New() validate.RegisterTagNameFunc(func(fld reflect.StructField) string { From 6292cffac4bca34b4f8c633d975033291f3cf8ec Mon Sep 17 00:00:00 2001 From: nikolay Date: Fri, 23 Feb 2024 16:27:43 +0700 Subject: [PATCH 4/4] cr: introduce the configurable behaviour for the feature --- cache.go | 4 + options.go | 9 ++ validator_instance.go | 29 +++--- validator_test.go | 213 +++++++++++++++++++++--------------------- 4 files changed, 133 insertions(+), 122 deletions(-) diff --git a/cache.go b/cache.go index e1eac704..b6bdd11a 100644 --- a/cache.go +++ b/cache.go @@ -126,6 +126,10 @@ func (v *Validate) extractStructCache(current reflect.Value, sName string) *cStr fld = typ.Field(i) + if !v.privateFieldValidation && !fld.Anonymous && len(fld.PkgPath) > 0 { + continue + } + if rtag, ok := rules[fld.Name]; ok { tag = rtag } else { diff --git a/options.go b/options.go index 1dea56fd..5e1e40de 100644 --- a/options.go +++ b/options.go @@ -14,3 +14,12 @@ func WithRequiredStructEnabled() Option { v.requiredStructEnabled = true } } + +// WithPrivateFieldValidation activates validation for unexported fields +// +// It's experimental feature that partially uses unsafe package +func WithPrivateFieldValidation() Option { + return func(v *Validate) { + v.privateFieldValidation = true + } +} diff --git a/validator_instance.go b/validator_instance.go index d5a7be1d..1a345138 100644 --- a/validator_instance.go +++ b/validator_instance.go @@ -80,20 +80,21 @@ type internalValidationFuncWrapper struct { // Validate contains the validator settings and cache type Validate struct { - tagName string - pool *sync.Pool - tagNameFunc TagNameFunc - structLevelFuncs map[reflect.Type]StructLevelFuncCtx - customFuncs map[reflect.Type]CustomTypeFunc - aliases map[string]string - validations map[string]internalValidationFuncWrapper - transTagFunc map[ut.Translator]map[string]TranslationFunc // map[]map[]TranslationFunc - rules map[reflect.Type]map[string]string - tagCache *tagCache - structCache *structCache - hasCustomFuncs bool - hasTagNameFunc bool - requiredStructEnabled bool + tagName string + pool *sync.Pool + tagNameFunc TagNameFunc + structLevelFuncs map[reflect.Type]StructLevelFuncCtx + customFuncs map[reflect.Type]CustomTypeFunc + aliases map[string]string + validations map[string]internalValidationFuncWrapper + transTagFunc map[ut.Translator]map[string]TranslationFunc // map[]map[]TranslationFunc + rules map[reflect.Type]map[string]string + tagCache *tagCache + structCache *structCache + hasCustomFuncs bool + hasTagNameFunc bool + requiredStructEnabled bool + privateFieldValidation bool } // New returns a new instance of 'validate' with sane defaults. diff --git a/validator_test.go b/validator_test.go index ffbf1f3d..3b6d2634 100644 --- a/validator_test.go +++ b/validator_test.go @@ -470,9 +470,8 @@ func TestAnonymous(t *testing.T) { errs := err.(ValidationErrors) - Equal(t, len(errs), 2) + Equal(t, len(errs), 1) AssertError(t, errs, "Test.AnonymousB.BEE", "Test.AnonymousB.B", "BEE", "B", "required") - AssertError(t, errs, "Test.anonymousC.c", "Test.anonymousC.c", "c", "c", "required") fe := getError(errs, "Test.AnonymousB.BEE", "Test.AnonymousB.B") NotEqual(t, fe, nil) @@ -486,110 +485,7 @@ func TestAnonymous(t *testing.T) { } err = validate.Struct(s) - NotEqual(t, err, nil) - AssertError(t, err, "c", "c", "c", "c", "required") -} - -func TestPrivateFieldsStruct(t *testing.T) { - type tc struct { - stct interface{} - errorNum int - } - - tcs := []tc{ - { - stct: &struct { - f1 int8 `validate:"required"` - f2 int16 `validate:"required"` - f3 int32 `validate:"required"` - f4 int64 `validate:"required"` - }{}, - errorNum: 4, - }, - { - stct: &struct { - f1 uint8 `validate:"required"` - f2 uint16 `validate:"required"` - f3 uint32 `validate:"required"` - f4 uint64 `validate:"required"` - }{}, - errorNum: 4, - }, - { - stct: &struct { - f1 complex64 `validate:"required"` - f2 complex128 `validate:"required"` - }{}, - errorNum: 2, - }, - { - stct: &struct { - f1 float32 `validate:"required"` - f2 float64 `validate:"required"` - }{}, - errorNum: 2, - }, - { - stct: struct { - f1 int8 `validate:"required"` - f2 int16 `validate:"required"` - f3 int32 `validate:"required"` - f4 int64 `validate:"required"` - }{}, - errorNum: 4, - }, - { - stct: struct { - f1 uint8 `validate:"required"` - f2 uint16 `validate:"required"` - f3 uint32 `validate:"required"` - f4 uint64 `validate:"required"` - }{}, - errorNum: 4, - }, - { - stct: struct { - f1 complex64 `validate:"required"` - f2 complex128 `validate:"required"` - }{}, - errorNum: 2, - }, - { - stct: struct { - f1 float32 `validate:"required"` - f2 float64 `validate:"required"` - }{}, - errorNum: 2, - }, - { - stct: struct { - f1 *int `validate:"required"` - f2 struct { - f3 int `validate:"required"` - } - }{}, - errorNum: 2, - }, - { - stct: &struct { - f1 *int `validate:"required"` - f2 struct { - f3 int `validate:"required"` - } - }{}, - errorNum: 2, - }, - } - - validate := New() - - for _, tc := range tcs { - err := validate.Struct(tc.stct) - NotEqual(t, err, nil) - - errs := err.(ValidationErrors) - Equal(t, len(errs), tc.errorNum) - } + Equal(t, err, nil) } func TestAnonymousSameStructDifferentTags(t *testing.T) { @@ -7553,8 +7449,7 @@ func TestUnexposedStruct(t *testing.T) { Equal(t, s.unexposed.A, "") errs := validate.Struct(s) - NotEqual(t, errs, nil) - AssertError(t, errs, "Test.unexposed.A", "Test.unexposed.A", "A", "A", "required") + Equal(t, errs, nil) } func TestBadParams(t *testing.T) { @@ -13797,3 +13692,105 @@ func TestOmitNilAndRequired(t *testing.T) { AssertError(t, err2, "OmitNil.Inner.Str", "OmitNil.Inner.Str", "Str", "Str", "required") }) } + +func TestPrivateFieldsStruct(t *testing.T) { + type tc struct { + stct interface{} + errorNum int + } + + tcs := []tc{ + { + stct: &struct { + f1 int8 `validate:"required"` + f2 int16 `validate:"required"` + f3 int32 `validate:"required"` + f4 int64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: &struct { + f1 uint8 `validate:"required"` + f2 uint16 `validate:"required"` + f3 uint32 `validate:"required"` + f4 uint64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: &struct { + f1 complex64 `validate:"required"` + f2 complex128 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: &struct { + f1 float32 `validate:"required"` + f2 float64 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 int8 `validate:"required"` + f2 int16 `validate:"required"` + f3 int32 `validate:"required"` + f4 int64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: struct { + f1 uint8 `validate:"required"` + f2 uint16 `validate:"required"` + f3 uint32 `validate:"required"` + f4 uint64 `validate:"required"` + }{}, + errorNum: 4, + }, + { + stct: struct { + f1 complex64 `validate:"required"` + f2 complex128 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 float32 `validate:"required"` + f2 float64 `validate:"required"` + }{}, + errorNum: 2, + }, + { + stct: struct { + f1 *int `validate:"required"` + f2 struct { + f3 int `validate:"required"` + } + }{}, + errorNum: 2, + }, + { + stct: &struct { + f1 *int `validate:"required"` + f2 struct { + f3 int `validate:"required"` + } + }{}, + errorNum: 2, + }, + } + + validate := New(WithPrivateFieldValidation()) + + for _, tc := range tcs { + err := validate.Struct(tc.stct) + NotEqual(t, err, nil) + + errs := err.(ValidationErrors) + Equal(t, len(errs), tc.errorNum) + } +}