sumdb/dirhash: fix a panic when argument is not a directory

This patch fixes a case where a path to a non directory can cause DirHash func to panic. Fixes golang/go#57269 Change-Id: I743970dcafdf3bfee94baf4b714429a404decda9 GitHub-Last-Rev: a17ca1a GitHub-Pull-Request: #16 Reviewed-on: https://go-review.googlesource.com/c/mod/+/457075 Run-TryBot: Bryan Mills <bcmills@google.com> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Benny Siegert <bsiegert@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
golang · Jan 16, 2023 · 77d797e · 77d797e
1 parent 7c05a44
commit 77d797e
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 26 deletions.
diff --git a/sumdb/dirhash/hash.go b/sumdb/dirhash/hash.go
@@ -90,7 +90,10 @@ func DirFiles(dir, prefix string) ([]string, error) {
 		}
 		if info.IsDir() {
 			return nil
+		} else if file == dir {
+			return fmt.Errorf("%s is not a directory", dir)
 		}
+
 		rel := file
 		if dir != "." {
 			rel = file[len(dir)+1:]

diff --git a/sumdb/dirhash/hash_test.go b/sumdb/dirhash/hash_test.go
@@ -105,31 +105,49 @@ func TestHashZip(t *testing.T) {
 }
 
 func TestDirFiles(t *testing.T) {
-	dir, err := ioutil.TempDir("", "dirfiles-test-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(dir)
-	if err := ioutil.WriteFile(filepath.Join(dir, "xyz"), []byte("data for xyz"), 0666); err != nil {
-		t.Fatal(err)
-	}
-	if err := ioutil.WriteFile(filepath.Join(dir, "abc"), []byte("data for abc"), 0666); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Mkdir(filepath.Join(dir, "subdir"), 0777); err != nil {
-		t.Fatal(err)
-	}
-	if err := ioutil.WriteFile(filepath.Join(dir, "subdir", "xyz"), []byte("data for subdir xyz"), 0666); err != nil {
-		t.Fatal(err)
-	}
-	prefix := "foo/bar@v2.3.4"
-	out, err := DirFiles(dir, prefix)
-	if err != nil {
-		t.Fatalf("DirFiles: %v", err)
-	}
-	for _, file := range out {
-		if !strings.HasPrefix(file, prefix) {
-			t.Errorf("Dir file = %s, want prefix %s", file, prefix)
+	t.Run("valid directory with files", func(t *testing.T) {
+		dir, err := ioutil.TempDir("", "dirfiles-test-")
+		if err != nil {
+			t.Fatal(err)
 		}
-	}
+		defer os.RemoveAll(dir)
+		if err := ioutil.WriteFile(filepath.Join(dir, "xyz"), []byte("data for xyz"), 0666); err != nil {
+			t.Fatal(err)
+		}
+		if err := ioutil.WriteFile(filepath.Join(dir, "abc"), []byte("data for abc"), 0666); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Mkdir(filepath.Join(dir, "subdir"), 0777); err != nil {
+			t.Fatal(err)
+		}
+		if err := ioutil.WriteFile(filepath.Join(dir, "subdir", "xyz"), []byte("data for subdir xyz"), 0666); err != nil {
+			t.Fatal(err)
+		}
+		prefix := "foo/bar@v2.3.4"
+		out, err := DirFiles(dir, prefix)
+		if err != nil {
+			t.Fatalf("DirFiles: %v", err)
+		}
+		for _, file := range out {
+			if !strings.HasPrefix(file, prefix) {
+				t.Errorf("Dir file = %s, want prefix %s", file, prefix)
+			}
+		}
+	})
+
+	t.Run("invalid directory", func(t *testing.T) {
+		path := filepath.Join(t.TempDir(), "not-a-directory.txt")
+		if err := os.WriteFile(path, []byte("This is a file."), 0644); err != nil {
+			t.Fatal(err)
+		}
+		defer os.RemoveAll(path)
+
+		out, err := DirFiles(path, "")
+		if err == nil {
+			t.Errorf("DirFiles(...) = %v, expected an error", err)
+		}
+		if len(out) > 0 {
+			t.Errorf("DirFiles(...) = unexpected files %s", out)
+		}
+	})
 }