From 79892d09129f22fb3dc12328ef0edcc5a327f395 Mon Sep 17 00:00:00 2001 From: Abhishek Shroff Date: Tue, 15 Oct 2024 23:45:30 +0530 Subject: [PATCH] [server][webdav] Stop relying on os.ErrXXX --- server/internal/api/errors/errors.go | 2 +- server/internal/core/errors.go | 3 ++- server/internal/webdav/handler.go | 27 ++++++++++++++++++--------- server/internal/webdav/impl/file.go | 22 ++++++++++++---------- server/internal/webdav/impl/webdav.go | 8 ++++---- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/server/internal/api/errors/errors.go b/server/internal/api/errors/errors.go index f61b39ff..f563969b 100644 --- a/server/internal/api/errors/errors.go +++ b/server/internal/api/errors/errors.go @@ -21,7 +21,7 @@ func New(httpStatus int, code, msg string) error { } } -func (e *ApiErr) Error() string { +func (e ApiErr) Error() string { return fmt.Sprintf("%d %s (%s)", e.Status, e.Code, e.Message) } diff --git a/server/internal/core/errors.go b/server/internal/core/errors.go index 1cee4607..268030bd 100644 --- a/server/internal/core/errors.go +++ b/server/internal/core/errors.go @@ -16,6 +16,7 @@ var ( ErrResourceCollection = errors.New(http.StatusMethodNotAllowed, "cannot write to collection resource", "") ErrResourceIDConflict = errors.New(http.StatusConflict, "resource_id_conflict", "") ErrResourceNameInvalid = errors.New(http.StatusBadRequest, "resource_name_invalid", "") - ErrResourceNameConflict = errors.New(http.StatusConflict, "resource_name_conflict", "") + ErrResourceNameConflict = errors.New(http.StatusPreconditionFailed, "resource_name_conflict", "") ErrResourceMoveTargetSubdirectory = errors.New(http.StatusConflict, "move target subdirectory", "") + ErrParentNotFound = errors.New(http.StatusConflict, "parent_not_found", "") ) diff --git a/server/internal/webdav/handler.go b/server/internal/webdav/handler.go index 6b4adc8b..c0603b51 100644 --- a/server/internal/webdav/handler.go +++ b/server/internal/webdav/handler.go @@ -2,8 +2,8 @@ package webdav import ( "context" + "errors" "io" - "io/fs" "net/http" "strings" @@ -102,22 +102,28 @@ func (a adapter) OpenWrite(ctx context.Context, name string) (io.WriteCloser, er func (a adapter) RemoveAll(ctx context.Context, name string) error { resource, err := a.Stat(ctx, name) if err != nil { - return fs.ErrNotExist + return err } _, err = a.fs.DeleteRecursive(resource, false) return err } func (a adapter) Mkdir(ctx context.Context, name string) error { - if _, err := a.Stat(ctx, name); err == nil { - return fs.ErrExist + if _, err := a.Stat(ctx, name); !errors.Is(err, core.ErrResourceNotFound) { + if err == nil { + return core.ErrResourceNameConflict + } + return err } name = strings.TrimRight(name, "/") index := strings.LastIndex(name, "/") parentPath := name[0:index] parent, err := a.Stat(ctx, parentPath) if err != nil { - return fs.ErrNotExist + if errors.Is(err, core.ErrResourceNotFound) { + return core.ErrParentNotFound + } + return err } dirName := name[index+1:] _, err = a.fs.CreateMemberResource(parent, uuid.New(), dirName, true) @@ -127,22 +133,25 @@ func (a adapter) Mkdir(ctx context.Context, name string) error { func (a adapter) Rename(ctx context.Context, oldName, newName string) error { src, err := a.Stat(ctx, oldName) if err != nil { - return fs.ErrNotExist + return core.ErrResourceNotFound } _, err = a.Stat(ctx, newName) if err == nil { - return fs.ErrExist + return core.ErrResourceNameConflict } newName = strings.TrimRight(newName, "/") index := strings.LastIndex(newName, "/") parentPath := newName[0:index] - parent, err := a.Stat(ctx, parentPath) newName = newName[index+1:] + parent, err := a.Stat(ctx, parentPath) if err != nil { - return fs.ErrNotExist + if errors.Is(err, core.ErrResourceNotFound) { + return core.ErrParentNotFound + } + return err } parentID := parent.ID diff --git a/server/internal/webdav/impl/file.go b/server/internal/webdav/impl/file.go index 1a099018..9bd91771 100644 --- a/server/internal/webdav/impl/file.go +++ b/server/internal/webdav/impl/file.go @@ -6,13 +6,12 @@ package webdav import ( "context" - "errors" "io" "net/http" - "os" "path" "path/filepath" + "github.com/shroff/phylum/server/internal/api/errors" "github.com/shroff/phylum/server/internal/core" ) @@ -35,7 +34,10 @@ type FileSystem interface { func moveFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bool) (status int, err error) { created := false if _, err := fs.Stat(ctx, dst); err != nil { - if !errors.Is(err, os.ErrNotExist) { + if !errors.Is(err, core.ErrResourceNotFound) { + if e, ok := err.(errors.ApiErr); ok { + return e.Status, e + } return http.StatusForbidden, err } created = true @@ -48,7 +50,7 @@ func moveFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bo return http.StatusForbidden, err } } else { - return http.StatusPreconditionFailed, os.ErrExist + return http.StatusPreconditionFailed, core.ErrResourceNameConflict } if err := fs.Rename(ctx, src, dst); err != nil { return http.StatusForbidden, err @@ -73,7 +75,7 @@ func copyFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bo srcStat, err := fs.Stat(ctx, src) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusNotFound, err } return http.StatusInternalServerError, err @@ -81,16 +83,16 @@ func copyFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bo created := false if _, err := fs.Stat(ctx, dst); err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { created = true } else { return http.StatusForbidden, err } } else { if !overwrite { - return http.StatusPreconditionFailed, os.ErrExist + return http.StatusPreconditionFailed, core.ErrResourceNameConflict } - if err := fs.RemoveAll(ctx, dst); err != nil && !errors.Is(err, os.ErrNotExist) { + if err := fs.RemoveAll(ctx, dst); err != nil && !errors.Is(err, core.ErrResourceNotFound) { return http.StatusForbidden, err } } @@ -119,7 +121,7 @@ func copyFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bo } else { srcFile, err := fs.OpenRead(srcStat, 0, -1) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusNotFound, err } return http.StatusInternalServerError, err @@ -127,7 +129,7 @@ func copyFiles(ctx context.Context, fs FileSystem, src, dst string, overwrite bo defer srcFile.Close() dstFile, err := fs.OpenWrite(ctx, dst) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusConflict, err } return http.StatusForbidden, err diff --git a/server/internal/webdav/impl/webdav.go b/server/internal/webdav/impl/webdav.go index 829011f9..e2c19ffc 100644 --- a/server/internal/webdav/impl/webdav.go +++ b/server/internal/webdav/impl/webdav.go @@ -234,7 +234,7 @@ func (h *Handler) handleDelete(_ http.ResponseWriter, r *http.Request) (status i // returns nil (no error)." WebDAV semantics are that it should return a // "404 Not Found". We therefore have to Stat before we RemoveAll. if _, err := h.FileSystem.Stat(ctx, reqPath); err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusNotFound, err } return http.StatusMethodNotAllowed, err @@ -297,7 +297,7 @@ func (h *Handler) handleMkcol(_ http.ResponseWriter, r *http.Request) (status in return http.StatusUnsupportedMediaType, nil } if err := h.FileSystem.Mkdir(ctx, reqPath); err != nil { - if errors.Is(err, os.ErrExist) { + if errors.Is(err, core.ErrResourceNameConflict) { return http.StatusConflict, err } return http.StatusMethodNotAllowed, err @@ -505,7 +505,7 @@ func (h *Handler) handlePropfind(w http.ResponseWriter, r *http.Request) (status ctx := r.Context() fi, err := h.FileSystem.Stat(ctx, reqPath) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusNotFound, err } return http.StatusMethodNotAllowed, err @@ -580,7 +580,7 @@ func (h *Handler) handleProppatch(w http.ResponseWriter, r *http.Request) (statu ctx := r.Context() if _, err := h.FileSystem.Stat(ctx, reqPath); err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, core.ErrResourceNotFound) { return http.StatusNotFound, err } return http.StatusMethodNotAllowed, err