From f83df40e7720621d9449cd5f786c3d63269f492e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Virtus?= Date: Thu, 1 Jun 2023 23:44:33 +0200 Subject: [PATCH] fsutil: Add SlashedPathDir() and SlashedPathClean() 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 | 66 ++++++++++++++++++++++++++++++++++++ internal/fsutil/path_test.go | 54 +++++++++++++++++++++++++++++ internal/slicer/slicer.go | 18 ++++------ 3 files changed, 127 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..650742f2 --- /dev/null +++ b/internal/fsutil/path.go @@ -0,0 +1,66 @@ +package fsutil + +import ( + "path/filepath" +) + +// isDirPath returns whether the path refers a directory. +// The path refers to a directory when it ends with "/", "/." or "/..", or when +// it equals "." or "..". +func isDirPath(path string) bool { + i := len(path) - 1 + if i < 0 { + return true + } + if path[i] == '.' { + i-- + if i < 0 { + return true + } + if path[i] == '.' { + i-- + if i < 0 { + return true + } + } + } + if path[i] == '/' { + return true + } + return false +} + +// Debian package tarballs present paths slightly differently to what we would +// normally classify as clean paths. While a traditional clean file path is identical +// to a clean deb package file path, the deb package directory path always ends +// with a slash. Although the change only affects directory paths, the implication +// is that a directory path without a slash is interpreted as a file path. For this +// reason, we need to be very careful and handle both file and directory paths using +// a new set of functions. We call this new path type a Slashed Path. A slashed path +// allows us to identify a file or directory simply using lexical analysis. + +// SlashedPathDir takes a file or slashed directory path as input, cleans the +// path and returns the parent directory. An input path ending without a slash +// will be interpreted as a file path. Directory paths should always end with a slash. +// Clean is like filepath.Clean() but trailing slash is kept. +func SlashedPathClean(path string) string { + clean := filepath.Clean(path) + if clean != "/" && isDirPath(path) { + clean += "/" + } + return clean +} + +// SlashedPathClean takes a file or slashed directory path as input, and produces +// the shortest equivalent as output. An input path ending without a slash will be +// interpreted as a file path. Directory paths should always end with a slash. +// 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. +func SlashedPathDir(path string) string { + parent := filepath.Dir(filepath.Clean(path)) + if parent != "/" { + parent += "/" + } + return parent +} diff --git a/internal/fsutil/path_test.go b/internal/fsutil/path_test.go new file mode 100644 index 00000000..864ec594 --- /dev/null +++ b/internal/fsutil/path_test.go @@ -0,0 +1,54 @@ +package fsutil_test + +import ( + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/fsutil" +) + +var cleanAndDirTestCases = []struct { + inputPath string + resultClean string + resultDir string +}{ + {"/a/b/c", "/a/b/c", "/a/b/"}, + {"/a/b/c/", "/a/b/c/", "/a/b/"}, + {"/a/b/c//", "/a/b/c/", "/a/b/"}, + {"/a/b//c", "/a/b/c", "/a/b/"}, + {"/a/b/c/.", "/a/b/c/", "/a/b/"}, + {"/a/b/c/.///.", "/a/b/c/", "/a/b/"}, + {"/a/b/./c/", "/a/b/c/", "/a/b/"}, + {"/a/b/.///./c", "/a/b/c", "/a/b/"}, + {"/a/b/c/..", "/a/b/", "/a/"}, + {"/a/b/c/..///./", "/a/b/", "/a/"}, + {"/a/b/c/../.", "/a/b/", "/a/"}, + {"/a/b/../c/", "/a/c/", "/a/"}, + {"/a/b/..///./c", "/a/c", "/a/"}, + {"a/b/./c", "a/b/c", "a/b/"}, + {"./a/b/./c", "a/b/c", "a/b/"}, + {"/", "/", "/"}, + {"///", "/", "/"}, + {"///.///", "/", "/"}, + {"/././.", "/", "/"}, + {".", "./", "./"}, + {".///", "./", "./"}, + {"..", "../", "./"}, + {"..///.", "../", "./"}, + {"../../..", "../../../", "../../"}, + {"..///.///../..", "../../../", "../../"}, + {"", "./", "./"}, +} + +func (s *S) TestSlashedPathClean(c *C) { + for _, t := range cleanAndDirTestCases { + c.Logf("%s => %s", t.inputPath, t.resultClean) + c.Assert(fsutil.SlashedPathClean(t.inputPath), Equals, t.resultClean) + } +} + +func (s *S) TestSlashedPathDir(c *C) { + for _, t := range cleanAndDirTestCases { + c.Logf("%s => %s", t.inputPath, t.resultDir) + c.Assert(fsutil.SlashedPathDir(t.inputPath), Equals, t.resultDir) + } +} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 2b684b5d..9cd35aec 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -32,25 +32,21 @@ func Run(options *RunOptions) error { knownPaths["/"] = true + // addKnownPath path adds path and all its directory parent paths into + // knownPaths set. addKnownPath := func(path string) { 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.SlashedPathClean(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.SlashedPathDir(path); path == "/" { break } - slashPath = cleanPath + "/" } } @@ -113,7 +109,7 @@ func Run(options *RunOptions) error { hasCopyright = true } } else { - targetDir := filepath.Dir(strings.TrimRight(targetPath, "/")) + "/" + targetDir := fsutil.SlashedPathDir(targetPath) if targetDir == "" || targetDir == "/" { continue }