Skip to content

Commit 75de4a8

Browse files
RichardSimisonRayze-Bm39johnsonm
authored
Issue #396 Circular Symlinks Bug (#694)
* Added a duplicate file check case into file.go * Created an automated test to ensure the program can detect broken symlinks and stop the program rather than hanging indefinitely * Test: Created a Circular symlink test, verifies that scc detects symlink loops, and skips instead of running indefinitely (Michael) * Added an if-statement to TestNewFileJobBrokenSymLink to skip the test on Windows machines, because Windows requires administrator mode or developer mode to be enabled to create symlinks. * Test: Created a Circular symlink test, verifies that scc detects symlink loops, and skips instead of running indefinitely (Michael) this is a recommit, the other commit by Richard, possibly might have overwritten it? overall very weird that both our code dissapered * Added an if-statement to TestNewFileJobBrokenSymLink to skip the test on Windows machines, because Windows requires administrator mode or developer mode to be enabled to create symlinks. * Added test to check for duplicate file counting via symlinks. The function TestNewFileJobDuplicateCounting checks that the same file is not processed twice when accessed through multiple paths with symLink. This should skip duplicate files and prevents double counting. * Updated test functions: Used Brokensymlink window check, made an check for fist link created in my test, and used BrokenSymlink check window check in the Duplicate test --------- Co-authored-by: Rayze-B <vrajpatel0615@gmail.com> Co-authored-by: m39johnsonm <m39johnsonm@student.bridgew.edu>
1 parent e7ae63a commit 75de4a8

2 files changed

Lines changed: 125 additions & 0 deletions

File tree

processor/file.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import (
1313
// needs to be sync.Map as it potentially could be called by many GoRoutines
1414
var extensionCache sync.Map
1515

16+
// Added as a way to track files per run.
17+
var visitedPaths sync.Map
18+
1619
// A custom version of extracting extensions for a file
1720
// which also has a case-insensitive cache in order to save
1821
// some needless processing
@@ -78,6 +81,19 @@ func newFileJob(path, name string, fileInfo os.FileInfo) *FileJob {
7881
return nil
7982
}
8083

84+
// This determines the real path
85+
realPath := path
86+
if symPath != "" {
87+
realPath = symPath
88+
}
89+
90+
// Prevent duplicate processing and loops
91+
if _, exists := visitedPaths.Load(realPath); exists {
92+
printWarnF("skipping already processed file: %s", realPath)
93+
return nil
94+
}
95+
visitedPaths.Store(realPath, true)
96+
8197
language, extension := DetectLanguage(name)
8298

8399
if len(language) != 0 {

processor/file_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ package processor
55
import (
66
"math/rand/v2"
77
"os"
8+
"path/filepath"
9+
"runtime"
10+
"sync"
811
"testing"
912
)
1013

@@ -183,6 +186,38 @@ func TestNewFileJobSize(t *testing.T) {
183186
LargeByteCount = 1000000
184187
}
185188

189+
func TestNewFileJobBrokenSymlink(t *testing.T) {
190+
if runtime.GOOS == "windows" {
191+
t.Skip("skipping symlink test on Windows due to privilege requirements")
192+
}
193+
194+
ProcessConstants()
195+
IncludeSymLinks = true
196+
197+
// Create a temp directory to work in
198+
dir, err := os.MkdirTemp("", "scc-broken-symlink-test")
199+
if err != nil {
200+
t.Fatal("Failed to create temp dir:", err)
201+
}
202+
defer os.RemoveAll(dir)
203+
204+
// Create a symlink that points to a path that doesn't exist
205+
symPath := dir + "/broken.go"
206+
err = os.Symlink("/this/path/does/not/exist.go", symPath)
207+
if err != nil {
208+
t.Fatal("Failed to create broken symlink:", err)
209+
}
210+
211+
fi, _ := os.Lstat(symPath)
212+
job := newFileJob(symPath, "broken.go", fi)
213+
214+
if job != nil {
215+
t.Error("Expected nil for broken symlink got", job)
216+
}
217+
218+
IncludeSymLinks = false
219+
}
220+
186221
func BenchmarkGetExtensionDifferent(b *testing.B) {
187222
for i := 0; i < b.N; i++ {
188223

@@ -211,3 +246,77 @@ func randStringBytes(n int) string {
211246
}
212247
return string(b)
213248
}
249+
250+
func TestNewFileJobCircularSymlink(t *testing.T) {
251+
if runtime.GOOS == "windows" {
252+
t.Skip("skipping symlink test on Windows due to privilege requirements")
253+
}
254+
ProcessConstants()
255+
IncludeSymLinks = true
256+
defer func() { IncludeSymLinks = false }()
257+
visitedPaths = sync.Map{}
258+
// Create a temp directory to work in
259+
dir := t.TempDir()
260+
link1 := filepath.Join(dir, "link1.go")
261+
link2 := filepath.Join(dir, "link2.go")
262+
// Create a loop: link1 -> link2 and link2 -> link1
263+
if err := os.Symlink(link2, link1); err != nil {
264+
t.Fatal("Failed to create first link:", err)
265+
}
266+
if err := os.Symlink(link1, link2); err != nil {
267+
t.Fatal("Failed to create circular link:", err)
268+
}
269+
270+
fi, err := os.Lstat(link1)
271+
if err != nil {
272+
t.Fatal(err)
273+
}
274+
// It should return the 'too many links' error.
275+
job := newFileJob(link1, "link1.go", fi)
276+
277+
if job != nil {
278+
t.Error("Expected nil for circular symlink, but got a FileJob")
279+
}
280+
}
281+
282+
func TestNewFileJobDuplicateCounting(t *testing.T) {
283+
if runtime.GOOS == "windows" {
284+
t.Skip("skipping symlink test on Windows due to privilege requirements")
285+
}
286+
ProcessConstants()
287+
IncludeSymLinks = true
288+
defer func() { IncludeSymLinks = false }()
289+
visitedPaths = sync.Map{}
290+
291+
// Create Temp directory
292+
dir := t.TempDir()
293+
// Create a test file
294+
testFile := filepath.Join(dir, "file.go")
295+
296+
if err := os.WriteFile(testFile, []byte("package main"), 0644); err != nil {
297+
t.Fatal(err)
298+
}
299+
300+
// Create a symlink to the same file
301+
linkFile := filepath.Join(dir, "link.go")
302+
if err := os.Symlink(testFile, linkFile); err != nil {
303+
t.Skip("Symlinks not supported:", err)
304+
}
305+
// Process the test file
306+
fi1, _ := os.Lstat(testFile)
307+
job1 := newFileJob(testFile, "file.go", fi1)
308+
309+
// Process the symlink (same target)
310+
fi2, _ := os.Lstat(linkFile)
311+
job2 := newFileJob(linkFile, "link.go", fi2)
312+
313+
// First count should go through
314+
if job1 == nil {
315+
t.Fatal("Expected first file job to be created")
316+
}
317+
318+
// Second count should be skipped
319+
if job2 != nil {
320+
t.Error("Expected nil for duplicate file through symlink, but got a FileJob")
321+
}
322+
}

0 commit comments

Comments
 (0)