From 3a26a5d39d2f813bb7361a40600745aa2ab9bd94 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Tue, 15 Dec 2020 20:51:13 -0800 Subject: [PATCH] Convert all filesystem error types into the same Error struct --- router/error.go | 6 +- router/router_server_files.go | 9 ++- server/filesystem/compress.go | 4 +- server/filesystem/decompress.go | 6 +- server/filesystem/disk_space.go | 4 +- server/filesystem/errors.go | 84 +++++++++++++++++++--------- server/filesystem/errors_test.go | 4 +- server/filesystem/filesystem.go | 4 +- server/filesystem/filesystem_test.go | 24 ++++---- server/filesystem/path_test.go | 32 +++++------ server/power.go | 2 +- 11 files changed, 104 insertions(+), 75 deletions(-) diff --git a/router/error.go b/router/error.go index 20b1281..126094f 100644 --- a/router/error.go +++ b/router/error.go @@ -99,8 +99,8 @@ func (e *RequestError) Abort(c *gin.Context) { // Handle specific filesystem errors for a server. func (e *RequestError) AbortFilesystemError(c *gin.Context) { - if errors.Is(e.Err, os.ErrNotExist) || filesystem.IsBadPathResolutionError(e.Err) { - if filesystem.IsBadPathResolutionError(e.Err) { + if errors.Is(e.Err, os.ErrNotExist) || filesystem.IsErrorCode(e.Err, filesystem.ErrCodePathResolution) { + if filesystem.IsErrorCode(e.Err, filesystem.ErrCodePathResolution) { e.logger().Warn(e.Err.Error()) } @@ -108,7 +108,7 @@ func (e *RequestError) AbortFilesystemError(c *gin.Context) { return } - if errors.Is(e.Err, filesystem.ErrNotEnoughDiskSpace) { + if filesystem.IsErrorCode(e.Err, filesystem.ErrCodeDiskSpace) { c.AbortWithStatusJSON(http.StatusConflict, gin.H{"error": "There is not enough disk space available to perform that action."}) return } diff --git a/router/router_server_files.go b/router/router_server_files.go index 58a0e56..c41d6f0 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -56,7 +56,7 @@ func getServerFileContents(c *gin.Context) { // Returns the contents of a directory for a server. func getServerListDirectory(c *gin.Context) { - s := GetServer(c.Param("server")) + s := ExtractServer(c) stats, err := s.Filesystem().ListDirectory(c.Query("directory")) if err != nil { @@ -211,7 +211,7 @@ func postServerWriteFile(c *gin.Context) { f = "/" + strings.TrimLeft(f, "/") if err := s.Filesystem().Writefile(f, c.Request.Body); err != nil { - if errors.Is(err, filesystem.ErrIsDirectory) { + if filesystem.IsErrorCode(err, filesystem.ErrCodeIsDirectory) { c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "Cannot write file, name conflicts with an existing directory by the same name.", }) @@ -257,7 +257,7 @@ func postServerDownloadRemoteFile(c *gin.Context) { filename := strings.Split(u.Path, "/") if err := s.Filesystem().Writefile(filepath.Join(data.BasePath, filename[len(filename)-1]), resp.Body); err != nil { - if errors.Is(err, filesystem.ErrIsDirectory) { + if filesystem.IsErrorCode(err, filesystem.ErrCodeIsDirectory) { c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "Cannot write file, name conflicts with an existing directory by the same name.", }) @@ -351,9 +351,8 @@ func postServerDecompressFiles(c *gin.Context) { hasSpace, err := s.Filesystem().SpaceAvailableForDecompression(data.RootPath, data.File) if err != nil { // Handle an unknown format error. - if errors.Is(err, filesystem.ErrUnknownArchiveFormat) { + if filesystem.IsErrorCode(err, filesystem.ErrCodeUnknownArchive) { s.Log().WithField("error", err).Warn("failed to decompress file due to unknown format") - c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "unknown archive format", }) diff --git a/server/filesystem/compress.go b/server/filesystem/compress.go index 98c77bc..07c7e19 100644 --- a/server/filesystem/compress.go +++ b/server/filesystem/compress.go @@ -40,7 +40,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In if e.IsSymlink() { sp, err = fs.SafePath(p) if err != nil { - if IsBadPathResolutionError(err) { + if IsErrorCode(err, ErrCodePathResolution) { return godirwalk.SkipThis } @@ -114,7 +114,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er // use the resolved location for the rest of this function. sp, err = fs.SafePath(p) if err != nil { - if IsBadPathResolutionError(err) { + if IsErrorCode(err, ErrCodePathResolution) { return godirwalk.SkipThis } diff --git a/server/filesystem/decompress.go b/server/filesystem/decompress.go index 28f0d3b..8663dd9 100644 --- a/server/filesystem/decompress.go +++ b/server/filesystem/decompress.go @@ -36,7 +36,7 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b // Walk over the archive and figure out just how large the final output would be from unarchiving it. err = archiver.Walk(source, func(f archiver.File) error { if atomic.AddInt64(&size, f.Size())+dirSize > fs.MaxDisk() { - return ErrNotEnoughDiskSpace + return &Error{code: ErrCodeDiskSpace} } return nil @@ -44,7 +44,7 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b if err != nil { if strings.HasPrefix(err.Error(), "format ") { - return false, ErrUnknownArchiveFormat + return false, &Error{code: ErrCodeUnknownArchive} } return false, err @@ -100,7 +100,7 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error { }) if err != nil { if strings.HasPrefix(err.Error(), "format ") { - return ErrUnknownArchiveFormat + return &Error{code: ErrCodeUnknownArchive} } return err diff --git a/server/filesystem/disk_space.go b/server/filesystem/disk_space.go index 6d3a099..ccef0ac 100644 --- a/server/filesystem/disk_space.go +++ b/server/filesystem/disk_space.go @@ -171,7 +171,7 @@ func (fs *Filesystem) DirectorySize(dir string) (int64, error) { // it. Otherwise, allow it to continue. if e.IsSymlink() { if _, err := fs.SafePath(p); err != nil { - if IsBadPathResolutionError(err) { + if IsErrorCode(err, ErrCodePathResolution) { return godirwalk.SkipThis } @@ -205,7 +205,7 @@ func (fs *Filesystem) hasSpaceFor(size int64) error { } if (s + size) > fs.MaxDisk() { - return ErrNotEnoughDiskSpace + return &Error{code: ErrCodeDiskSpace} } return nil diff --git a/server/filesystem/errors.go b/server/filesystem/errors.go index 823441c..2f8cd52 100644 --- a/server/filesystem/errors.go +++ b/server/filesystem/errors.go @@ -8,42 +8,72 @@ import ( "path/filepath" ) -var ErrIsDirectory = errors.New("filesystem: is a directory") -var ErrNotEnoughDiskSpace = errors.New("filesystem: not enough disk space") -var ErrUnknownArchiveFormat = errors.New("filesystem: unknown archive format") +type ErrorCode string -type BadPathResolutionError struct { +const ( + ErrCodeIsDirectory ErrorCode = "E_ISDIR" + ErrCodeDiskSpace ErrorCode = "E_NODISK" + ErrCodeUnknownArchive ErrorCode = "E_UNKNFMT" + ErrCodePathResolution ErrorCode = "E_BADPATH" +) + +type Error struct { + code ErrorCode path string resolved string } -// Returns the specific error for a bad path resolution. -func (b *BadPathResolutionError) Error() string { - r := b.resolved - if r == "" { - r = "" +// Returns a human-readable error string to identify the Error by. +func (e *Error) Error() string { + switch e.code { + case ErrCodeIsDirectory: + return "filesystem: is a directory" + case ErrCodeDiskSpace: + return "filesystem: not enough disk space" + case ErrCodeUnknownArchive: + return "filesystem: unknown archive format" + case ErrCodePathResolution: + r := e.resolved + if r == "" { + r = "" + } + return fmt.Sprintf("filesystem: server path [%s] resolves to a location outside the server root: %s", e.path, r) } + return "filesystem: unhandled error type" +} - return fmt.Sprintf("filesystem: server path [%s] resolves to a location outside the server root: %s", b.path, r) +// Returns the ErrorCode for this specific error instance. +func (e *Error) Code() ErrorCode { + return e.code +} + +// Checks if the given error is one of the Filesystem errors. +func IsFilesystemError(err error) (*Error, bool) { + if e := errors.Unwrap(err); e != nil { + err = e + } + if fserr, ok := err.(*Error); ok { + return fserr, true + } + return nil, false +} + +// Checks if "err" is a filesystem Error type. If so, it will then drop in and check +// that the error code is the same as the provided ErrorCode passed in "code". +func IsErrorCode(err error, code ErrorCode) bool { + if e, ok := IsFilesystemError(err); ok { + return e.code == code + } + return false +} + +func NewDiskSpaceError() *Error { + return &Error{code: ErrCodeDiskSpace} } // Returns a new BadPathResolution error. -func NewBadPathResolution(path string, resolved string) *BadPathResolutionError { - return &BadPathResolutionError{path, resolved} -} - -// Determines if the given error is a bad path resolution error. -func IsBadPathResolutionError(err error) bool { - e := errors.Unwrap(err) - if e == nil { - e = err - } - - if _, ok := e.(*BadPathResolutionError); ok { - return true - } - - return false +func NewBadPathResolution(path string, resolved string) *Error { + return &Error{code: ErrCodePathResolution, path: path, resolved: resolved} } // Generates an error logger instance with some basic information. @@ -57,7 +87,7 @@ func (fs *Filesystem) error(err error) *log.Entry { // directory, otherwise return nil. Returning this error for a file will stop the walking // for the remainder of the directory. This is assuming an os.FileInfo struct was even returned. func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error { - if !IsBadPathResolutionError(err) { + if !IsErrorCode(err, ErrCodePathResolution) { return err } diff --git a/server/filesystem/errors_test.go b/server/filesystem/errors_test.go index e860cc6..0104e4c 100644 --- a/server/filesystem/errors_test.go +++ b/server/filesystem/errors_test.go @@ -11,9 +11,9 @@ func TestFilesystem_PathResolutionError(t *testing.T) { g.Describe("NewBadPathResolutionError", func() { g.It("is can detect itself as an error correctly", func() { err := NewBadPathResolution("foo", "bar") - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() g.Assert(err.Error()).Equal("filesystem: server path [foo] resolves to a location outside the server root: bar") - g.Assert(IsBadPathResolutionError(ErrIsDirectory)).IsFalse() + g.Assert(IsErrorCode(&Error{code: ErrCodeIsDirectory}, ErrCodePathResolution)).IsFalse() }) g.It("returns if no destination path is provided", func() { diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go index f04a7f2..5679bfb 100644 --- a/server/filesystem/filesystem.go +++ b/server/filesystem/filesystem.go @@ -62,7 +62,7 @@ func (fs *Filesystem) Readfile(p string, w io.Writer) error { if st, err := os.Stat(cleaned); err != nil { return err } else if st.IsDir() { - return ErrIsDirectory + return &Error{code: ErrCodeIsDirectory} } f, err := os.Open(cleaned) @@ -100,7 +100,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error { } } else { if stat.IsDir() { - return ErrIsDirectory + return &Error{code: ErrCodeIsDirectory} } currentSize = stat.Size() diff --git a/server/filesystem/filesystem_test.go b/server/filesystem/filesystem_test.go index c39eacc..10b0b70 100644 --- a/server/filesystem/filesystem_test.go +++ b/server/filesystem/filesystem_test.go @@ -98,7 +98,7 @@ func TestFilesystem_Readfile(t *testing.T) { err = fs.Readfile("test.txt", buf) g.Assert(err).IsNotNil() - g.Assert(errors.Is(err, ErrIsDirectory)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodeIsDirectory)).IsTrue() }) g.It("cannot open a file outside the root directory", func() { @@ -107,7 +107,7 @@ func TestFilesystem_Readfile(t *testing.T) { err = fs.Readfile("/../test.txt", buf) g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.AfterEach(func() { @@ -168,7 +168,7 @@ func TestFilesystem_Writefile(t *testing.T) { err := fs.Writefile("/some/../foo/../../test.txt", r) g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot write a file that exceeds the disk limits", func() { @@ -182,7 +182,7 @@ func TestFilesystem_Writefile(t *testing.T) { r := bytes.NewReader(b) err = fs.Writefile("test.txt", r) g.Assert(err).IsNotNil() - g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() }) /*g.It("updates the total space used when a file is appended to", func() { @@ -259,7 +259,7 @@ func TestFilesystem_CreateDirectory(t *testing.T) { g.It("should not allow the creation of directories outside the root", func() { err := fs.CreateDirectory("test", "e/../../something") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("should not increment the disk usage", func() { @@ -309,7 +309,7 @@ func TestFilesystem_Rename(t *testing.T) { g.It("does not allow renaming to a location outside the root", func() { err := fs.Rename("source.txt", "../target.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("does not allow renaming from a location outside the root", func() { @@ -317,7 +317,7 @@ func TestFilesystem_Rename(t *testing.T) { err = fs.Rename("/../ext-source.txt", "target.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("allows a file to be renamed", func() { @@ -395,7 +395,7 @@ func TestFilesystem_Copy(t *testing.T) { err = fs.Copy("../ext-source.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("should return an error if the source directory is outside the root", func() { @@ -407,11 +407,11 @@ func TestFilesystem_Copy(t *testing.T) { err = fs.Copy("../nested/in/dir/ext-source.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() err = fs.Copy("nested/in/../../../nested/in/dir/ext-source.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("should return an error if the source is a directory", func() { @@ -428,7 +428,7 @@ func TestFilesystem_Copy(t *testing.T) { err := fs.Copy("source.txt") g.Assert(err).IsNotNil() - g.Assert(errors.Is(err, ErrNotEnoughDiskSpace)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodeDiskSpace)).IsTrue() }) g.It("should create a copy of the file and increment the disk used", func() { @@ -503,7 +503,7 @@ func TestFilesystem_Delete(t *testing.T) { err = fs.Delete("../ext-source.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("does not allow the deletion of the root directory", func() { diff --git a/server/filesystem/path_test.go b/server/filesystem/path_test.go index e80a040..2bef006 100644 --- a/server/filesystem/path_test.go +++ b/server/filesystem/path_test.go @@ -73,22 +73,22 @@ func TestFilesystem_SafePath(t *testing.T) { g.It("blocks access to files outside the root directory", func() { p, err := fs.SafePath("../test.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() g.Assert(p).Equal("") p, err = fs.SafePath("/../test.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() g.Assert(p).Equal("") p, err = fs.SafePath("./foo/../../test.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() g.Assert(p).Equal("") p, err = fs.SafePath("..") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() g.Assert(p).Equal("") }) }) @@ -124,7 +124,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { err := fs.Readfile("symlinked.txt", &b) g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) @@ -134,7 +134,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { err := fs.Writefile("symlinked.txt", r) g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot write a file to a directory symlinked outside the root", func() { @@ -142,7 +142,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { err := fs.Writefile("external_dir/foo.txt", r) g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) @@ -150,19 +150,19 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { g.It("cannot create a directory outside the root", func() { err := fs.CreateDirectory("my_dir", "external_dir") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot create a nested directory outside the root", func() { err := fs.CreateDirectory("my/nested/dir", "external_dir/foo/bar") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot create a nested directory outside the root", func() { err := fs.CreateDirectory("my/nested/dir", "external_dir/server") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) @@ -170,13 +170,13 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { g.It("cannot rename a file symlinked outside the directory root", func() { err := fs.Rename("symlinked.txt", "foo.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot rename a symlinked directory outside the root", func() { err := fs.Rename("external_dir", "foo") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot rename a file to a location outside the directory root", func() { @@ -184,7 +184,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { err := fs.Rename("my_file.txt", "external_dir/my_file.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) @@ -192,13 +192,13 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { g.It("cannot chown a file symlinked outside the directory root", func() { err := fs.Chown("symlinked.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) g.It("cannot chown a directory symlinked outside the directory root", func() { err := fs.Chown("external_dir") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) @@ -206,7 +206,7 @@ func TestFilesystem_Blocks_Symlinks(t *testing.T) { g.It("cannot copy a file symlinked outside the directory root", func() { err := fs.Copy("symlinked.txt") g.Assert(err).IsNotNil() - g.Assert(IsBadPathResolutionError(err)).IsTrue() + g.Assert(IsErrorCode(err, ErrCodePathResolution)).IsTrue() }) }) diff --git a/server/power.go b/server/power.go index a309515..fec86c3 100644 --- a/server/power.go +++ b/server/power.go @@ -169,7 +169,7 @@ func (s *Server) onBeforeStart() error { } else { s.PublishConsoleOutputFromDaemon("Checking server disk space usage, this could take a few seconds...") if !s.Filesystem().HasSpaceAvailable(false) { - return filesystem.ErrNotEnoughDiskSpace + return filesystem.NewDiskSpaceError() } }