Add better error handling for filesystem

This commit is contained in:
Dane Everitt 2021-04-17 13:29:18 -07:00
parent a0ae5fd131
commit 0676a82a21
8 changed files with 86 additions and 43 deletions

View File

@ -2,13 +2,13 @@ package backup
import ( import (
"errors" "errors"
"github.com/pterodactyl/wings/server/filesystem"
"io" "io"
"os" "os"
"github.com/pterodactyl/wings/server/filesystem"
"github.com/mholt/archiver/v3" "github.com/mholt/archiver/v3"
"github.com/pterodactyl/wings/remote" "github.com/pterodactyl/wings/remote"
"github.com/pterodactyl/wings/system"
) )
type LocalBackup struct { type LocalBackup struct {
@ -78,10 +78,6 @@ func (b *LocalBackup) Restore(_ io.Reader, callback RestoreCallback) error {
if f.IsDir() { if f.IsDir() {
return nil return nil
} }
name, err := system.ExtractArchiveSourceName(f, "/") return callback(f.Name(), f)
if err != nil {
return err
}
return callback(name, f)
}) })
} }

View File

@ -9,8 +9,8 @@ import (
"sync/atomic" "sync/atomic"
"time" "time"
"emperror.dev/errors"
"github.com/mholt/archiver/v3" "github.com/mholt/archiver/v3"
"github.com/pterodactyl/wings/system"
) )
// CompressFiles compresses all of the files matching the given paths in the // CompressFiles compresses all of the files matching the given paths in the
@ -86,13 +86,13 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) er
// Walk over the archive and figure out just how large the final output would be from unarchiving it. // 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 { err = archiver.Walk(source, func(f archiver.File) error {
if atomic.AddInt64(&size, f.Size())+dirSize > fs.MaxDisk() { if atomic.AddInt64(&size, f.Size())+dirSize > fs.MaxDisk() {
return &Error{code: ErrCodeDiskSpace} return newFilesystemError(ErrCodeDiskSpace, nil)
} }
return nil return nil
}) })
if err != nil { if err != nil {
if strings.HasPrefix(err.Error(), "format ") { if IsUnknownArchiveFormatError(err) {
return &Error{code: ErrCodeUnknownArchive} return newFilesystemError(ErrCodeUnknownArchive, err)
} }
return err return err
} }
@ -111,7 +111,7 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error {
} }
// Ensure that the source archive actually exists on the system. // Ensure that the source archive actually exists on the system.
if _, err := os.Stat(source); err != nil { if _, err := os.Stat(source); err != nil {
return err return errors.WithStack(err)
} }
// Walk all of the files in the archiver file and write them to the disk. If any // Walk all of the files in the archiver file and write them to the disk. If any
@ -127,13 +127,13 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error {
return nil return nil
} }
if err := fs.Writefile(p, f); err != nil { if err := fs.Writefile(p, f); err != nil {
return &Error{code: ErrCodeUnknownError, err: err, resolved: source} return wrapError(err, source)
} }
return nil return nil
}) })
if err != nil { if err != nil {
if strings.HasPrefix(err.Error(), "format ") { if IsUnknownArchiveFormatError(err) {
return &Error{code: ErrCodeUnknownArchive} return newFilesystemError(ErrCodeUnknownArchive, err)
} }
return err return err
} }

View File

