From e9e70b60817fc58683e1a9a5d508e3e9f2fdc668 Mon Sep 17 00:00:00 2001
From: Dane Everitt <dane@daneeveritt.com>
Date: Sun, 10 Jan 2021 17:01:41 -0800
Subject: [PATCH] Better error handling; skip file when unarchiving

---
 router/error.go                 | 20 +++++++++++---------
 router/router_server_files.go   | 13 +++----------
 server/filesystem/decompress.go |  3 ++-
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/router/error.go b/router/error.go
index 7503f89..f7be956 100644
--- a/router/error.go
+++ b/router/error.go
@@ -77,7 +77,6 @@ func (e *RequestError) AbortWithStatus(status int, c *gin.Context) {
 	// If this error is because the resource does not exist, we likely do not need to log
 	// the error anywhere, just return a 404 and move on with our lives.
 	if errors.Is(e.err, os.ErrNotExist) {
-		e.logger().Debug("encountered os.IsNotExist error while handling request")
 		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
 			"error": "The requested resource was not found on the system.",
 		})
@@ -122,16 +121,19 @@ func (e *RequestError) Abort(c *gin.Context) {
 // Looks at the given RequestError and determines if it is a specific filesystem error that
 // we can process and return differently for the user.
 func (e *RequestError) getAsFilesystemError() (int, string) {
-	if filesystem.IsErrorCode(e.err, filesystem.ErrCodePathResolution) || errors.Is(e.err, os.ErrNotExist) {
-		return http.StatusNotFound, "The requested resource was not found on the system."
-	}
-	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeDiskSpace) {
-		return http.StatusConflict, "There is not enough disk space available to perform that action."
-	}
-	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeDenylistFile) {
+	// Some external things end up calling fmt.Errorf() on our filesystem errors
+	// which ends up just unleashing chaos on the system. For the sake of this
+	// fallback to using text checks...
+	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeDenylistFile) || strings.Contains(e.err.Error(), "filesystem: file access prohibited") {
 		return http.StatusForbidden, "This file cannot be modified: present in egg denylist."
 	}
-	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeIsDirectory) {
+	if filesystem.IsErrorCode(e.err, filesystem.ErrCodePathResolution) || strings.Contains(e.err.Error(), "resolves to a location outside the server root") {
+		return http.StatusNotFound, "The requested resource was not found on the system."
+	}
+	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeIsDirectory) || strings.Contains(e.err.Error(), "filesystem: is a directory") {
+		return http.StatusBadRequest, "Cannot perform that action: file is a directory."
+	}
+	if filesystem.IsErrorCode(e.err, filesystem.ErrCodeDiskSpace) || strings.Contains(e.err.Error(), "filesystem: not enough disk space") {
 		return http.StatusBadRequest, "Cannot perform that action: file is a directory."
 	}
 	if strings.HasSuffix(e.err.Error(), "file name too long") {
diff --git a/router/router_server_files.go b/router/router_server_files.go
index bee077d..b6a3af5 100644
--- a/router/router_server_files.go
+++ b/router/router_server_files.go
@@ -410,13 +410,6 @@ func postServerDecompressFiles(c *gin.Context) {
 	}
 
 	if err := s.Filesystem().DecompressFile(data.RootPath, data.File); err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			c.AbortWithStatusJSON(http.StatusNotFound, gin.H{
-				"error": "The requested archive was not found.",
-			})
-			return
-		}
-
 		// If the file is busy for some reason just return a nicer error to the user since there is not
 		// much we specifically can do. They'll need to stop the running server process in order to overwrite
 		// a file like this.
@@ -429,7 +422,7 @@ func postServerDecompressFiles(c *gin.Context) {
 			return
 		}
 
-		NewServerError(err, s).AbortFilesystemError(c)
+		NewServerError(err, s).Abort(c)
 		return
 	}
 
@@ -548,14 +541,14 @@ func postServerUploadFiles(c *gin.Context) {
 	for _, header := range headers {
 		p, err := s.Filesystem().SafePath(filepath.Join(directory, header.Filename))
 		if err != nil {
-			NewServerError(err, s).AbortFilesystemError(c)
+			NewServerError(err, s).Abort(c)
 			return
 		}
 
 		// We run this in a different method so I can use defer without any of
 		// the consequences caused by calling it in a loop.
 		if err := handleFileUpload(p, s, header); err != nil {
-			NewServerError(err, s).AbortFilesystemError(c)
+			NewServerError(err, s).Abort(c)
 			return
 		}
 	}
diff --git a/server/filesystem/decompress.go b/server/filesystem/decompress.go
index 66d7901..4fb2988 100644
--- a/server/filesystem/decompress.go
+++ b/server/filesystem/decompress.go
@@ -92,8 +92,9 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error {
 		}
 
 		p := filepath.Join(dir, name)
+		// If it is ignored, just don't do anything with the file and skip over it.
 		if err := fs.IsIgnored(p); err != nil {
-			return err
+			return nil
 		}
 		return errors.WithMessage(fs.Writefile(p, f), "could not extract file from archive")
 	})