Skip to content

Commit

Permalink
Address data race issues #487
Browse files Browse the repository at this point in the history
needed some locks
  • Loading branch information
daveshanley committed Apr 21, 2024
1 parent b62ed62 commit 9ba4676
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 14 deletions.
20 changes: 20 additions & 0 deletions functions/openapi/examples_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/sourcegraph/conc"
"gopkg.in/yaml.v3"
"strings"
"sync"
)

// ExamplesSchema will check anything that has an example, has a schema and it's valid.
Expand Down Expand Up @@ -50,6 +51,7 @@ func (es ExamplesSchema) RunRule(_ []*yaml.Node, context model.RuleFunctionConte
return result
}
wg := conc.WaitGroup{}
var expLock sync.Mutex

validator := schema_validation.NewSchemaValidator()
validateSchema := func(iKey *int,
Expand Down Expand Up @@ -122,7 +124,9 @@ func (es ExamplesSchema) RunRule(_ []*yaml.Node, context model.RuleFunctionConte
s.Value.GoLow().Examples.GetKeyNode(), example)

if result != nil {
expLock.Lock()
results = append(results, result...)
expLock.Unlock()
}
}
}
Expand All @@ -146,7 +150,9 @@ func (es ExamplesSchema) RunRule(_ []*yaml.Node, context model.RuleFunctionConte
result := validateSchema(nil, "", "example", s, s, s.Value.Example,
s.Value.GoLow().Example.GetKeyNode(), example)
if result != nil {
expLock.Lock()
results = append(results, result...)
expLock.Unlock()
}
}
})
Expand Down Expand Up @@ -194,11 +200,15 @@ func (es ExamplesSchema) RunRule(_ []*yaml.Node, context model.RuleFunctionConte
p := context.DrDocument.Parameters[i]
wg.Go(func() {
if p.Value.Examples.Len() >= 1 && p.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExamples(p.SchemaProxy.Schema, p, p.Value.Examples)...)
expLock.Unlock()
} else {
if p.Value.Example != nil && p.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExample(p.SchemaProxy.Schema, p.Value.Example,
p.Value.GoLow().Example.GetKeyNode())...)
expLock.Unlock()
}
}
})
Expand All @@ -210,31 +220,41 @@ func (es ExamplesSchema) RunRule(_ []*yaml.Node, context model.RuleFunctionConte
h := context.DrDocument.Headers[i]
wg.Go(func() {
if h.Value.Examples.Len() >= 1 && h.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExamples(h.SchemaProxy.Schema, h, h.Value.Examples)...)
expLock.Unlock()
} else {
if h.Value.Example != nil && h.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExample(h.SchemaProxy.Schema, h.Value.Example,
h.Value.GoLow().Example.GetKeyNode())...)
expLock.Unlock()
}
}
})
}
}