@ -1,13 +1,14 @@
package filesystem package filesystem
import ( import (
"emperror.dev/errors"
"github.com/apex/log"
"github.com/karrick/godirwalk"
"sync" "sync"
"sync/atomic" "sync/atomic"
"syscall" "syscall"
"time" "time"
"emperror.dev/errors"
"github.com/apex/log"
"github.com/karrick/godirwalk"
) )
type SpaceCheckingOpts struct { type SpaceCheckingOpts struct {
@ -48,7 +49,7 @@ func (fs *Filesystem) SetDiskLimit(i int64) {
// no space, rather than a boolean value. // no space, rather than a boolean value.
func (fs *Filesystem) HasSpaceErr(allowStaleValue bool) error { func (fs *Filesystem) HasSpaceErr(allowStaleValue bool) error {
if !fs.HasSpaceAvailable(allowStaleValue) { if !fs.HasSpaceAvailable(allowStaleValue) {
return &Error{code: ErrCodeDiskSpace} return newFilesystemError(ErrCodeDiskSpace, nil)
} }
return nil return nil
} }
@ -200,16 +201,13 @@ func (fs *Filesystem) HasSpaceFor(size int64) error {
if fs.MaxDisk() == 0 { if fs.MaxDisk() == 0 {
return nil return nil
} }
s, err := fs.DiskUsage(true) s, err := fs.DiskUsage(true)
if err != nil { if err != nil {
return err return err
} }
if (s + size) > fs.MaxDisk() { if (s + size) > fs.MaxDisk() {
return &Error{code: ErrCodeDiskSpace} return newFilesystemError(ErrCodeDiskSpace, nil)
} }
return nil return nil
} }

View File

@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"emperror.dev/errors" "emperror.dev/errors"
"github.com/apex/log" "github.com/apex/log"
@ -34,6 +35,14 @@ type Error struct {
path string path string
} }
// newFilesystemError returns a new error instance with a stack trace associated.
func newFilesystemError(code ErrorCode, err error) error {
if err != nil {
return errors.WithStackDepth(&Error{code: code, err: err}, 1)
}
return errors.WithStackDepth(&Error{code: code}, 1)
}
// Code returns the ErrorCode for this specific error instance. // Code returns the ErrorCode for this specific error instance.
func (e *Error) Code() ErrorCode { func (e *Error) Code() ErrorCode {
return e.code return e.code
@ -63,13 +72,13 @@ func (e *Error) Error() string {
case ErrCodeUnknownError: case ErrCodeUnknownError:
fallthrough fallthrough
default: default:
return fmt.Sprintf("filesystem: an error occurred: %s", e.Cause()) return fmt.Sprintf("filesystem: an error occurred: %s", e.Unwrap())
} }
} }
// Cause returns the underlying cause of this filesystem error. In some causes // Unwrap returns the underlying cause of this filesystem error. In some causes
// there may not be a cause present, in which case nil will be returned. // there may not be a cause present, in which case nil will be returned.
func (e *Error) Cause() error { func (e *Error) Unwrap() error {
return e.err return e.err
} }
@ -113,20 +122,26 @@ func IsErrorCode(err error, code ErrorCode) bool {
return false return false
} }
// NewBadPathResolution returns a new BadPathResolution error. // IsUnknownArchiveFormatError checks if the error is due to the archive being
func NewBadPathResolution(path string, resolved string) *Error { // in an unexpected file format.
return &Error{code: ErrCodePathResolution, path: path, resolved: resolved} func IsUnknownArchiveFormatError(err error) bool {
if err != nil && strings.HasPrefix(err.Error(), "format ") {
return true
}
return false
} }
// WrapError wraps the provided error as a Filesystem error and attaches the // NewBadPathResolution returns a new BadPathResolution error.
func NewBadPathResolution(path string, resolved string) error {
return errors.WithStackDepth(&Error{code: ErrCodePathResolution, path: path, resolved: resolved}, 1)
}
// wrapError wraps the provided error as a Filesystem error and attaches the
// provided resolved source to it. If the error is already a Filesystem error // provided resolved source to it. If the error is already a Filesystem error
// no action is taken. // no action is taken.
func WrapError(err error, resolved string) *Error { func wrapError(err error, resolved string) error {
if err == nil { if err == nil || IsFilesystemError(err) {
return nil return err
} }
if IsFilesystemError(err) { return errors.WithStackDepth(&Error{code: ErrCodeUnknownError, err: err, resolved: resolved}, 1)
return err.(*Error)
}
return &Error{code: ErrCodeUnknownError, err: err, resolved: resolved}
} }

View File

