Repository: google/osv-scanner
Pull request #366 (2023-05-04 23:24:35)

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

google-cla[bot]: Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this [failed invocation](https://github.com/google/osv-scanner/pull/366/checks?check_run_id=13248752918) of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Patch:
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 0000000..b0a64dd
--- /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 0000000..22a5685
--- /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 0000000..da9fa11
--- /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 0000000..99af87f
--- /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 0000000..123602f
--- /dev/null
+++ b/pkg/lockfile/fixtures/pip/duplicate-r-base.txt
@@ -0,0 +1,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 0000000..44645b8
--- /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/parse-requirements-txt.go b/pkg/lockfile/parse-requirements-txt.go
index 4f46a0b..d7564a4 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)


Test patch:
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 0000000..13d13c6
--- /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_test.go b/pkg/lockfile/parse-requirements-txt_test.go
index 767c9a8..845bb15 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,
+		},
+	})
+}