Skip to content

Commit a2b0fbc

Browse files
rafaela-soaresrafaela-soares
authored
fix(memory consumption): improved SplitLines function calls (Checkmarx#5680)
* added pprof * improving splitLines usage * fixing tests * correcting unit tests * fixing tests * fixing more tests * fixing test
1 parent 033fb20 commit a2b0fbc

35 files changed

+717
-654
lines changed

‎cmd/builder/main.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,15 @@ func saveFile(filePath string, content []byte) error {
7171
if err != nil {
7272
return err
7373
}
74+
75+
defer func() {
76+
if err := f.Close(); err != nil {
77+
log.Err(err).Msgf("failed to close '%s'", filePath)
78+
}
79+
}()
80+
7481
if _, err := f.Write(content); err != nil {
7582
return err
7683
}
77-
return f.Close()
84+
return nil
7885
}

‎e2e/cli_test.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,12 @@ func loadTemplates(lines []string, templates testcases.TestTemplates) []string {
231231
return []string{}
232232
}
233233

234-
return strings.Split(builder.String(), "\n")
234+
t := builder.String()
235+
236+
builder.Reset()
237+
builder = nil
238+
239+
return strings.Split(t, "\n")
235240
}
236241

237242
func printTestDetails(output []string) {

‎pkg/builder/engine/engine.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,12 @@ func (e *Engine) expToString(expr hclsyntax.Expression) (string, error) {
152152
}
153153
return v.AsString(), nil
154154
}
155-
builder, err := e.buildString(t.Parts)
155+
builderString, err := e.buildString(t.Parts)
156156
if err != nil {
157157
return "", err
158158
}
159159

160-
return builder.String(), nil
160+
return builderString, nil
161161
case *hclsyntax.TemplateWrapExpr:
162162
return e.expToString(t.Wrapped)
163163
case *hclsyntax.ObjectConsKeyExpr:
@@ -180,16 +180,23 @@ func (e *Engine) expToString(expr hclsyntax.Expression) (string, error) {
180180
return "", fmt.Errorf("can't convert expression %T to string", expr)
181181
}
182182

