From b962f3d7d5e503b4b971f9e4a20b7c824fa2c3a1 Mon Sep 17 00:00:00 2001 From: Dean Karn Date: Sun, 14 Jan 2018 13:58:17 -0800 Subject: [PATCH 1/2] Correct Var tagCache locking --- README.md | 2 +- validator.go | 2 -- validator_instance.go | 5 ++--- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index afe0ed1..e783725 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ Package validator ================ [![Join the chat at https://gitter.im/go-playground/validator](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/go-playground/validator?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) -![Project status](https://img.shields.io/badge/version-9.9.2-green.svg) +![Project status](https://img.shields.io/badge/version-9.9.3-green.svg) [![Build Status](https://semaphoreci.com/api/v1/joeybloggs/validator/branches/v9/badge.svg)](https://semaphoreci.com/joeybloggs/validator) [![Coverage Status](https://coveralls.io/repos/go-playground/validator/badge.svg?branch=v9&service=github)](https://coveralls.io/github/go-playground/validator?branch=v9) [![Go Report Card](https://goreportcard.com/badge/github.com/go-playground/validator)](https://goreportcard.com/report/github.com/go-playground/validator) diff --git a/validator.go b/validator.go index 10ed593..5fbb166 100644 --- a/validator.go +++ b/validator.go @@ -473,9 +473,7 @@ OUTER: ) return - } - ct = ct.next } } diff --git a/validator_instance.go b/validator_instance.go index d3a1543..5d2444a 100644 --- a/validator_instance.go +++ b/validator_instance.go @@ -524,7 +524,6 @@ func (v *Validate) VarCtx(ctx context.Context, field interface{}, tag string) (e ctag, ok := v.tagCache.Get(tag) if !ok { v.tagCache.lock.Lock() - defer v.tagCache.lock.Unlock() // could have been multiple trying to access, but once first is done this ensures tag // isn't parsed again. @@ -533,6 +532,7 @@ func (v *Validate) VarCtx(ctx context.Context, field interface{}, tag string) (e ctag, _ = v.parseFieldTagsRecursive(tag, "", "", false) v.tagCache.Set(tag, ctag) } + v.tagCache.lock.Unlock() } val := reflect.ValueOf(field) @@ -540,7 +540,6 @@ func (v *Validate) VarCtx(ctx context.Context, field interface{}, tag string) (e vd := v.pool.Get().(*validate) vd.top = val vd.isPartial = false - vd.traverseField(ctx, val, val, vd.ns[0:0], vd.actualNs[0:0], defaultCField, ctag) if len(vd.errs) > 0 { @@ -595,7 +594,6 @@ func (v *Validate) VarWithValueCtx(ctx context.Context, field interface{}, other ctag, ok := v.tagCache.Get(tag) if !ok { v.tagCache.lock.Lock() - defer v.tagCache.lock.Unlock() // could have been multiple trying to access, but once first is done this ensures tag // isn't parsed again. @@ -604,6 +602,7 @@ func (v *Validate) VarWithValueCtx(ctx context.Context, field interface{}, other ctag, _ = v.parseFieldTagsRecursive(tag, "", "", false) v.tagCache.Set(tag, ctag) } + v.tagCache.lock.Unlock() } otherVal := reflect.ValueOf(other) From e00f5e092aa90fe56394bb189640db2f9be50835 Mon Sep 17 00:00:00 2001 From: Dean Karn Date: Sun, 14 Jan 2018 14:10:57 -0800 Subject: [PATCH 2/2] split out to function, defer is needed to handle panics --- cache.go | 19 ++++++++++++++++++- validator_instance.go | 40 ++-------------------------------------- 2 files changed, 20 insertions(+), 39 deletions(-) diff --git a/cache.go b/cache.go index 1706274..3906d25 100644 --- a/cache.go +++ b/cache.go @@ -315,6 +315,23 @@ func (v *Validate) parseFieldTagsRecursive(tag string, fieldName string, alias s current.isBlockEnd = true } } - return } + +func (v *Validate) fetchCacheTag(tag string) *cTag { + // find cached tag + ctag, found := v.tagCache.Get(tag) + if !found { + v.tagCache.lock.Lock() + defer v.tagCache.lock.Unlock() + + // could have been multiple trying to access, but once first is done this ensures tag + // isn't parsed again. + ctag, found = v.tagCache.Get(tag) + if !found { + ctag, _ = v.parseFieldTagsRecursive(tag, "", "", false) + v.tagCache.Set(tag, ctag) + } + } + return ctag +} diff --git a/validator_instance.go b/validator_instance.go index 5d2444a..f4b49a8 100644 --- a/validator_instance.go +++ b/validator_instance.go @@ -520,23 +520,8 @@ func (v *Validate) VarCtx(ctx context.Context, field interface{}, tag string) (e return nil } - // find cached tag - ctag, ok := v.tagCache.Get(tag) - if !ok { - v.tagCache.lock.Lock() - - // could have been multiple trying to access, but once first is done this ensures tag - // isn't parsed again. - ctag, ok = v.tagCache.Get(tag) - if !ok { - ctag, _ = v.parseFieldTagsRecursive(tag, "", "", false) - v.tagCache.Set(tag, ctag) - } - v.tagCache.lock.Unlock() - } - + ctag := v.fetchCacheTag(tag) val := reflect.ValueOf(field) - vd := v.pool.Get().(*validate) vd.top = val vd.isPartial = false @@ -546,9 +531,7 @@ func (v *Validate) VarCtx(ctx context.Context, field interface{}, tag string) (e err = vd.errs vd.errs = nil } - v.pool.Put(vd) - return } @@ -589,36 +572,17 @@ func (v *Validate) VarWithValueCtx(ctx context.Context, field interface{}, other if len(tag) == 0 || tag == skipValidationTag { return nil } - - // find cached tag - ctag, ok := v.tagCache.Get(tag) - if !ok { - v.tagCache.lock.Lock() - - // could have been multiple trying to access, but once first is done this ensures tag - // isn't parsed again. - ctag, ok = v.tagCache.Get(tag) - if !ok { - ctag, _ = v.parseFieldTagsRecursive(tag, "", "", false) - v.tagCache.Set(tag, ctag) - } - v.tagCache.lock.Unlock() - } - + ctag := v.fetchCacheTag(tag) otherVal := reflect.ValueOf(other) - vd := v.pool.Get().(*validate) vd.top = otherVal vd.isPartial = false - vd.traverseField(ctx, otherVal, reflect.ValueOf(field), vd.ns[0:0], vd.actualNs[0:0], defaultCField, ctag) if len(vd.errs) > 0 { err = vd.errs vd.errs = nil } - v.pool.Put(vd) - return }