From 0cd0f763dd8e79f247086d2718f636f139b63360 Mon Sep 17 00:00:00 2001 From: Dana Sherson Date: Fri, 5 May 2023 11:19:00 +1200 Subject: [PATCH] fix: handle cyclical `-r`s in `requirements.txt` Ported from G-Rath/osv-detector#191 Note that this is not actually supported by `pip` itself, but doing so actually optimizes the parser a bit anyway by only reading each file exactly once regardless of how often it is required fixes: #354 Co-authored-by: Gareth Jones --- .../fixtures/pip/cyclic-r-complex-1.txt | 3 + .../fixtures/pip/cyclic-r-complex-2.txt | 4 + .../fixtures/pip/cyclic-r-complex-3.txt | 4 + pkg/lockfile/fixtures/pip/cyclic-r-self.txt | 4 + .../fixtures/pip/duplicate-r-base.txt | 1 + pkg/lockfile/fixtures/pip/duplicate-r-dev.txt | 5 + .../fixtures/pip/duplicate-r-test.txt | 4 + pkg/lockfile/parse-requirements-txt.go | 17 +++- pkg/lockfile/parse-requirements-txt_test.go | 91 +++++++++++++++++++ 9 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 pkg/lockfile/fixtures/pip/cyclic-r-complex-1.txt create mode 100644 pkg/lockfile/fixtures/pip/cyclic-r-complex-2.txt create mode 100644 pkg/lockfile/fixtures/pip/cyclic-r-complex-3.txt create mode 100644 pkg/lockfile/fixtures/pip/cyclic-r-self.txt create mode 100644 pkg/lockfile/fixtures/pip/duplicate-r-base.txt create mode 100644 pkg/lockfile/fixtures/pip/duplicate-r-dev.txt create mode 100644 pkg/lockfile/fixtures/pip/duplicate-r-test.txt diff --git a/pkg/lockfile/fixtures/pip/cyclic-r-complex-1.txt b/pkg/lockfile/fixtures/pip/cyclic-r-complex-1.txt new file mode 100644 index 00000000000..b0a64dd81a0 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/cyclic-r-complex-1.txt @@ -0,0 +1,3 @@ +-r ./cyclic-r-complex-2.txt + +cyclic-r-complex==1 diff --git a/pkg/lockfile/fixtures/pip/cyclic-r-complex-2.txt b/pkg/lockfile/fixtures/pip/cyclic-r-complex-2.txt new file mode 100644 index 00000000000..22a5685fb57 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/cyclic-r-complex-2.txt @@ -0,0 +1,4 @@ +-r ./../pip/cyclic-r-complex-1.txt +-r ./cyclic-r-complex-3.txt + +cyclic-r-complex==2 diff --git a/pkg/lockfile/fixtures/pip/cyclic-r-complex-3.txt b/pkg/lockfile/fixtures/pip/cyclic-r-complex-3.txt new file mode 100644 index 00000000000..da9fa11c910 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/cyclic-r-complex-3.txt @@ -0,0 +1,4 @@ +-r ./cyclic-r-complex-1.txt +-r ./cyclic-r-complex-2.txt + +cyclic-r-complex==3 diff --git a/pkg/lockfile/fixtures/pip/cyclic-r-self.txt b/pkg/lockfile/fixtures/pip/cyclic-r-self.txt new file mode 100644 index 00000000000..99af87f1922 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/cyclic-r-self.txt @@ -0,0 +1,4 @@ +-r ./cyclic-r-self.txt + +requests==1.2.3 +pandas==0.23.4 diff --git a/pkg/lockfile/fixtures/pip/duplicate-r-base.txt b/pkg/lockfile/fixtures/pip/duplicate-r-base.txt new file mode 100644 index 00000000000..123602fd699 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/duplicate-r-base.txt @@ -0,0 +1 @@ +django==0.1.0 diff --git a/pkg/lockfile/fixtures/pip/duplicate-r-dev.txt b/pkg/lockfile/fixtures/pip/duplicate-r-dev.txt new file mode 100644 index 00000000000..44645b8d458 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/duplicate-r-dev.txt @@ -0,0 +1,5 @@ +-r ./duplicate-r-base.txt +-r ./duplicate-r-test.txt + +pandas==0.23.4 +requests==1.2.3 diff --git a/pkg/lockfile/fixtures/pip/duplicate-r-test.txt b/pkg/lockfile/fixtures/pip/duplicate-r-test.txt new file mode 100644 index 00000000000..13d13c65846 --- /dev/null +++ b/pkg/lockfile/fixtures/pip/duplicate-r-test.txt @@ -0,0 +1,4 @@ +-r ./duplicate-r-base.txt + +requests==1.2.3 +unittest==1.0.0 diff --git a/pkg/lockfile/parse-requirements-txt.go b/pkg/lockfile/parse-requirements-txt.go index 4f46a0b3e4c..d7564a436f0 100644 --- a/pkg/lockfile/parse-requirements-txt.go +++ b/pkg/lockfile/parse-requirements-txt.go @@ -94,6 +94,9 @@ func isNotRequirementLine(line string) bool { } func ParseRequirementsTxt(pathToLockfile string) ([]PackageDetails, error) { + return parseRequirementsTxt(pathToLockfile, map[string]struct{}{}) +} +func parseRequirementsTxt(pathToLockfile string, requiredAlready map[string]struct{}) ([]PackageDetails, error) { packages := map[string]PackageDetails{} file, err := os.Open(pathToLockfile) @@ -107,10 +110,16 @@ func ParseRequirementsTxt(pathToLockfile string) ([]PackageDetails, error) { for scanner.Scan() { line := removeComments(scanner.Text()) - if strings.HasPrefix(line, "-r ") { - details, err := ParseRequirementsTxt( - filepath.Join(filepath.Dir(pathToLockfile), strings.TrimPrefix(line, "-r ")), - ) + if ar := strings.TrimPrefix(line, "-r "); ar != line { + ar = filepath.Join(filepath.Dir(pathToLockfile), ar) + + if _, ok := requiredAlready[ar]; ok { + continue + } + + requiredAlready[ar] = struct{}{} + + details, err := parseRequirementsTxt(ar, requiredAlready) if err != nil { return []PackageDetails{}, fmt.Errorf("failed to include %s: %w", line, err) diff --git a/pkg/lockfile/parse-requirements-txt_test.go b/pkg/lockfile/parse-requirements-txt_test.go index 767c9a82d61..845bb15db24 100644 --- a/pkg/lockfile/parse-requirements-txt_test.go +++ b/pkg/lockfile/parse-requirements-txt_test.go @@ -432,3 +432,94 @@ func TestParseRequirementsTxt_WithBadROption(t *testing.T) { expectErrContaining(t, err, "could not open") expectPackages(t, packages, []lockfile.PackageDetails{}) } + +func TestParseRequirementsTxt_DuplicateROptions(t *testing.T) { + t.Parallel() + + packages, err := lockfile.ParseRequirementsTxt("fixtures/pip/duplicate-r-dev.txt") + + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + + expectPackages(t, packages, []lockfile.PackageDetails{ + { + Name: "django", + Version: "0.1.0", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "pandas", + Version: "0.23.4", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "requests", + Version: "1.2.3", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "unittest", + Version: "1.0.0", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + }) +} +func TestParseRequirementsTxt_CyclicRSelf(t *testing.T) { + t.Parallel() + + packages, err := lockfile.ParseRequirementsTxt("fixtures/pip/cyclic-r-self.txt") + + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + + expectPackages(t, packages, []lockfile.PackageDetails{ + { + Name: "pandas", + Version: "0.23.4", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "requests", + Version: "1.2.3", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + }) +} +func TestParseRequirementsTxt_CyclicRComplex(t *testing.T) { + t.Parallel() + + packages, err := lockfile.ParseRequirementsTxt("fixtures/pip/cyclic-r-complex-1.txt") + + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + + expectPackages(t, packages, []lockfile.PackageDetails{ + { + Name: "cyclic-r-complex", + Version: "1", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "cyclic-r-complex", + Version: "2", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + { + Name: "cyclic-r-complex", + Version: "3", + Ecosystem: lockfile.PipEcosystem, + CompareAs: lockfile.PipEcosystem, + }, + }) +}