183-
func (e *Engine) buildString(parts []hclsyntax.Expression) (strings.Builder, error) {
184-
var builder strings.Builder
183+
func (e *Engine) buildString(parts []hclsyntax.Expression) (string, error) {
184+
builder := &strings.Builder{}
185+
185186
for _, part := range parts {
186187
s, err := e.expToString(part)
187188
if err != nil {
188-
return strings.Builder{}, err
189+
return "", err
189190
}
190191
builder.WriteString(s)
191192
}
192-
return builder, nil
193+
194+
s := builder.String()
195+
196+
builder.Reset()
197+
builder = nil
198+
199+
return s, nil
193200
}
194201

195202
func (e *Engine) walkConstantItem(item hclsyntax.ObjectConsItem, walkHistory []build.PathItem) error {

‎pkg/builder/engine/engine_test.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package engine
22

33
import (
44
"reflect"
5-
"strings"
65
"testing"
76

87
build "github.com/Checkmarx/kics/pkg/builder/model"
@@ -108,7 +107,7 @@ func TestEngine_BuildString(t *testing.T) {
108107
name string
109108
args args
110109
fields fields
111-
want strings.Builder
110+
want string
112111
wantErr bool
113112
}{
114113
{
@@ -119,7 +118,7 @@ func TestEngine_BuildString(t *testing.T) {
119118
args: args{
120119
parts: []hclsyntax.Expression{},
121120
},
122-
want: strings.Builder{},
121+
want: "",
123122
wantErr: false,
124123
},
125124
}

‎pkg/detector/default_detect.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func (d defaultDetectLine) DetectLine(file *model.FileMetadata, searchKey string
3333
sanitizedSubstring = strings.Replace(sanitizedSubstring, str[0], `{{`+strconv.Itoa(idx)+`}}`, -1)
3434
}
3535

36-
lines := d.SplitLines(file.OriginalData)
36+
lines := *file.LinesOriginalData
3737
for _, key := range strings.Split(sanitizedSubstring, ".") {
3838
substr1, substr2 := GenerateSubstrings(key, extractedString)
3939

@@ -56,22 +56,17 @@ func (d defaultDetectLine) DetectLine(file *model.FileMetadata, searchKey string
5656

5757
return model.VulnerabilityLines{
5858
Line: undetectedVulnerabilityLine,
59-
VulnLines: []model.CodeLine{},
59+
VulnLines: &[]model.CodeLine{},
6060
ResolvedFile: detector.ResolvedFile,
6161
}
6262
}
6363

64-
func (d defaultDetectLine) SplitLines(content string) []string {
65-
text := strings.ReplaceAll(content, "\r", "")
66-
return strings.Split(text, "\n")
67-
}
68-
6964
func (d defaultDetectLine) prepareResolvedFiles(resFiles map[string]model.ResolvedFile) map[string]model.ResolvedFileSplit {
7065
resolvedFiles := make(map[string]model.ResolvedFileSplit)
7166
for f, res := range resFiles {
7267
resolvedFiles[f] = model.ResolvedFileSplit{
7368
Path: res.Path,
74-
Lines: d.SplitLines(string(res.Content)),
69+
Lines: *res.LinesContent,
7570
}
7671
}
7772
return resolvedFiles

‎pkg/detector/default_detect_test.go

+43-53
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,24 @@ import (
55
"testing"
66

77
"github.com/Checkmarx/kics/pkg/model"
8+
"github.com/Checkmarx/kics/pkg/utils"
89
"github.com/Checkmarx/kics/test"
910
"github.com/rs/zerolog"
1011
"github.com/stretchr/testify/require"
1112
)
1213

14+
var OriginalData = `resource "aws_s3_bucket" "b" {
15+
bucket = "my-tf-test-bucket"
16+
acl = "authenticated-read"
17+
18+
tags = {
19+
Name = "My bucket"
20+
Environment = "Dev.123"
21+
Environment = "test"
22+
}
23+
}
24+
`
25+
1326
// Test_detectLine tests the functions [detectLine()] and all the methods called by them
1427
func Test_detectLine(t *testing.T) { //nolint
1528
type args struct {
@@ -29,19 +42,11 @@ func Test_detectLine(t *testing.T) { //nolint
2942
name: "detect_line",
3043
args: args{
3144
file: &model.FileMetadata{
32-
ScanID: "scanID",
33-
ID: "Test",
34-
Kind: model.KindTerraform,
35-
OriginalData: `resource "aws_s3_bucket" "b" {
36-
bucket = "my-tf-test-bucket"
37-
acl = "authenticated-read"
38-
39-
tags = {
40-
Name = "My bucket"
41-
Environment = "Dev"
42-
}
43-
}
44-
`,
45+
ScanID: "scanID",
46+
ID: "Test",
47+
Kind: model.KindTerraform,
48+
OriginalData: OriginalData,
49+
LinesOriginalData: utils.SplitLines(OriginalData),
4550
},
4651
searchKey: "aws_s3_bucket[b].acl",
4752
},
@@ -50,14 +55,14 @@ func Test_detectLine(t *testing.T) { //nolint
5055
},
5156
want: model.VulnerabilityLines{
5257
Line: 3,
53-
VulnLines: []model.CodeLine{
58+
VulnLines: &[]model.CodeLine{
5459
{
5560
Position: 2,
56-
Line: ` bucket = "my-tf-test-bucket"`,
61+
Line: ` bucket = "my-tf-test-bucket"`,
5762
},
5863
{
5964
Position: 3,
60-
Line: ` acl = "authenticated-read"`,
65+
Line: ` acl = "authenticated-read"`,
6166
},
6267
{
6368
Position: 4,
@@ -71,20 +76,11 @@ func Test_detectLine(t *testing.T) { //nolint
7176
name: "detect_line_with_curly_brackets",
7277
args: args{
7378
file: &model.FileMetadata{
74-
ScanID: "scanID",
75-
ID: "Test",
76-
Kind: model.KindTerraform,
77-
OriginalData: `resource "aws_s3_bucket" "b" {
78-
bucket = "my-tf-test-bucket"
79-
acl = "authenticated-read"
80-
81-
tags = {
82-
Name = "My bucket"
83-
Environment = "Dev.123"
84-
Environment = "test"
85-
}
86-
}
87-
`,
79+
ScanID: "scanID",
80+
ID: "Test",
81+
Kind: model.KindTerraform,
82+
OriginalData: OriginalData,
83+
LinesOriginalData: utils.SplitLines(OriginalData),
8884
},
8985
searchKey: "aws_s3_bucket[b].Environment={{Dev.123}}",
9086
},
@@ -93,18 +89,18 @@ func Test_detectLine(t *testing.T) { //nolint
9389
},
9490
want: model.VulnerabilityLines{
9591
Line: 7,
96-
VulnLines: []model.CodeLine{
92+
VulnLines: &[]model.CodeLine{
9793
{
9894
Position: 6,
99-
Line: ` Name = "My bucket"`,
95+
Line: ` Name = "My bucket"`,
10096
},
10197
{
10298
Position: 7,
103-
Line: ` Environment = "Dev.123"`,
99+
Line: ` Environment = "Dev.123"`,
104100
},
105101
{
106102
Position: 8,
107-
Line: ` Environment = "test"`,
103+
Line: ` Environment = "test"`,
108104
},
109105
},
110106
LineWithVulnerabilty: "",
@@ -114,20 +110,11 @@ func Test_detectLine(t *testing.T) { //nolint
114110
name: "detect_line_error",
115111
args: args{
116112
file: &model.FileMetadata{
117-
ScanID: "scanID",
118-
ID: "Test",
119-
Kind: model.KindTerraform,
120-
OriginalData: `resource "aws_s3_bucket" "b" {
121-
bucket = "my-tf-test-bucket"
122-
acl = "authenticated-read"
123-
124-
tags = {
125-
Name = "My bucket"
126-
Environment = "Dev.123"
127-
Environment = "test"
128-
}
129-
}
130-
`,
113+
ScanID: "scanID",
114+
ID: "Test",
115+
Kind: model.KindTerraform,
116+
OriginalData: OriginalData,
117+
LinesOriginalData: utils.SplitLines(OriginalData),
131118
},
132119
searchKey: "testing.error",
133120
},
@@ -136,7 +123,7 @@ func Test_detectLine(t *testing.T) { //nolint
136123
},
137124
want: model.VulnerabilityLines{
138125
Line: -1,
139-
VulnLines: []model.CodeLine{},
126+
VulnLines: &[]model.CodeLine{},
140127
},
141128
},
142129
}
@@ -155,6 +142,10 @@ func Test_detectLine(t *testing.T) { //nolint
155142
}
156143
}
157144

145+
var content = []byte(
146+
`content1
147+
content2`)
148+
158149
func Test_defaultDetectLine_prepareResolvedFiles(t *testing.T) {
159150
type args struct {
160151
resFiles map[string]model.ResolvedFile
@@ -169,10 +160,9 @@ func Test_defaultDetectLine_prepareResolvedFiles(t *testing.T) {
169160
args: args{
170161
resFiles: map[string]model.ResolvedFile{
171162
"file1": {
172-
Content: []byte(
173-
`content1
174-
content2`),
175-
Path: "testing/file1",
163+
Content: content,
164+
Path: "testing/file1",
165+
LinesContent: utils.SplitLines(string(content)),
176166
},
177167
},
178168
},

‎pkg/detector/detector.go

+1-17
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
type kindDetectLine interface {
99
DetectLine(file *model.FileMetadata, searchKey string, outputLines int, logWithFields *zerolog.Logger) model.VulnerabilityLines
10-
SplitLines(content string) []string
1110
}
1211

1312
// DetectLine is a struct that associates a kindDetectLine to its FileKind
@@ -50,24 +49,9 @@ func (d *DetectLine) DetectLine(file *model.FileMetadata, searchKey string, logW
5049

5150
// GetAdjecent finds and returns the lines adjecent to the line containing the vulnerability
5251
func (d *DetectLine) GetAdjecent(file *model.FileMetadata, line int) model.VulnerabilityLines {
53-
if det, ok := d.detectors[file.Kind]; ok {
54-
return model.VulnerabilityLines{
55-
Line: line,
56-
VulnLines: GetAdjacentVulnLines(line-1, d.outputLines, det.SplitLines(file.OriginalData)),
57-
ResolvedFile: file.FilePath,
58-
}
59-
}
6052
return model.VulnerabilityLines{
6153
Line: line,
62-
VulnLines: GetAdjacentVulnLines(line-1, d.outputLines, d.defaultDetector.SplitLines(file.OriginalData)),
54+
VulnLines: GetAdjacentVulnLines(line-1, d.outputLines, *file.LinesOriginalData),
6355
ResolvedFile: file.FilePath,
6456
}
6557
}
66-
67-
// SplitLines splits lines splits the file by lines based on the type of file
68-
func (d *DetectLine) SplitLines(file *model.FileMetadata) []string {
69-
if det, ok := d.detectors[file.Kind]; ok {
70-
return det.SplitLines(file.OriginalData)
71-
}
72-
return d.defaultDetector.SplitLines(file.OriginalData)
73-
}

0 commit comments

Comments
 (0)