From 8d9b05b8658094d4dd30f5d38518819632ac5b16 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= <nechtom@gmail.com>
Date: Thu, 1 Jun 2023 23:44:33 +0200
Subject: [PATCH] fsutil: Add Dir() and Clean() functions

From the code:

  These functions exists because we work with slash terminated
  directory paths that come from deb package tarballs but standard
  library path functions treat slash terminated paths as unclean.

We use ad-hoc code to make filepath.Dir() slash terminated in two places
right now. For example this code

  targetDir := filepath.Dir(strings.TrimRight(targetPath, "/")) + "/"

is not strictly correct as "/a/b/.///" will be turned into "/a/b/./"
which is still "/a/b".

Since we deal with slash terminated directory paths everywhere, create
and use dedicated helper functions to process them.
---
 internal/fsutil/path.go      | 47 ++++++++++++++++++++++++++++++++
 internal/fsutil/path_test.go | 52 ++++++++++++++++++++++++++++++++++++
 internal/slicer/slicer.go    | 16 ++++-------
 3 files changed, 104 insertions(+), 11 deletions(-)
 create mode 100644 internal/fsutil/path.go
 create mode 100644 internal/fsutil/path_test.go

diff --git a/internal/fsutil/path.go b/internal/fsutil/path.go
new file mode 100644
index 00000000..7d73c523
--- /dev/null
+++ b/internal/fsutil/path.go
@@ -0,0 +1,47 @@
+package fsutil
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+// These functions exists because we work with slash terminated directory paths
+// that come from deb package tarballs but standard library path functions
+// treat slash terminated paths as unclean.
+
+// Like filepath.Dir() but trailing slash on input is ignored and the return
+// value always includes trailing slash.
+//
+// Comparison of filepath.Dir() with fsutil.Dir():
+//
+//	filepath.Dir("/foo/bar/")	== "/foo/bar"
+//	filepath.Dir("/foo/bar")	== "/foo"
+//
+//	fsutil.Dir("/foo/bar")		== "/foo/"
+//	fsutil.Dir("/foo/bar/")		== "/foo/"
+func Dir(path string) string {
+	parent := filepath.Dir(filepath.Clean(path))
+	if parent != "/" {
+		parent += "/"
+	}
+	return parent
+}
+
+// Like filepath.Clean() but keeps trailing slash.
+//
+// Comparison of filepath.Clean() with fsutil.Clean():
+//
+//	filepath.Clean("/foo/bar")	== "/foo/bar"
+//	filepath.Clean("/foo/bar/")	== "/foo/bar"
+//	filepath.Clean("/foo/bar/.//)	== "/foo/bar"
+//
+//	fsutil.Clean("/foo/bar")	== "/foo/bar"
+//	fsutil.Clean("/foo/bar/")	== "/foo/bar/"
+//	fsutil.Clean("/foo/bar/.//")	== "/foo/bar/"
+func Clean(path string) string {
+	clean := filepath.Clean(path)
+	if strings.HasSuffix(path, "/") {
+		clean += "/"
+	}
+	return clean
+}
diff --git a/internal/fsutil/path_test.go b/internal/fsutil/path_test.go
new file mode 100644
index 00000000..9d39230c
--- /dev/null
+++ b/internal/fsutil/path_test.go
@@ -0,0 +1,52 @@
+package fsutil_test
+
+import (
+	. "gopkg.in/check.v1"
+
+	"github.com/canonical/chisel/internal/fsutil"
+)
+
+var dirTests = []struct {
+	input  string
+	output string
+}{
+	{"/a/b/c", "/a/b/"},
+	{"/a/b/c/", "/a/b/"},
+	{"/a/b/c//", "/a/b/"},
+	{"/a/b/c/././", "/a/b/"},
+	{"/a/b/c/.././", "/a/"},
+	{"/a/b//c", "/a/b/"},
+	{"/a/b/./c", "/a/b/"},
+	{"a/b/./c", "a/b/"},
+	{"./a/b/./c", "a/b/"},
+	{"/", "/"},
+	{"///.///", "/"},
+	{".", "./"},
+	{"", "./"},
+}
+
+var cleanTests = []struct {
+	input  string
+	output string
+}{
+	{"/a/b/c", "/a/b/c"},
+	{"/a/b/c/", "/a/b/c/"},
+	{"/a/b/c/.//", "/a/b/c/"},
+	{"/a/b//./c", "/a/b/c"},
+	{"/a/b//./c/", "/a/b/c/"},
+	{"/a/b/.././c/", "/a/c/"},
+}
+
+func (s *S) TestDir(c *C) {
+	for _, t := range dirTests {
+		c.Logf("%s => %s", t.input, t.output)
+		c.Assert(fsutil.Dir(t.input), Equals, t.output)
+	}
+}
+
+func (s *S) TestClean(c *C) {
+	for _, t := range cleanTests {
+		c.Logf("%s => %s", t.input, t.output)
+		c.Assert(fsutil.Clean(t.input), Equals, t.output)
+	}
+}
diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go
index 8fca7426..50198a96 100644
--- a/internal/slicer/slicer.go
+++ b/internal/slicer/slicer.go
@@ -36,21 +36,15 @@ func Run(options *RunOptions) error {
 		if path[0] != '/' {
 			panic("bug: tried to add relative path to known paths")
 		}
-		cleanPath := filepath.Clean(path)
-		slashPath := cleanPath
-		if path[len(path)-1] == '/' && cleanPath != "/" {
-			slashPath += "/"
-		}
+		path = fsutil.Clean(path)
 		for {
-			if _, ok := knownPaths[slashPath]; ok {
+			if _, ok := knownPaths[path]; ok {
 				break
 			}
-			knownPaths[slashPath] = true
-			cleanPath = filepath.Dir(cleanPath)
-			if cleanPath == "/" {
+			knownPaths[path] = true
+			if path = fsutil.Dir(path); path == "/" {
 				break
 			}
-			slashPath = cleanPath + "/"
 		}
 	}
 
@@ -113,7 +107,7 @@ func Run(options *RunOptions) error {
 					hasCopyright = true
 				}
 			} else {
-				targetDir := filepath.Dir(strings.TrimRight(targetPath, "/")) + "/"
+				targetDir := fsutil.Dir(targetPath)
 				if targetDir == "" || targetDir == "/" {
 					continue
 				}