Skip to content

Commit

Permalink
Fix data race around accessing custom validators
Browse files Browse the repository at this point in the history
Because I already introduced a BC break, why not add another? The last
build mysteriously failed due to a data race that happens when accessing
the custom validators. You need to use the new `Get` and `Set` methods
on the `CustomTypeTagMap` struct which protects its private validator
type map with a mutex.
  • Loading branch information
annismckenzie committed May 17, 2016
1 parent 1ba975b commit da9d480
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 6 deletions.
22 changes: 21 additions & 1 deletion types.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package govalidator
import (
"reflect"
"regexp"
"sync"
)

// Validator is a wrapper for a validator function that returns bool and accepts string.
Expand Down Expand Up @@ -39,10 +40,29 @@ var ParamTagRegexMap = map[string]*regexp.Regexp{
"matches": regexp.MustCompile(`matches\(([^)]+)\)`),
}

type customTypeTagMap struct {
validators map[string]CustomTypeValidator

sync.RWMutex
}

func (tm *customTypeTagMap) Get(name string) (CustomTypeValidator, bool) {
tm.RLock()
defer tm.RUnlock()
v, ok := tm.validators[name]
return v, ok
}

func (tm *customTypeTagMap) Set(name string, ctv CustomTypeValidator) {
tm.Lock()
defer tm.Unlock()
tm.validators[name] = ctv
}

// CustomTypeTagMap is a map of functions that can be used as tags for ValidateStruct function.
// Use this to validate compound or custom types that need to be handled as a whole, e.g.
// `type UUID [16]byte` (this would be handled as an array of bytes).
var CustomTypeTagMap = map[string]CustomTypeValidator{}
var CustomTypeTagMap = &customTypeTagMap{validators: make(map[string]CustomTypeValidator)}

// TagMap is a map of functions, that can be used as tags for ValidateStruct function.
var TagMap = map[string]Validator{
Expand Down
2 changes: 1 addition & 1 deletion validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ func typeCheck(v reflect.Value, t reflect.StructField, o reflect.Value) (bool, e
if ok := isValidTag(tagOpts[0]); !ok {
continue
}
if validatefunc, ok := CustomTypeTagMap[tagOpts[0]]; ok {
if validatefunc, ok := CustomTypeTagMap.Get(tagOpts[0]); ok {
customTypeValidatorsExist = true
if result := validatefunc(v.Interface(), o.Interface()); !result {
if len(tagOpts) == 2 {
Expand Down
8 changes: 4 additions & 4 deletions validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1933,7 +1933,7 @@ func TestStructWithCustomByteArray(t *testing.T) {
t.Parallel()

// add our custom byte array validator that fails when the byte array is pristine (all zeroes)
CustomTypeTagMap["customByteArrayValidator"] = CustomTypeValidator(func(i interface{}, o interface{}) bool {
CustomTypeTagMap.Set("customByteArrayValidator", CustomTypeValidator(func(i interface{}, o interface{}) bool {
switch v := o.(type) {
case StructWithCustomByteArray:
if len(v.Email) > 0 {
Expand All @@ -1954,14 +1954,14 @@ func TestStructWithCustomByteArray(t *testing.T) {
}
}
return false
})
CustomTypeTagMap["customMinLengthValidator"] = CustomTypeValidator(func(i interface{}, o interface{}) bool {
}))
CustomTypeTagMap.Set("customMinLengthValidator", CustomTypeValidator(func(i interface{}, o interface{}) bool {
switch v := o.(type) {
case StructWithCustomByteArray:
return len(v.ID) >= v.CustomMinLength
}
return false
})
}))
testCustomByteArray := CustomByteArray{'1', '2', '3', '4', '5', '6'}
var tests = []struct {
param StructWithCustomByteArray
Expand Down

0 comments on commit da9d480

Please sign in to comment.