From 787f6b257f3ee34431e4f5a9aa33a5399096a78c Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Tue, 2 Jan 2024 08:48:55 +0300 Subject: [PATCH] chore: check against errors of `io/fs` instead of `os` (#6011) * chore: replace `os.ErrNotExist` with `fs.ErrNotExist` * check against permission error from `io/fs` package --- caddy.go | 3 ++- caddytest/caddytest.go | 3 ++- cmd/commandfuncs.go | 2 +- cmd/main.go | 3 ++- modules/caddyhttp/fileserver/browse.go | 5 ++-- modules/caddyhttp/fileserver/staticfiles.go | 4 +-- .../caddyhttp/templates/tplcontext_test.go | 25 ++++++++----------- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/caddy.go b/caddy.go index 4a7bd8b6d..cd8af5fe2 100644 --- a/caddy.go +++ b/caddy.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "io/fs" "log" "net/http" "os" @@ -828,7 +829,7 @@ func InstanceID() (uuid.UUID, error) { appDataDir := AppDataDir() uuidFilePath := filepath.Join(appDataDir, "instance.uuid") uuidFileBytes, err := os.ReadFile(uuidFilePath) - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { uuid, err := uuid.NewRandom() if err != nil { return uuid, err diff --git a/caddytest/caddytest.go b/caddytest/caddytest.go index 39aca2347..26d3de660 100644 --- a/caddytest/caddytest.go +++ b/caddytest/caddytest.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "io/fs" "log" "net" "net/http" @@ -232,7 +233,7 @@ const initConfig = `{ func validateTestPrerequisites(t *testing.T) error { // check certificates are found for _, certName := range Default.Certifcates { - if _, err := os.Stat(getIntegrationDir() + certName); os.IsNotExist(err) { + if _, err := os.Stat(getIntegrationDir() + certName); errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("caddy integration test certificates (%s) not found", certName) } } diff --git a/cmd/commandfuncs.go b/cmd/commandfuncs.go index b0c576a3d..746cf3da6 100644 --- a/cmd/commandfuncs.go +++ b/cmd/commandfuncs.go @@ -190,7 +190,7 @@ func cmdRun(fl Flags) (int, error) { var config []byte if resumeFlag { config, err = os.ReadFile(caddy.ConfigAutosavePath) - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { // not a bad error; just can't resume if autosave file doesn't exist caddy.Log().Info("no autosave file exists", zap.String("autosave_file", caddy.ConfigAutosavePath)) resumeFlag = false diff --git a/cmd/main.go b/cmd/main.go index fa15c0874..73d70f04d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -21,6 +21,7 @@ import ( "flag" "fmt" "io" + "io/fs" "log" "net" "os" @@ -128,7 +129,7 @@ func loadConfigWithLogger(logger *zap.Logger, configFile, adapterName string) ([ cfgAdapter = caddyconfig.GetAdapter("caddyfile") if cfgAdapter != nil { config, err = os.ReadFile("Caddyfile") - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { // okay, no default Caddyfile; pretend like this never happened cfgAdapter = nil } else if err != nil { diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 81eb08592..492d52d65 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -19,6 +19,7 @@ import ( "context" _ "embed" "encoding/json" + "errors" "fmt" "io" "io/fs" @@ -92,9 +93,9 @@ func (fsrv *FileServer) serveBrowse(root, dirPath string, w http.ResponseWriter, // TODO: not entirely sure if path.Clean() is necessary here but seems like a safe plan (i.e. /%2e%2e%2f) - someone could verify this listing, err := fsrv.loadDirectoryContents(r.Context(), dir.(fs.ReadDirFile), root, path.Clean(r.URL.EscapedPath()), repl) switch { - case os.IsPermission(err): + case errors.Is(err, fs.ErrPermission): return caddyhttp.Error(http.StatusForbidden, err) - case os.IsNotExist(err): + case errors.Is(err, fs.ErrNotExist): return fsrv.notFound(w, r, next) case err != nil: return caddyhttp.Error(http.StatusInternalServerError, err) diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 0ed558e8b..aa582eac0 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -506,10 +506,10 @@ func (fsrv *FileServer) openFile(filename string, w http.ResponseWriter) (fs.Fil file, err := fsrv.fileSystem.Open(filename) if err != nil { err = fsrv.mapDirOpenError(err, filename) - if os.IsNotExist(err) { + if errors.Is(err, fs.ErrNotExist) { fsrv.logger.Debug("file not found", zap.String("filename", filename), zap.Error(err)) return nil, caddyhttp.Error(http.StatusNotFound, err) - } else if os.IsPermission(err) { + } else if errors.Is(err, fs.ErrPermission) { fsrv.logger.Debug("permission denied", zap.String("filename", filename), zap.Error(err)) return nil, caddyhttp.Error(http.StatusForbidden, err) } diff --git a/modules/caddyhttp/templates/tplcontext_test.go b/modules/caddyhttp/templates/tplcontext_test.go index fdf2c1065..67ebbac70 100644 --- a/modules/caddyhttp/templates/tplcontext_test.go +++ b/modules/caddyhttp/templates/tplcontext_test.go @@ -17,7 +17,9 @@ package templates import ( "bytes" "context" + "errors" "fmt" + "io/fs" "net/http" "os" "path/filepath" @@ -30,8 +32,7 @@ import ( "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) -type handle struct { -} +type handle struct{} func (h *handle) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Header.Get("Accept-Encoding") == "identity" { @@ -176,11 +177,10 @@ func TestImport(t *testing.T) { } else if !templateWasDefined && actual != "" { // template should be defined, return value should be an empty string t.Errorf("Test %d: Expected template %s to be define but got %s", i, test.expect, tplContext.tpl.DefinedTemplates()) - } if absFilePath != "" { - if err := os.Remove(absFilePath); err != nil && !os.IsNotExist(err) { + if err := os.Remove(absFilePath); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test file, got: %v", i, err) } } @@ -255,21 +255,20 @@ func TestNestedInclude(t *testing.T) { } else if buf.String() != test.expect { // t.Errorf("Test %d: Expected '%s' but got '%s'", i, test.expect, buf.String()) - } if absFilePath != "" { - if err := os.Remove(absFilePath); err != nil && !os.IsNotExist(err) { + if err := os.Remove(absFilePath); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test file, got: %v", i, err) } } if absFilePath0 != "" { - if err := os.Remove(absFilePath0); err != nil && !os.IsNotExist(err) { + if err := os.Remove(absFilePath0); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test file, got: %v", i, err) } } if absFilePath1 != "" { - if err := os.Remove(absFilePath1); err != nil && !os.IsNotExist(err) { + if err := os.Remove(absFilePath1); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test file, got: %v", i, err) } } @@ -342,11 +341,10 @@ func TestInclude(t *testing.T) { t.Errorf("Test %d: Expected error but had none", i) } else if actual != test.expect { t.Errorf("Test %d: Expected %s but got %s", i, test.expect, actual) - } if absFilePath != "" { - if err := os.Remove(absFilePath); err != nil && !os.IsNotExist(err) { + if err := os.Remove(absFilePath); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test file, got: %v", i, err) } } @@ -460,7 +458,7 @@ func TestFileListing(t *testing.T) { fileNames: nil, inputBase: "doesNotExist", shouldErr: true, - verifyErr: os.IsNotExist, + verifyErr: func(err error) bool { return errors.Is(err, fs.ErrNotExist) }, }, { // directory and files exist, but path to a file @@ -476,7 +474,7 @@ func TestFileListing(t *testing.T) { fileNames: nil, inputBase: filepath.Join("..", "..", "..", "..", "..", "etc"), shouldErr: true, - verifyErr: os.IsNotExist, + verifyErr: func(err error) bool { return errors.Is(err, fs.ErrNotExist) }, }, } { tplContext := getContextOrFail(t) @@ -525,7 +523,7 @@ func TestFileListing(t *testing.T) { } if dirPath != "" { - if err := os.RemoveAll(dirPath); err != nil && !os.IsNotExist(err) { + if err := os.RemoveAll(dirPath); err != nil && !errors.Is(err, fs.ErrNotExist) { t.Fatalf("Test %d: Expected no error removing temporary test directory, got: %v", i, err) } } @@ -602,7 +600,6 @@ title = "Welcome" t.Errorf("Test %d: Expected body %s, found %s. Input was SplitFrontMatter(%s)", i, test.body, result.Body, test.input) } } - } func TestHumanize(t *testing.T) {