@ -1,13 +1,45 @@
package filesystem package filesystem
import ( import (
. "github.com/franela/goblin" "io"
"testing" "testing"
"emperror.dev/errors"
. "github.com/franela/goblin"
) )
type stackTracer interface {
StackTrace() errors.StackTrace
}
func TestFilesystem_PathResolutionError(t *testing.T) { func TestFilesystem_PathResolutionError(t *testing.T) {
g := Goblin(t) g := Goblin(t)
g.Describe("NewFilesystemError", func() {
g.It("includes a stack trace for the error", func() {
err := newFilesystemError(ErrCodeUnknownError, nil)
_, ok := err.(stackTracer)
g.Assert(ok).IsTrue()
})
g.It("properly wraps the underlying error cause", func() {
underlying := io.EOF
err := newFilesystemError(ErrCodeUnknownError, underlying)
_, ok := err.(stackTracer)
g.Assert(ok).IsTrue()
_, ok = err.(*Error)
g.Assert(ok).IsFalse()
fserr, ok := errors.Unwrap(err).(*Error)
g.Assert(ok).IsTrue()
g.Assert(fserr.Unwrap()).IsNotNil()
g.Assert(fserr.Unwrap()).Equal(underlying)
})
})
g.Describe("NewBadPathResolutionError", func() { g.Describe("NewBadPathResolutionError", func() {
g.It("is can detect itself as an error correctly", func() { g.It("is can detect itself as an error correctly", func() {
err := NewBadPathResolution("foo", "bar") err := NewBadPathResolution("foo", "bar")
@ -18,6 +50,7 @@ func TestFilesystem_PathResolutionError(t *testing.T) {
g.It("returns <empty> if no destination path is provided", func() { g.It("returns <empty> if no destination path is provided", func() {
err := NewBadPathResolution("foo", "") err := NewBadPathResolution("foo", "")
g.Assert(err).IsNotNil()
g.Assert(err.Error()).Equal("filesystem: server path [foo] resolves to a location outside the server root: <empty>") g.Assert(err.Error()).Equal("filesystem: server path [foo] resolves to a location outside the server root: <empty>")
}) })
}) })

View File

@ -67,7 +67,7 @@ func (fs *Filesystem) File(p string) (*os.File, Stat, error) {
return nil, Stat{}, err return nil, Stat{}, err
} }
if st.IsDir() { if st.IsDir() {
return nil, Stat{}, &Error{code: ErrCodeIsDirectory} return nil, Stat{}, newFilesystemError(ErrCodeIsDirectory, nil)
} }
f, err := os.Open(cleaned) f, err := os.Open(cleaned)
if err != nil { if err != nil {
@ -144,7 +144,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
return errors.Wrap(err, "server/filesystem: writefile: failed to stat file") return errors.Wrap(err, "server/filesystem: writefile: failed to stat file")
} else if err == nil { } else if err == nil {
if stat.IsDir() { if stat.IsDir() {
return &Error{code: ErrCodeIsDirectory, resolved: cleaned} return errors.WithStack(&Error{code: ErrCodeIsDirectory, resolved: cleaned})
} }
currentSize = stat.Size() currentSize = stat.Size()
} }

View File

@ -20,7 +20,7 @@ func (fs *Filesystem) IsIgnored(paths ...string) error {
return err return err
} }
if fs.denylist.MatchesPath(sp) { if fs.denylist.MatchesPath(sp) {
return &Error{code: ErrCodeDenylistFile, path: p, resolved: sp} return errors.WithStack(&Error{code: ErrCodeDenylistFile, path: p, resolved: sp})
} }
} }
return nil return nil

View File

@ -2,11 +2,12 @@ package filesystem
import ( import (
"bytes" "bytes"
"emperror.dev/errors"
. "github.com/franela/goblin"
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
"emperror.dev/errors"
. "github.com/franela/goblin"
) )
func TestFilesystem_Path(t *testing.T) { func TestFilesystem_Path(t *testing.T) {