if context.DrDocument != nil && context.DrDocument.MediaTypes != nil {

for i := range context.DrDocument.MediaTypes {
mt := context.DrDocument.MediaTypes[i]
wg.Go(func() {
if mt.Value.Examples.Len() >= 1 && mt.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExamples(mt.SchemaProxy.Schema, mt, mt.Value.Examples)...)
expLock.Unlock()
} else {
if mt.Value.Example != nil && mt.SchemaProxy != nil {
expLock.Lock()
results = append(results, parseExample(mt.SchemaProxy.Schema, mt.Value.Example,
mt.Value.GoLow().Example.GetKeyNode())...)
expLock.Unlock()
}
}
})
}

}
wg.Wait()
return results
Expand Down
9 changes: 5 additions & 4 deletions motor/rule_applicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,11 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult {
done := make(chan bool)
indexConfig.Logger.Debug("running rules", "total", totalRules)
now = time.Now()

if execution.Timeout <= 0 {
execution.Timeout = time.Second * 5 // default
}

for _, rule := range execution.RuleSet.Rules {

go func(rule *model.Rule, done chan bool) {
Expand Down Expand Up @@ -594,10 +599,6 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult {
ctx.panicFunc = execution.PanicFunction
}

if execution.Timeout <= 0 {
execution.Timeout = time.Second * 5 // default
}

timeoutCtx, ruleCancel := context.WithTimeout(context.Background(), execution.Timeout)
defer ruleCancel()
doneChan := make(chan bool)
Expand Down
47 changes: 38 additions & 9 deletions motor/rule_applicator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@ package motor

import (
"fmt"
"github.com/daveshanley/vacuum/plugin"
"os"
"testing"
"time"

"github.com/daveshanley/vacuum/model"
"github.com/daveshanley/vacuum/plugin"
"github.com/daveshanley/vacuum/rulesets"
"github.com/pb33f/libopenapi"
"github.com/pb33f/libopenapi/datamodel"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
"os"
"testing"
)

func TestApplyRules_PostResponseSuccess(t *testing.T) {
Expand Down Expand Up @@ -1609,9 +1607,10 @@ components:
none:
type: int`)

panicRan := false
panicChan := make(chan bool)

saveMePlease := func(r any) {
panicRan = true
panicChan <- true
}

ApplyRulesToRuleSet(
Expand All @@ -1625,8 +1624,7 @@ components:
})

//nolint:staticcheck // ignore this linting issue, it's not a bug, it's on purpose.
time.Sleep(100)
assert.True(t, panicRan)
<-panicChan

}

Expand Down Expand Up @@ -2281,3 +2279,34 @@ func Benchmark_StripeSpecAgainstDefaultRuleSet(b *testing.B) {
assert.NotNil(b, results)
}
}

func Test_Issue486(t *testing.T) {

yml := `openapi: 3.1.0
paths:
/hi:
get:
requestBody:
content:
application/json:
schema:
$ref: "https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.0/schema.json"
`

rules := make(map[string]*model.Rule)
rules["oas3-host-not-example"] = rulesets.GetOAS3HostNotExampleRule()

rs := &rulesets.RuleSet{
Rules: rules,
}

rse := &RuleSetExecution{
RuleSet: rs,
Spec: []byte(yml),
AllowLookup: true,
}
results := ApplyRulesToRuleSet(rse)
assert.Len(t, results.Errors, 0)
assert.Len(t, results.Results, 4)

}
10 changes: 10 additions & 0 deletions rulesets/remote_ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,14 @@ func SniffOutAllExternalRules(

// iterate over the remote ruleset and add the rules in
for ruleName, ruleValue := range drs.Rules {
rs.mutex.Lock()
rs.Rules[ruleName] = ruleValue
rs.mutex.Unlock()
}
for ruleName, ruleValue := range drs.RuleDefinitions {
rs.mutex.Lock()
rs.RuleDefinitions[ruleName] = ruleValue
rs.mutex.Unlock()
}

visited = append(visited, location)
Expand All @@ -138,10 +142,14 @@ func SniffOutAllExternalRules(
// suck in all recommended rules
recommended := rsm.GenerateOpenAPIRecommendedRuleSet()
for k, v := range recommended.Rules {
rs.mutex.Lock()
rs.Rules[k] = v
rs.mutex.Unlock()
}
for k, v := range recommended.RuleDefinitions {
rs.mutex.Lock()
rs.RuleDefinitions[k] = v
rs.mutex.Unlock()
}
}

Expand All @@ -150,7 +158,9 @@ func SniffOutAllExternalRules(
// suck in all rules
allRules := rsm.openAPIRuleSet
for k, v := range allRules.Rules {
rs.mutex.Lock()
rs.Rules[k] = v
rs.mutex.Unlock()
}
for k, v := range allRules.RuleDefinitions {
rs.RuleDefinitions[k] = v
Expand Down
4 changes: 4 additions & 0 deletions rulesets/rulesets.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"time"

"github.com/daveshanley/vacuum/model"
Expand Down Expand Up @@ -274,6 +275,7 @@ func (rsm ruleSetsModel) GenerateRuleSetFromSuppliedRuleSet(ruleset *RuleSet) *R

// now all the base rules are in, let's run through the raw definitions and decide
// what we need to add, enable, disable, replace or change severity on.
rs.mutex.Lock()
for k, v := range rs.RuleDefinitions {

// let's try to cast to a string first (enable/disable/severity)
Expand Down Expand Up @@ -345,6 +347,7 @@ func (rsm ruleSetsModel) GenerateRuleSetFromSuppliedRuleSet(ruleset *RuleSet) *R
rs.Rules[k] = &nr
}
}
rs.mutex.Unlock()
return rs
}

Expand Down Expand Up @@ -492,6 +495,7 @@ type RuleSet struct {
Rules map[string]*model.Rule `json:"-" yaml:"-"`
Extends interface{} `json:"extends,omitempty" yaml:"extends,omitempty"` // can be string or tuple (again... why stoplight?)
extendsMeta map[string]string
mutex sync.Mutex
}

// GetExtendsValue returns an array of maps defining which ruleset this one extends. The value can be
Expand Down
2 changes: 1 addition & 1 deletion statistics/statistics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestCreateReportStatistics_BigLoadOfIssues(t *testing.T) {
selectedRS := defaultRuleSets.GenerateOpenAPIRecommendedRuleSet()
specBytes, _ := os.ReadFile("../model/test_files/api.github.com.yaml")

ctx, cancel := context.WithTimeout(context.Background(), time.Second*20)
ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
defer cancel()
d := make(chan bool)
go func(f chan bool) {
Expand Down

0 comments on commit 9ba4676

Please sign in to comment.