Test max include and max file count

bitrise-io · Jul 10, 2024 · d5d2b53 · d5d2b53
1 parent e8a54c2
commit d5d2b53
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 14 deletions.
diff --git a/configmerge/configmerge.go b/configmerge/configmerge.go
@@ -14,8 +14,9 @@ import (
 )
 
 const (
-	MaxFilesCountTotal = 20
-	MaxIncludeDepth    = 5 // root + 4 includes
+	MaxIncludeCountPerFile = 10
+	MaxFilesCountTotal     = 20
+	MaxIncludeDepth        = 5 // root + 4 includes
 )
 
 func IsModularConfig(mainConfigPth string) (bool, error) {
@@ -54,7 +55,6 @@ type Merger struct {
 	repoInfo RepoInfo
 
 	filesCount int
-	keys       []string
 }
 
 func NewMerger(repoInfoProvider RepoInfoProvider, fileReader FileReader, logger logV2.Logger) Merger {
@@ -86,7 +86,7 @@ func (m *Merger) MergeConfig(mainConfigPth string) (string, *models.ConfigFileTr
 		return "", nil, err
 	}
 
-	configTree, err := m.buildConfigTree(mainConfigBytes, mainConfigRef, 1)
+	configTree, err := m.buildConfigTree(mainConfigBytes, mainConfigRef, 1, nil)
 	if err != nil {
 		return "", nil, err
 	}
@@ -99,16 +99,13 @@ func (m *Merger) MergeConfig(mainConfigPth string) (string, *models.ConfigFileTr
 	return mergedConfigContent, configTree, nil
 }
 
-func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference, depth int) (*models.ConfigFileTreeModel, error) {
+func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference, depth int, keys []string) (*models.ConfigFileTreeModel, error) {
+	keys = append(keys, reference.Key())
+
 	if depth > MaxIncludeDepth {
 		return nil, fmt.Errorf("max include depth (%d) exceeded", MaxIncludeDepth)
 	}
 
-	if sliceutil.IsStringInSlice(reference.Key(), m.keys) {
-		return nil, fmt.Errorf("circular includes detected: %s -> %s", strings.Join(m.keys, " -> "), reference.Key())
-	}
-	m.keys = append(m.keys, reference.Key())
-
 	m.filesCount++
 	if m.filesCount > MaxFilesCountTotal {
 		return nil, fmt.Errorf("max include count (%d) exceeded", MaxFilesCountTotal)
@@ -131,10 +128,19 @@ func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference
 			include.Tag = reference.Tag
 		}
 		config.Include[idx] = include
+
+		if sliceutil.IsStringInSlice(include.Key(), keys) {
+			return nil, fmt.Errorf("circular reference detected: %s -> %s", strings.Join(keys, " -> "), include.Key())
+		}
+	}
+
+	if len(config.Include) > MaxIncludeCountPerFile {
+		return nil, fmt.Errorf("max include count (%d) exceeded", MaxIncludeCountPerFile)
+
 	}
 
 	if m.filesCount+len(config.Include) > MaxFilesCountTotal {
-		return nil, fmt.Errorf("max include count (%d) exceeded", MaxFilesCountTotal)
+		return nil, fmt.Errorf("max file count (%d) exceeded", MaxFilesCountTotal)
 	}
 
 	var includedConfigTrees []models.ConfigFileTreeModel
@@ -144,7 +150,7 @@ func (m *Merger) buildConfigTree(configContent []byte, reference ConfigReference
 			return nil, err
 		}
 
-		moduleConfigTree, err := m.buildConfigTree(moduleBytes, include, depth+1)
+		moduleConfigTree, err := m.buildConfigTree(moduleBytes, include, depth+1, keys)
 		if err != nil {
 			return nil, err
 		}

diff --git a/configmerge/configmerge_test.go b/configmerge/configmerge_test.go
@@ -43,6 +43,28 @@ include:
 			mainConfigPth: "bitrise.yml",
 			wantErr:       "circular includes detected: repo:https://github.com/bitrise-io/example.git,bitrise.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_1.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_2.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf -> repo:https://github.com/bitrise-io/example.git,module_1.yml@commit:016883ca9498f75d03cd45c0fa400ad9f8141edf",
 		},
+		{
+			name: "Max 10 include items are allowed",
+			repoInfoProvider: mockRepoInfoProvider{
+				repoInfo: &RepoInfo{
+					DefaultRemoteURL: "https://github.com/bitrise-io/example.git",
+					Branch:           "main",
+					Commit:           "016883ca9498f75d03cd45c0fa400ad9f8141edf",
+				},
+				err: nil,
+			},
+			fileReader: mockFileReader{
+				fileSystemFiles: map[string][]byte{
+					"bitrise.yml": []byte(fmt.Sprintf(`format_version: "15"
+default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git
+
+include:
+%s`, strings.Repeat("- path: path_1.yml\n", MaxIncludeCountPerFile+1))),
+				},
+			},
+			mainConfigPth: "bitrise.yml",
+			wantErr:       "max include count (10) exceeded",
+		},
 		{
 			name: "Max 20 config files are allowed",
 			repoInfoProvider: mockRepoInfoProvider{
@@ -59,11 +81,13 @@ include:
 default_step_lib_source: https://github.com/bitrise-io/bitrise-steplib.git
 
 include:
-%s`, strings.Repeat("- path: path_1.yml\n", MaxFilesCountTotal))),
+%s`, strings.Repeat("- path: path_1.yml\n", 10))),
+					"path_1.yml": []byte(`include:
+- path: path_2.yml`),
 				},
 			},
 			mainConfigPth: "bitrise.yml",
-			wantErr:       "max include count (20) exceeded",
+			wantErr:       "max file count (20) exceeded",
 		},
 		{
 			name: "Max include depth is 5",