Skip to content

Commit

Permalink
breaking: removes code parameter from ErrorLog and AuditLog (#800)
Browse files Browse the repository at this point in the history
  • Loading branch information
M4tteoP authored May 27, 2023
1 parent 1a4f355 commit 721d1de
Show file tree
Hide file tree
Showing 10 changed files with 24 additions and 25 deletions.
2 changes: 1 addition & 1 deletion examples/http-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@ func createWAF() coraza.WAF {
}

func logError(error types.MatchedRule) {
msg := error.ErrorLog(0)
msg := error.ErrorLog()
fmt.Printf("[logError][%s] %s", error.Rule().Severity(), msg)
}
2 changes: 1 addition & 1 deletion http/middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func TestChainEvaluation(t *testing.T) {

func errLogger(t *testing.T) func(rule types.MatchedRule) {
return func(rule types.MatchedRule) {
t.Log(rule.ErrorLog(0))
t.Log(rule.ErrorLog())
}
}

Expand Down
8 changes: 4 additions & 4 deletions internal/corazarules/rule_match.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ func (mr MatchedRule) matchData(log *strings.Builder, matchData types.MatchData)

// AuditLog transforms the matched rule into an error log
// using the legacy Modsecurity syntax
func (mr MatchedRule) AuditLog(code int) string {
func (mr MatchedRule) AuditLog() string {
log := &strings.Builder{}
for _, matchData := range mr.MatchedDatas_ {
fmt.Fprintf(log, "[client %q] ", mr.ClientIPAddress_)
if mr.Disruptive_ {
fmt.Fprintf(log, "Coraza: Access denied with code %d (phase %d). ", code, mr.Rule_.Phase())
fmt.Fprintf(log, "Coraza: Access denied (phase %d). ", mr.Rule_.Phase())
} else {
log.WriteString("Coraza: Warning. ")
}
Expand All @@ -187,7 +187,7 @@ func (mr MatchedRule) AuditLog(code int) string {
}

// ErrorLog returns the same as audit log but without matchData
func (mr MatchedRule) ErrorLog(code int) string {
func (mr MatchedRule) ErrorLog() string {
matchData := mr.MatchedDatas_[0]
msg := matchData.Message()
for _, md := range mr.MatchedDatas_ {
Expand All @@ -205,7 +205,7 @@ func (mr MatchedRule) ErrorLog(code int) string {

fmt.Fprintf(log, "[client %q] ", mr.ClientIPAddress_)
if mr.Disruptive_ {
fmt.Fprintf(log, "Coraza: Access denied with code %d (phase %d). ", code, mr.Rule_.Phase())
fmt.Fprintf(log, "Coraza: Access denied (phase %d). ", mr.Rule_.Phase())
} else {
log.WriteString("Coraza: Warning. ")
}
Expand Down
12 changes: 6 additions & 6 deletions internal/corazarules/rule_match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ func TestErrorLogMessagesSizesNoExtraRuleDetails(t *testing.T) {
},
},
}
LogSizeWithoutMsg := len(matchedRule.ErrorLog(403))
LogSizeWithoutMsg := len(matchedRule.ErrorLog())
matchedRule.MatchedDatas_[0].(*MatchData).Message_ = strings.Repeat("a", 300)
logWithMsg := matchedRule.ErrorLog(403)
logWithMsg := matchedRule.ErrorLog()
logSizeWithMsg := len(logWithMsg)
// The parent message is repeated twice when loggin error log
if lenDiff := logSizeWithMsg - LogSizeWithoutMsg; lenDiff != maxSizeLogMessage*2 {
t.Errorf("Expected message repeated twice with total len equal to %d, got %d", maxSizeLogMessage*2, lenDiff)
}
matchedRule.MatchedDatas_[0].(*MatchData).Data_ = strings.Repeat("b", 300)
logWithMsgData := matchedRule.ErrorLog(403)
logWithMsgData := matchedRule.ErrorLog()
logSizeWithMsgData := len(logWithMsgData)
// The parent message is repeated twice when loggin error log
if lenDiff := logSizeWithMsgData - logSizeWithMsg; lenDiff != maxSizeLogMessage {
Expand All @@ -47,7 +47,7 @@ func TestErrorLogMessagesSizesNoExtraRuleDetails(t *testing.T) {
t.Errorf("Expected string \"%s\" if not disruptive rule, got %s", noDisruptiveLine, logWithMsg)
}
matchedRule.Disruptive_ = true
logWithDisruptive := matchedRule.ErrorLog(403)
logWithDisruptive := matchedRule.ErrorLog()
disruptiveLine := "Coraza: Access denied"
if !strings.Contains(logWithDisruptive, disruptiveLine) {
t.Errorf("Expected string \"%s\" if disruptive rule, got %s", disruptiveLine, logWithMsg)
Expand Down Expand Up @@ -76,14 +76,14 @@ func TestErrorLogMessagesSizesWithExtraRuleDetails(t *testing.T) {
},
},
}
logWithExtraMsg := matchedRule.ErrorLog(403)
logWithExtraMsg := matchedRule.ErrorLog()
expectedExtraMsgLine := "\"" + strings.Repeat("c", maxSizeLogMessage) + "\""
if !strings.Contains(logWithExtraMsg, expectedExtraMsgLine) {
t.Errorf("Expected \"%s\" in log string, got %s", expectedExtraMsgLine, logWithExtraMsg)
}

matchedRule.MatchedDatas_[1].(*MatchData).Data_ = strings.Repeat("d", 300)
logWithExtraMsgData := matchedRule.ErrorLog(403)
logWithExtraMsgData := matchedRule.ErrorLog()

expectedExtraDataLine := "\"" + strings.Repeat("d", maxSizeLogMessage) + "\""
if !strings.Contains(logWithExtraMsgData, expectedExtraDataLine) {
Expand Down
3 changes: 1 addition & 2 deletions internal/corazawaf/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -1456,8 +1456,7 @@ func (tx *Transaction) String() string {
res := strings.Builder{}
res.WriteString("\n\n----------------------- ERRORLOG ----------------------\n")
for _, mr := range tx.matchedRules {
status, _ := strconv.Atoi(tx.variables.responseStatus.Get())
res.WriteString(mr.ErrorLog(status))
res.WriteString(mr.ErrorLog())
res.WriteString("\n\n----------------------- MATCHDATA ---------------------\n")
for _, md := range mr.MatchedDatas() {
fmt.Fprintf(&res, "%+v\n", md)
Expand Down
2 changes: 1 addition & 1 deletion internal/corazawaf/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ func TestLogCallback(t *testing.T) {
waf := NewWAF()
buffer := ""
waf.SetErrorCallback(func(mr types.MatchedRule) {
buffer = mr.ErrorLog(403)
buffer = mr.ErrorLog()
})
waf.RuleEngine = testCase.engineStatus
tx := waf.NewTransaction()
Expand Down
12 changes: 6 additions & 6 deletions internal/seclang/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func TestRuleLogging(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down Expand Up @@ -217,7 +217,7 @@ func TestTagsAreNotPrintedTwice(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down Expand Up @@ -249,7 +249,7 @@ func TestPrintedExtraMsgAndDataFromChainedRules(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down Expand Up @@ -286,7 +286,7 @@ func TestPrintedMultipleMsgAndDataWithMultiMatch(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestStatusFromInterruptions(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down Expand Up @@ -356,7 +356,7 @@ func TestLogsAreNotPrintedManyTimes(t *testing.T) {
waf := corazawaf.NewWAF()
var logs []string
waf.SetErrorCallback(func(mr types.MatchedRule) {
logs = append(logs, mr.ErrorLog(403))
logs = append(logs, mr.ErrorLog())
})
parser := NewParser(waf)
err := parser.FromString(`
Expand Down
2 changes: 1 addition & 1 deletion testing/coreruleset/coreruleset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ SecRule REQUEST_HEADERS:X-CRS-Test "@rx ^.*$" \
}
errorWriter := bufio.NewWriter(errorFile)
conf = conf.WithErrorCallback(func(rule types.MatchedRule) {
msg := rule.ErrorLog(0)
msg := rule.ErrorLog()
if _, err := io.WriteString(errorWriter, msg); err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion testing/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (t *Test) OutputErrors() []string {
// LogContains checks if the log contains a string
func (t *Test) LogContains(log string) bool {
for _, mr := range t.transaction.MatchedRules() {
if strings.Contains(mr.ErrorLog(t.ResponseCode), log) {
if strings.Contains(mr.ErrorLog(), log) {
return true
}
}
Expand Down
4 changes: 2 additions & 2 deletions types/rule_match.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ type MatchedRule interface {

Rule() RuleMetadata

AuditLog(code int) string
ErrorLog(code int) string
AuditLog() string
ErrorLog() string
}

0 comments on commit 721d1de

Please sign in to comment.