diff --git a/checks/fileparser/listing.go b/checks/fileparser/listing.go index 3efb8208991..82f0c20ef7b 100644 --- a/checks/fileparser/listing.go +++ b/checks/fileparser/listing.go @@ -17,6 +17,7 @@ package fileparser import ( "bufio" "fmt" + "io" "path" "strings" @@ -94,9 +95,14 @@ func OnMatchingFileContentDo(repoClient clients.RepoClient, matchPathTo PathMatc } for _, file := range matchedFiles { - content, err := repoClient.GetFileContent(file) + rc, err := repoClient.GetFileReader(file) if err != nil { - return fmt.Errorf("error during GetFileContent: %w", err) + return fmt.Errorf("error during GetFileReader: %w", err) + } + content, err := io.ReadAll(rc) + rc.Close() + if err != nil { + return fmt.Errorf("reading from file: %w", err) } continueIter, err := onFileContent(file, content, args...) diff --git a/checks/fileparser/listing_test.go b/checks/fileparser/listing_test.go index 1b09c921fd7..be9ef0c0294 100644 --- a/checks/fileparser/listing_test.go +++ b/checks/fileparser/listing_test.go @@ -16,6 +16,8 @@ package fileparser import ( "errors" + "io" + "strings" "testing" "github.com/golang/mock/gomock" @@ -526,7 +528,7 @@ func TestOnMatchingFileContent(t *testing.T) { ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).Return(nil, nil).AnyTimes() + mockRepo.EXPECT().GetFileReader(gomock.Any()).Return(io.NopCloser(strings.NewReader("")), nil).AnyTimes() result := OnMatchingFileContentDo(mockRepo, PathMatcher{ Pattern: tt.shellPattern, diff --git a/checks/fuzzing_test.go b/checks/fuzzing_test.go index 852b0172ff4..e543bcb8d14 100644 --- a/checks/fuzzing_test.go +++ b/checks/fuzzing_test.go @@ -16,6 +16,8 @@ package checks import ( "errors" + "io" + "strings" "testing" "github.com/golang/mock/gomock" @@ -144,11 +146,12 @@ func TestFuzzing(t *testing.T) { }).AnyTimes() mockFuzz.EXPECT().ListProgrammingLanguages().Return(tt.langs, nil).AnyTimes() mockFuzz.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockFuzz.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) (string, error) { + mockFuzz.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { - return "", errors.New("error") + return nil, errors.New("error") } - return tt.fileContent, nil + rc := io.NopCloser(strings.NewReader(tt.fileContent)) + return rc, nil }).AnyTimes() dl := scut.TestDetailLogger{} raw := checker.RawResults{} diff --git a/checks/permissions_test.go b/checks/permissions_test.go index 2d80ef33f1c..16c7b9bc4ab 100644 --- a/checks/permissions_test.go +++ b/checks/permissions_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "strings" "testing" @@ -443,12 +443,8 @@ func TestGithubTokenPermissions(t *testing.T) { } return files, nil }).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { - content, err := os.ReadFile("./testdata/" + fn) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { + return os.Open("./testdata/" + fn) }).AnyTimes() dl := scut.TestDetailLogger{} c := checker.CheckRequest{ @@ -499,11 +495,6 @@ func TestGithubTokenPermissionsLineNumber(t *testing.T) { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { t.Parallel() - content, err := os.ReadFile(tt.filename) - if err != nil { - t.Errorf("cannot read file: %v", err) - } - p := strings.Replace(tt.filename, "./testdata/", "", 1) ctrl := gomock.NewController(t) mockRepo := mockrepo.NewMockRepoClient(ctrl) @@ -514,8 +505,8 @@ func TestGithubTokenPermissionsLineNumber(t *testing.T) { mockRepo.EXPECT().ListFiles(gomock.Any()).DoAndReturn(func(predicate func(string) (bool, error)) ([]string, error) { return []string{p}, nil }).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { - return content, nil + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { + return os.Open(tt.filename) }).AnyTimes() dl := scut.TestDetailLogger{} c := checker.CheckRequest{ diff --git a/checks/pinned_dependencies_test.go b/checks/pinned_dependencies_test.go index b22fa7fb849..ea22bdd7302 100644 --- a/checks/pinned_dependencies_test.go +++ b/checks/pinned_dependencies_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "testing" @@ -58,15 +58,11 @@ func TestPinningDependencies(t *testing.T) { mockRepo.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { return nil, nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/raw/binary_artifact_test.go b/checks/raw/binary_artifact_test.go index 590e86a7833..8b956ab3d41 100644 --- a/checks/raw/binary_artifact_test.go +++ b/checks/raw/binary_artifact_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "testing" @@ -227,13 +227,8 @@ func TestBinaryArtifacts(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil) } for i := 0; i < tt.getFileContentCount; i++ { - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(file) }) } if tt.successfulWorkflowRuns != nil { @@ -276,19 +271,11 @@ func TestBinaryArtifacts_workflow_runs_unsupported(t *testing.T) { const verifyWorkflow = ".github/workflows/verify.yaml" files := []string{jarFile, verifyWorkflow} mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return(files, nil).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(jarFile).DoAndReturn(func(file string) ([]byte, error) { - content, err := os.ReadFile("../testdata/binaryartifacts/jars/gradle-wrapper.jar") - if err != nil { - return nil, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(jarFile).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/binaryartifacts/jars/gradle-wrapper.jar") }).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(verifyWorkflow).DoAndReturn(func(file string) ([]byte, error) { - content, err := os.ReadFile("../testdata/binaryartifacts/workflows/verify.yaml") - if err != nil { - return nil, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(verifyWorkflow).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/binaryartifacts/workflows/verify.yaml") }).AnyTimes() mockRepoClient.EXPECT().ListSuccessfulWorkflowRuns(gomock.Any()).Return(nil, clients.ErrUnsupportedFeature).AnyTimes() diff --git a/checks/raw/dangerous_workflow_test.go b/checks/raw/dangerous_workflow_test.go index 787f37f3311..753dbab2b92 100644 --- a/checks/raw/dangerous_workflow_test.go +++ b/checks/raw/dangerous_workflow_test.go @@ -17,7 +17,7 @@ package raw import ( "context" "errors" - "fmt" + "io" "os" "testing" @@ -159,13 +159,8 @@ func TestGithubDangerousWorkflow(t *testing.T) { ctrl := gomock.NewController(t) mockRepoClient := mockrepo.NewMockRepoClient(ctrl) mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil) - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile("../testdata/" + file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("../testdata/" + file) }) req := &checker.CheckRequest{ diff --git a/checks/raw/fuzzing_test.go b/checks/raw/fuzzing_test.go index e409b401447..18d669740d5 100644 --- a/checks/raw/fuzzing_test.go +++ b/checks/raw/fuzzing_test.go @@ -16,8 +16,10 @@ package raw import ( "errors" + "io" "path" "regexp" + "strings" "testing" "github.com/golang/mock/gomock" @@ -135,11 +137,11 @@ func Test_checkCFLite(t *testing.T) { defer ctrl.Finish() mockFuzz := mockrepo.NewMockRepoClient(ctrl) mockFuzz.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockFuzz.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) (string, error) { + mockFuzz.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { - return "", errors.New("error") + return nil, errors.New("error") } - return tt.fileContent, nil + return io.NopCloser(strings.NewReader(tt.fileContent)), nil }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockFuzz, @@ -486,11 +488,11 @@ func Test_checkFuzzFunc(t *testing.T) { defer ctrl.Finish() mockClient := mockrepo.NewMockRepoClient(ctrl) mockClient.EXPECT().ListFiles(gomock.Any()).Return(tt.fileName, nil).AnyTimes() - mockClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(f string) ([]byte, error) { + mockClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(f string) (io.ReadCloser, error) { if tt.wantErr { return nil, errors.New("error") } - return []byte(tt.fileContent), nil + return io.NopCloser(strings.NewReader(tt.fileContent)), nil }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockClient, diff --git a/checks/raw/github/packaging.go b/checks/raw/github/packaging.go index 1769dcec7ec..291fdf36172 100644 --- a/checks/raw/github/packaging.go +++ b/checks/raw/github/packaging.go @@ -16,6 +16,7 @@ package github import ( "fmt" + "io" "path/filepath" "github.com/rhysd/actionlint" @@ -37,9 +38,14 @@ func Packaging(c *checker.CheckRequest) (checker.PackagingData, error) { } for _, fp := range matchedFiles { - fc, err := c.RepoClient.GetFileContent(fp) + fr, err := c.RepoClient.GetFileReader(fp) if err != nil { - return data, fmt.Errorf("RepoClient.GetFileContent: %w", err) + return data, fmt.Errorf("RepoClient.GetFileReader: %w", err) + } + fc, err := io.ReadAll(fr) + fr.Close() + if err != nil { + return data, fmt.Errorf("reading file: %w", err) } workflow, errs := actionlint.Parse(fc) diff --git a/checks/raw/gitlab/packaging.go b/checks/raw/gitlab/packaging.go index d6d9277fd2c..72de346f73d 100644 --- a/checks/raw/gitlab/packaging.go +++ b/checks/raw/gitlab/packaging.go @@ -16,6 +16,7 @@ package gitlab import ( "fmt" + "io" "strings" "github.com/ossf/scorecard/v4/checker" @@ -32,9 +33,14 @@ func Packaging(c *checker.CheckRequest) (checker.PackagingData, error) { } for _, fp := range matchedFiles { - fc, err := c.RepoClient.GetFileContent(fp) + fr, err := c.RepoClient.GetFileReader(fp) if err != nil { - return data, fmt.Errorf("RepoClient.GetFileContent: %w", err) + return data, fmt.Errorf("RepoClient.GetFileReader: %w", err) + } + fc, err := io.ReadAll(fr) + fr.Close() + if err != nil { + return data, fmt.Errorf("reading from file: %w", err) } file, found := isGitlabPackagingWorkflow(fc, fp) diff --git a/checks/raw/gitlab/packaging_test.go b/checks/raw/gitlab/packaging_test.go index 1e66f045200..ffd3eea47c5 100644 --- a/checks/raw/gitlab/packaging_test.go +++ b/checks/raw/gitlab/packaging_test.go @@ -15,6 +15,7 @@ package gitlab import ( + "io" "os" "testing" @@ -134,10 +135,9 @@ func TestGitlabPackagingPackager(t *testing.T) { moqRepoClient.EXPECT().ListFiles(gomock.Any()). Return([]string{tt.filename}, nil).AnyTimes() - moqRepoClient.EXPECT().GetFileContent(tt.filename). - DoAndReturn(func(b string) ([]byte, error) { - content, err := os.ReadFile(b) - return content, err + moqRepoClient.EXPECT().GetFileReader(tt.filename). + DoAndReturn(func(b string) (io.ReadCloser, error) { + return os.Open(b) }).AnyTimes() if tt.exists { diff --git a/checks/raw/pinned_dependencies_test.go b/checks/raw/pinned_dependencies_test.go index 82b0588511c..305c075a600 100644 --- a/checks/raw/pinned_dependencies_test.go +++ b/checks/raw/pinned_dependencies_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "path/filepath" "strings" @@ -1895,13 +1895,8 @@ func TestCollectDockerfilePinning(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil).AnyTimes() mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() mockRepoClient.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(file) }) req := checker.CheckRequest{ @@ -1994,13 +1989,8 @@ func TestCollectGitHubActionsWorkflowPinning(t *testing.T) { mockRepoClient.EXPECT().ListFiles(gomock.Any()).Return([]string{tt.filename}, nil).AnyTimes() mockRepoClient.EXPECT().GetDefaultBranchName().Return("main", nil).AnyTimes() mockRepoClient.EXPECT().URI().Return("github.com/ossf/scorecard").AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile(filepath.Join("testdata", file)) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open(filepath.Join("testdata", file)) }) req := checker.CheckRequest{ diff --git a/checks/raw/sast_test.go b/checks/raw/sast_test.go index 8bdf4020463..d724ec4ddfc 100644 --- a/checks/raw/sast_test.go +++ b/checks/raw/sast_test.go @@ -15,7 +15,7 @@ package raw import ( - "fmt" + "io" "os" "testing" @@ -207,13 +207,8 @@ func TestSAST(t *testing.T) { mockRepoClient.EXPECT().ListCommits().DoAndReturn(func() ([]clients.Commit, error) { return tt.commits, nil }) - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(file string) ([]byte, error) { - // This will read the file and return the content - content, err := os.ReadFile("./testdata/" + file) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(file string) (io.ReadCloser, error) { + return os.Open("./testdata/" + file) }).AnyTimes() req := checker.CheckRequest{ RepoClient: mockRepoClient, diff --git a/checks/raw/security_policy_test.go b/checks/raw/security_policy_test.go index d311d85e283..579b9576d73 100644 --- a/checks/raw/security_policy_test.go +++ b/checks/raw/security_policy_test.go @@ -15,8 +15,9 @@ package raw import ( - "fmt" + "io" "os" + "strings" "testing" "github.com/golang/mock/gomock" @@ -142,18 +143,14 @@ func TestSecurityPolicy(t *testing.T) { // file contents once found. This test will return that // mock file, but this specific unit test is not testing // for content. As such, this test will crash without - // a mock GetFileContent, so this will return no content + // a mock GetFileReader, so this will return no content // for the existing file. content test are in overall check // - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { - return nil, nil + return io.NopCloser(strings.NewReader("")), nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/sast_test.go b/checks/sast_test.go index 01da79ae43b..16b879b24bf 100644 --- a/checks/sast_test.go +++ b/checks/sast_test.go @@ -17,7 +17,7 @@ package checks import ( "context" "errors" - "fmt" + "io" "os" "strings" "testing" @@ -320,15 +320,11 @@ func Test_SAST(t *testing.T) { } return []string{tt.path}, nil }).AnyTimes() - mockRepoClient.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepoClient.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { - return nil, nil + return io.NopCloser(strings.NewReader("")), nil } - content, err := os.ReadFile("./testdata/" + tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open("./testdata/" + tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/checks/security_policy_test.go b/checks/security_policy_test.go index 1517c780589..4d74406927b 100644 --- a/checks/security_policy_test.go +++ b/checks/security_policy_test.go @@ -15,7 +15,7 @@ package checks import ( - "fmt" + "io" "os" "testing" @@ -178,15 +178,11 @@ func TestSecurityPolicy(t *testing.T) { mockRepo.EXPECT().ListFiles(gomock.Any()).Return(tt.files, nil).AnyTimes() - mockRepo.EXPECT().GetFileContent(gomock.Any()).DoAndReturn(func(fn string) ([]byte, error) { + mockRepo.EXPECT().GetFileReader(gomock.Any()).DoAndReturn(func(fn string) (io.ReadCloser, error) { if tt.path == "" { return nil, nil } - content, err := os.ReadFile(tt.path) - if err != nil { - return content, fmt.Errorf("%w", err) - } - return content, nil + return os.Open(tt.path) }).AnyTimes() dl := scut.TestDetailLogger{} diff --git a/clients/git/client.go b/clients/git/client.go index fa6d2e3fea0..f530cc7c82a 100644 --- a/clients/git/client.go +++ b/clients/git/client.go @@ -42,6 +42,9 @@ var ( errNilCommitFound = errors.New("nil commit found") errEmptyQuery = errors.New("query is empty") errDefaultBranch = errors.New("default branch name could not be determined") + + // ensure Client implements clients.RepoClient. + _ clients.RepoClient = (*Client)(nil) ) type Client struct { @@ -235,17 +238,17 @@ func (c *Client) ListFiles(predicate func(string) (bool, error)) ([]string, erro return files, nil } -func (c *Client) GetFileContent(filename string) ([]byte, error) { +func (c *Client) GetFileReader(filename string) (io.ReadCloser, error) { // Create the full path of the file fullPath := filepath.Join(c.tempDir, filename) // Read the file - content, err := os.ReadFile(fullPath) + f, err := os.Open(fullPath) if err != nil { - return nil, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("os.Open: %w", err) } - return content, nil + return f, nil } func (c *Client) IsArchived() (bool, error) { diff --git a/clients/githubrepo/client.go b/clients/githubrepo/client.go index dbef2cf9134..dc5dba42af7 100644 --- a/clients/githubrepo/client.go +++ b/clients/githubrepo/client.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "io" "net/http" "os" "strings" @@ -147,9 +148,9 @@ func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, return client.tarball.listFiles(predicate) } -// GetFileContent implements RepoClient.GetFileContent. -func (client *Client) GetFileContent(filename string) ([]byte, error) { - return client.tarball.getFileContent(filename) +// GetFileReader implements RepoClient.GetFileReader. +func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { + return client.tarball.getFile(filename) } // ListCommits implements RepoClient.ListCommits. diff --git a/clients/githubrepo/tarball.go b/clients/githubrepo/tarball.go index 8888e2f26b6..ae5455ce6e7 100644 --- a/clients/githubrepo/tarball.go +++ b/clients/githubrepo/tarball.go @@ -260,15 +260,15 @@ func (handler *tarballHandler) getLocalPath() (string, error) { return absTempDir, nil } -func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) { +func (handler *tarballHandler) getFile(filename string) (*os.File, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during tarballHandler.setup: %w", err) } - content, err := os.ReadFile(filepath.Join(handler.tempDir, filename)) + f, err := os.Open(filepath.Join(handler.tempDir, filename)) if err != nil { - return content, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } func (handler *tarballHandler) cleanup() error { diff --git a/clients/githubrepo/tarball_test.go b/clients/githubrepo/tarball_test.go index 106877dbdf7..89f570484bc 100644 --- a/clients/githubrepo/tarball_test.go +++ b/clients/githubrepo/tarball_test.go @@ -161,12 +161,18 @@ func TestExtractTarball(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := handler.getFileContent(getcontenttest.filename) + f, err := handler.getFile(getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if getcontenttest.err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } diff --git a/clients/gitlabrepo/client.go b/clients/gitlabrepo/client.go index 68644e241e8..a53588cf3ac 100644 --- a/clients/gitlabrepo/client.go +++ b/clients/gitlabrepo/client.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "io" "log" "os" "time" @@ -173,8 +174,8 @@ func (client *Client) ListFiles(predicate func(string) (bool, error)) ([]string, return client.tarball.listFiles(predicate) } -func (client *Client) GetFileContent(filename string) ([]byte, error) { - return client.tarball.getFileContent(filename) +func (client *Client) GetFileReader(filename string) (io.ReadCloser, error) { + return client.tarball.getFile(filename) } func (client *Client) ListCommits() ([]clients.Commit, error) { diff --git a/clients/gitlabrepo/tarball.go b/clients/gitlabrepo/tarball.go index be0b42ffcaf..75ef533f231 100644 --- a/clients/gitlabrepo/tarball.go +++ b/clients/gitlabrepo/tarball.go @@ -292,15 +292,15 @@ func (handler *tarballHandler) listFiles(predicate func(string) (bool, error)) ( return ret, nil } -func (handler *tarballHandler) getFileContent(filename string) ([]byte, error) { +func (handler *tarballHandler) getFile(filename string) (*os.File, error) { if err := handler.setup(); err != nil { return nil, fmt.Errorf("error during tarballHandler.setup: %w", err) } - content, err := os.ReadFile(filepath.Join(handler.tempDir, filename)) + f, err := os.Open(filepath.Join(handler.tempDir, filename)) if err != nil { - return content, fmt.Errorf("os.ReadFile: %w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } func (handler *tarballHandler) cleanup() error { diff --git a/clients/gitlabrepo/tarball_e2e_test.go b/clients/gitlabrepo/tarball_e2e_test.go index 38f938506d6..fa3d7b5a7cc 100644 --- a/clients/gitlabrepo/tarball_e2e_test.go +++ b/clients/gitlabrepo/tarball_e2e_test.go @@ -16,6 +16,7 @@ package gitlabrepo import ( "context" + "io" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -39,7 +40,10 @@ var _ = Describe("E2E TEST: gitlabrepo.ListFiles", func() { Expect(err).Should(BeNil()) Expect(len(files)).ShouldNot(BeZero()) - data, err := client.GetFileContent("README.md") + r, err := client.GetFileReader("README.md") + Expect(err).Should(BeNil()) + defer r.Close() + data, err := io.ReadAll(r) Expect(err).Should(BeNil()) Expect(len(data)).ShouldNot(BeZero()) }) diff --git a/clients/gitlabrepo/tarball_test.go b/clients/gitlabrepo/tarball_test.go index 75ad34e87e6..3bbbc18928b 100644 --- a/clients/gitlabrepo/tarball_test.go +++ b/clients/gitlabrepo/tarball_test.go @@ -153,12 +153,18 @@ func TestExtractTarball(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := handler.getFileContent(getcontenttest.filename) + f, err := handler.getFile(getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if getcontenttest.err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } diff --git a/clients/localdir/client.go b/clients/localdir/client.go index 45cf09924ae..2038108d1d2 100644 --- a/clients/localdir/client.go +++ b/clients/localdir/client.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "os" "path" @@ -159,19 +160,19 @@ func (client *localDirClient) ListFiles(predicate func(string) (bool, error)) ([ return applyPredicate(client.files, client.errFiles, predicate) } -func getFileContent(clientpath, filename string) ([]byte, error) { +func getFile(clientpath, filename string) (*os.File, error) { // Note: the filenames do not contain the original path - see ListFiles(). fn := path.Join(clientpath, filename) - content, err := os.ReadFile(fn) + f, err := os.Open(fn) if err != nil { - return content, fmt.Errorf("%w", err) + return nil, fmt.Errorf("open file: %w", err) } - return content, nil + return f, nil } -// GetFileContent implements RepoClient.GetFileContent. -func (client *localDirClient) GetFileContent(filename string) ([]byte, error) { - return getFileContent(client.path, filename) +// GetFileReader implements RepoClient.GetFileReader. +func (client *localDirClient) GetFileReader(filename string) (io.ReadCloser, error) { + return getFile(client.path, filename) } // GetBranch implements RepoClient.GetBranch. diff --git a/clients/localdir/client_test.go b/clients/localdir/client_test.go index c11db84c296..6d8964eca21 100644 --- a/clients/localdir/client_test.go +++ b/clients/localdir/client_test.go @@ -17,6 +17,7 @@ package localdir import ( "context" "errors" + "io" "os" "strings" "testing" @@ -119,6 +120,7 @@ func isSortedString(x, y string) bool { return x < y } +//nolint:gocognit func TestClient_GetFileListAndContent(t *testing.T) { t.Parallel() testcases := []struct { @@ -189,12 +191,18 @@ func TestClient_GetFileListAndContent(t *testing.T) { // Test GetFileContent API. for _, getcontenttest := range testcase.getcontentTests { - content, err := getFileContent(testcase.inputFolder, getcontenttest.filename) + f, err := getFile(testcase.inputFolder, getcontenttest.filename) if getcontenttest.err != nil && !errors.Is(err, getcontenttest.err) { t.Errorf("test failed: expected - %v, got - %v", getcontenttest.err, err) } - if getcontenttest.err == nil && !cmp.Equal(getcontenttest.output, content) { - t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + if err == nil { + content, err := io.ReadAll(f) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cmp.Equal(getcontenttest.output, content) { + t.Errorf("test failed: expected - %s, got - %s", string(getcontenttest.output), string(content)) + } } } }) diff --git a/clients/mockclients/repo_client.go b/clients/mockclients/repo_client.go index deb2af7ff29..6ad96ba1095 100644 --- a/clients/mockclients/repo_client.go +++ b/clients/mockclients/repo_client.go @@ -21,6 +21,7 @@ package mockrepo import ( context "context" + io "io" reflect "reflect" time "time" @@ -125,19 +126,19 @@ func (mr *MockRepoClientMockRecorder) GetDefaultBranchName() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultBranchName", reflect.TypeOf((*MockRepoClient)(nil).GetDefaultBranchName)) } -// GetFileContent mocks base method. -func (m *MockRepoClient) GetFileContent(filename string) ([]byte, error) { +// GetFileReader mocks base method. +func (m *MockRepoClient) GetFileReader(filename string) (io.ReadCloser, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetFileContent", filename) - ret0, _ := ret[0].([]byte) + ret := m.ctrl.Call(m, "GetFileReader", filename) + ret0, _ := ret[0].(io.ReadCloser) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetFileContent indicates an expected call of GetFileContent. -func (mr *MockRepoClientMockRecorder) GetFileContent(filename interface{}) *gomock.Call { +// GetFileReader indicates an expected call of GetFileReader. +func (mr *MockRepoClientMockRecorder) GetFileReader(filename interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFileContent", reflect.TypeOf((*MockRepoClient)(nil).GetFileContent), filename) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFileReader", reflect.TypeOf((*MockRepoClient)(nil).GetFileReader), filename) } // GetOrgRepoClient mocks base method. diff --git a/clients/ossfuzz/client.go b/clients/ossfuzz/client.go index 8c19d8038e6..2439244737f 100644 --- a/clients/ossfuzz/client.go +++ b/clients/ossfuzz/client.go @@ -182,9 +182,9 @@ func (c *client) ListFiles(predicate func(string) (bool, error)) ([]string, erro return nil, fmt.Errorf("ListFiles: %w", clients.ErrUnsupportedFeature) } -// GetFileContent implements RepoClient.GetFileContent. -func (c *client) GetFileContent(filename string) ([]byte, error) { - return nil, fmt.Errorf("GetFileContent: %w", clients.ErrUnsupportedFeature) +// GetFileReader implements RepoClient.GetFileReader. +func (c *client) GetFileReader(filename string) (io.ReadCloser, error) { + return nil, fmt.Errorf("GetFileReader: %w", clients.ErrUnsupportedFeature) } // GetBranch implements RepoClient.GetBranch. diff --git a/clients/ossfuzz/client_test.go b/clients/ossfuzz/client_test.go index fd0ad92a612..3900526170e 100644 --- a/clients/ossfuzz/client_test.go +++ b/clients/ossfuzz/client_test.go @@ -235,9 +235,9 @@ func TestAllClientMethods(t *testing.T) { // Test GetFileContent { - _, err := c.GetFileContent("") + _, err := c.GetFileReader("") if !errors.Is(err, clients.ErrUnsupportedFeature) { - t.Errorf("GetFileContent: Expected %v, but got %v", clients.ErrUnsupportedFeature, err) + t.Errorf("GetFileReader: Expected %v, but got %v", clients.ErrUnsupportedFeature, err) } } diff --git a/clients/repo_client.go b/clients/repo_client.go index a5804b3f7d7..d51661e9976 100644 --- a/clients/repo_client.go +++ b/clients/repo_client.go @@ -18,6 +18,7 @@ package clients import ( "context" "errors" + "io" "time" ) @@ -36,7 +37,9 @@ type RepoClient interface { // Returns an absolute path to the local repository // in the format that matches the local OS LocalPath() (string, error) - GetFileContent(filename string) ([]byte, error) + // GetFileReader returns an io.ReadCloser corresponding to the desired file. + // Callers should ensure to Close the Reader when finished. + GetFileReader(filename string) (io.ReadCloser, error) GetBranch(branch string) (*BranchRef, error) GetCreatedAt() (time.Time, error) GetDefaultBranchName() (string, error)