Skip to content
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.

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

fix: issue4092 #4097

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 33 additions & 19 deletions util/gvalid/gvalid_validator_check_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,26 +287,40 @@ func (v *Validator) doCheckValueRecursively(ctx context.Context, in doCheckValue
}

case reflect.Slice, reflect.Array:
var array []interface{}
if gjson.Valid(in.Value) {
array = gconv.Interfaces(gconv.Bytes(in.Value))
} else {
array = gconv.Interfaces(in.Value)
}
if len(array) == 0 {
return
loop := false
switch in.Type.Elem().Kind() {
// []struct []map
case reflect.Struct, reflect.Map:
loop = true
case reflect.Ptr:
loop = true
Copy link
Member

Choose a reason for hiding this comment

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

  • 针对[]string, []int等基础类型也不支持了吗?这样会有兼容问题的。
  • 个人建议改进如下:
    • 针对太大块的提交内容,定义校验规则没有意义,建议使用侧去掉校验规则,
    • 组件增加对校验内容大小的限制,比如默认限制1K

Copy link
Member Author

Choose a reason for hiding this comment

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

  • 针对[]string, []int等基础类型也不支持了吗?这样会有兼容问题的。

  • 个人建议改进如下:

    • 针对太大块的提交内容,定义校验规则没有意义,建议使用侧去掉校验规则,
    • 组件增加对校验内容大小的限制,比如默认限制1K
  1. 并不是注释那样,注释想说明的是当切片的元素类型为指针类型时,和原逻辑一样,需要循环逐个验证。
  2. 其实仅就目前的规则来说,只有一个foreach规则会对切片类型进行循环验证,实现在 gf\util\gvalid\gvalid_validator_check_value.go:145行开始
  3. 当切片元素类型非struct,map时,完全没有必要去循环验证
  4. 建议对gvalid文档增加一些说明,规则可以作用于哪些类型
  5. gvalid在性能方面可能不尽人意,相对于go-playground/validator来说有几十倍的差距.
  6. 在ghttp中关于验证部分的逻辑直接写死了gvalid,go的其他主流框架,都有开放自定义模型验证这方面的接口
  7. gvalid年久失修,在实现上也不易维护,建议抛弃掉现在的gvalid,改用go-playground/validator,或为框架提供一个可以自定义验证的接口

Copy link
Contributor

Choose a reason for hiding this comment

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

是不是可以开个issue讨论下重新设计gvalid和gconv,印象中在好几个issue中都有讨论gvalid和gconv的性能问题,或者使用现成的社区方案,通过适配器去适配那些不错的社区库

Copy link
Member Author

Choose a reason for hiding this comment

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

是不是可以开个issue讨论下重新设计gvalid和gconv,印象中在好几个issue中都有讨论gvalid和gconv的性能问题,或者使用现成的社区方案,通过适配器去适配那些不错的社区库

gconv应该不用了,之前我重新实现过了,现在性能不成问题,主要是gvalid这个,年久失修,维护困难,性能落后

}
for _, item := range array {
v.doCheckValueRecursively(ctx, doCheckValueRecursivelyInput{
Value: item,
Type: in.Type.Elem(),
Kind: in.Type.Elem().Kind(),
ErrorMaps: in.ErrorMaps,
ResultSequenceRules: in.ResultSequenceRules,
})
// Bail feature.
if v.bail && len(in.ErrorMaps) > 0 {
break
// When it is a base type array,
// there is no need for recursive loop validation,
// otherwise it will cause memory leakage
// https://github.com/gogf/gf/issues/4092
if loop {
var array []interface{}
if gjson.Valid(in.Value) {
array = gconv.Interfaces(gconv.Bytes(in.Value))
} else {
array = gconv.Interfaces(in.Value)
}
if len(array) == 0 {
return
}
for _, item := range array {
v.doCheckValueRecursively(ctx, doCheckValueRecursivelyInput{
Value: item,
Type: in.Type.Elem(),
Kind: in.Type.Elem().Kind(),
ErrorMaps: in.ErrorMaps,
ResultSequenceRules: in.ResultSequenceRules,
})
// Bail feature.
if v.bail && len(in.ErrorMaps) > 0 {
break
}
}
}
}
Expand Down
35 changes: 35 additions & 0 deletions util/gvalid/gvalid_z_unit_issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ package gvalid_test
import (
"context"
"fmt"
"runtime"
"testing"
"time"

Expand Down Expand Up @@ -114,3 +115,37 @@ func Test_Issue3636(t *testing.T) {
)
})
}

// https://github.com/gogf/gf/issues/4092
func Test_Issue4092(t *testing.T) {
type Model struct {
Raw []byte `v:"required"`
Test []byte `v:"foreach|in:1,2,3"`
}
gtest.C(t, func(t *gtest.T) {
const kb = 1024
const mb = 1024 * kb
raw := make([]byte, 50*mb)
in := &Model{
Raw: raw,
Test: []byte{40, 5, 6},
}
err := g.Validator().
Data(in).
Run(context.Background())
t.Assert(err, "The Test value `6` is not in acceptable range: 1,2,3")
allocMb := getMemAlloc()
t.AssertLE(allocMb, 110)
})
}

func getMemAlloc() uint64 {
byteToMb := func(b uint64) uint64 {
return b / 1024 / 1024
}
var m runtime.MemStats
runtime.ReadMemStats(&m)
// For info on each, see: https://golang.org/pkg/runtime/#MemStats
alloc := byteToMb(m.Alloc)
return alloc
}
Loading