Skip to content

Commit

Permalink
Merge pull request #4749 from JammingBen/feat/dav-response-error-codes
Browse files Browse the repository at this point in the history
feat: add error codes to DAV error responses
  • Loading branch information
JammingBen authored Jul 5, 2024
2 parents b7bdbef + 4d59b28 commit fdd92bd
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 84 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/dav-error-codes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: DAV error codes

DAV error responses now include an error code for clients to use if they need to check for a specific error type.

https://github.com/cs3org/reva/pull/4749
https://github.com/owncloud/ocis/issues/9533
32 changes: 16 additions & 16 deletions internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)

if r.Body != http.NoBody {
w.WriteHeader(http.StatusUnsupportedMediaType)
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand All @@ -83,21 +83,21 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
dst, err := net.ParseDestination(baseURI, dh)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "")
b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}

if err := ValidateName(filename(src), s.nameValidators); err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "")
b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}

if err := ValidateDestination(filename(dst), s.nameValidators); err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "destination failed naming rules", "")
b, err := errors.Marshal(http.StatusBadRequest, "destination failed naming rules", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand Down Expand Up @@ -163,7 +163,7 @@ func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gate
if createRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED {
w.WriteHeader(http.StatusForbidden)
m := fmt.Sprintf("Permission denied to create %v", createReq.Ref.Path)
b, err := errors.Marshal(http.StatusForbidden, m, "")
b, err := errors.Marshal(http.StatusForbidden, m, "", "")
errors.HandleWebdavError(log, w, b, err)
}
return nil
Expand Down Expand Up @@ -264,7 +264,7 @@ func (s *svc) executePathCopy(ctx context.Context, selector pool.Selectable[gate
if uRes.Status.Code == rpc.Code_CODE_PERMISSION_DENIED {
w.WriteHeader(http.StatusForbidden)
m := fmt.Sprintf("Permissions denied to create %v", uReq.Ref.Path)
b, err := errors.Marshal(http.StatusForbidden, m, "")
b, err := errors.Marshal(http.StatusForbidden, m, "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand Down Expand Up @@ -327,7 +327,7 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s

if r.Body != http.NoBody {
w.WriteHeader(http.StatusUnsupportedMediaType)
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand Down Expand Up @@ -394,7 +394,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele
w.WriteHeader(http.StatusForbidden)
// TODO path could be empty or relative...
m := fmt.Sprintf("Permission denied to create %v", createReq.Ref.Path)
b, err := errors.Marshal(http.StatusForbidden, m, "")
b, err := errors.Marshal(http.StatusForbidden, m, "", "")
errors.HandleWebdavError(log, w, b, err)
}
return nil
Expand Down Expand Up @@ -482,7 +482,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, sele
w.WriteHeader(http.StatusForbidden)
// TODO path can be empty or relative
m := fmt.Sprintf("Permissions denied to create %v", uReq.Ref.Path)
b, err := errors.Marshal(http.StatusForbidden, m, "")
b, err := errors.Marshal(http.StatusForbidden, m, "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand Down Expand Up @@ -554,7 +554,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
}
if isChild {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into one of its children", "")
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into one of its children", "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand All @@ -577,15 +577,15 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re

if isParent {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into its parent", "")
b, err := errors.Marshal(http.StatusBadRequest, "can not copy a folder into its parent", "", "")
errors.HandleWebdavError(log, w, b, err)
return nil

}

if srcRef.Path == dstRef.Path && srcRef.ResourceId == dstRef.ResourceId {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "source and destination are the same", "")
b, err := errors.Marshal(http.StatusBadRequest, "source and destination are the same", "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand All @@ -595,7 +595,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
if err != nil {
w.WriteHeader(http.StatusBadRequest)
m := fmt.Sprintf("Overwrite header is set to incorrect value %v", overwrite)
b, err := errors.Marshal(http.StatusBadRequest, m, "")
b, err := errors.Marshal(http.StatusBadRequest, m, "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand All @@ -605,7 +605,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
if err != nil {
w.WriteHeader(http.StatusBadRequest)
m := fmt.Sprintf("Depth header is set to incorrect value %v", dh)
b, err := errors.Marshal(http.StatusBadRequest, m, "")
b, err := errors.Marshal(http.StatusBadRequest, m, "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
Expand Down Expand Up @@ -634,7 +634,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
case srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND:
errors.HandleErrorStatus(log, w, srcStatRes.Status)
m := fmt.Sprintf("Resource %v not found", srcStatReq.Ref.Path)
b, err := errors.Marshal(http.StatusNotFound, m, "")
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(log, w, b, err)
return nil
case srcStatRes.Status.Code != rpc.Code_CODE_OK:
Expand Down Expand Up @@ -672,7 +672,7 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
log.Warn().Bool("overwrite", overwrite).Msg("dst already exists")
w.WriteHeader(http.StatusPreconditionFailed)
m := fmt.Sprintf("Could not overwrite Resource %v", dstRef.Path)
b, err := errors.Marshal(http.StatusPreconditionFailed, m, "")
b, err := errors.Marshal(http.StatusPreconditionFailed, m, "", "")
errors.HandleWebdavError(log, w, b, err) // 412, see https://tools.ietf.org/html/rfc4918#section-9.8.5
return nil
}
Expand Down
18 changes: 13 additions & 5 deletions internal/http/services/owncloud/ocdav/dav.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ const (
WwwAuthenticate = "Www-Authenticate"
)

const (
ErrListingMembers = "ERR_LISTING_MEMBERS_NOT_ALLOWED"
ErrInvalidCredentials = "ERR_INVALID_CREDENTIALS"
ErrMissingBasicAuth = "ERR_MISSING_BASIC_AUTH"
ErrMissingBearerAuth = "ERR_MISSING_BEARER_AUTH"
ErrFileNotFoundInRoot = "ERR_FILE_NOT_FOUND_IN_ROOT"
)

// DavHandler routes to the different sub handlers
type DavHandler struct {
AvatarsHandler *AvatarsHandler
Expand Down Expand Up @@ -132,7 +140,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {

if r.Header.Get(net.HeaderDepth) == "" {
w.WriteHeader(http.StatusMethodNotAllowed)
b, err := errors.Marshal(http.StatusMethodNotAllowed, "Listing members of this collection is disabled", "")
b, err := errors.Marshal(http.StatusMethodNotAllowed, "Listing members of this collection is disabled", "", ErrListingMembers)
if err != nil {
log.Error().Msgf("error marshaling xml response: %s", b)
w.WriteHeader(http.StatusInternalServerError)
Expand Down Expand Up @@ -312,11 +320,11 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
case res.Status.Code == rpc.Code_CODE_UNAUTHENTICATED:
w.WriteHeader(http.StatusUnauthorized)
if hasValidBasicAuthHeader {
b, err := errors.Marshal(http.StatusUnauthorized, "Username or password was incorrect", "")
b, err := errors.Marshal(http.StatusUnauthorized, "Username or password was incorrect", "", ErrInvalidCredentials)
errors.HandleWebdavError(log, w, b, err)
return
}
b, err := errors.Marshal(http.StatusUnauthorized, "No 'Authorization: Basic' header found", "")
b, err := errors.Marshal(http.StatusUnauthorized, "No 'Authorization: Basic' header found", "", ErrMissingBasicAuth)
errors.HandleWebdavError(log, w, b, err)
return
case res.Status.Code == rpc.Code_CODE_NOT_FOUND:
Expand Down Expand Up @@ -358,7 +366,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {
if !userExists {
w.Header().Add(WwwAuthenticate, fmt.Sprintf("Bearer realm=\"%s\", charset=\"UTF-8\"", r.Host))
w.WriteHeader(http.StatusUnauthorized)
b, err := errors.Marshal(http.StatusUnauthorized, "No 'Authorization: Bearer' header found", "")
b, err := errors.Marshal(http.StatusUnauthorized, "No 'Authorization: Bearer' header found", "", ErrMissingBearerAuth)
errors.HandleWebdavError(log, w, b, err)
return
}
Expand Down Expand Up @@ -388,7 +396,7 @@ func (h *DavHandler) Handler(s *svc) http.Handler {

default:
w.WriteHeader(http.StatusNotFound)
b, err := errors.Marshal(http.StatusNotFound, "File not found in root", "")
b, err := errors.Marshal(http.StatusNotFound, "File not found in root", "", ErrFileNotFoundInRoot)
errors.HandleWebdavError(log, w, b, err)
}
})
Expand Down
4 changes: 3 additions & 1 deletion internal/http/services/owncloud/ocdav/errors/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ type Exception struct {
}

// Marshal just calls the xml marshaller for a given exception.
func Marshal(code int, message string, header string) ([]byte, error) {
func Marshal(code int, message string, header string, errorCode string) ([]byte, error) {
xmlstring, err := xml.Marshal(&ErrorXML{
Xmlnsd: "DAV",
Xmlnss: "http://sabredav.org/ns",
Exception: sabreException[code],
Message: message,
Header: header,
ErrorCode: errorCode,
})
if err != nil {
return nil, err
Expand All @@ -126,6 +127,7 @@ type ErrorXML struct {
Xmlnss string `xml:"xmlns:s,attr"`
Exception string `xml:"s:exception"`
Message string `xml:"s:message"`
ErrorCode string `xml:"s:errorcode"`
InnerXML []byte `xml:",innerxml"`
// Header is used to indicate the conflicting request header
Header string `xml:"s:header,omitempty"`
Expand Down
6 changes: 3 additions & 3 deletions internal/http/services/owncloud/ocdav/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (h *MetaHandler) Handler(s *svc) http.Handler {
logger.Debug().Str("prop", net.PropOcMetaPathForUser).Msg("invalid resource id")
w.WriteHeader(http.StatusBadRequest)
m := fmt.Sprintf("Invalid resource id %v", id)
b, err := errors.Marshal(http.StatusBadRequest, m, "")
b, err := errors.Marshal(http.StatusBadRequest, m, "", "")
errors.HandleWebdavError(logger, w, b, err)
return
}
Expand Down Expand Up @@ -139,15 +139,15 @@ func (h *MetaHandler) handlePathForUser(w http.ResponseWriter, r *http.Request,
sublog.Debug().Str("code", string(pathRes.Status.Code)).Msg("resource not found")
w.WriteHeader(http.StatusNotFound)
m := fmt.Sprintf("Resource %s not found", id)
b, err := errors.Marshal(http.StatusNotFound, m, "")
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(&sublog, w, b, err)
return
case rpc.Code_CODE_PERMISSION_DENIED:
// raise StatusNotFound so that resources can't be enumerated
sublog.Debug().Str("code", string(pathRes.Status.Code)).Msg("resource access denied")
w.WriteHeader(http.StatusNotFound)
m := fmt.Sprintf("Resource %s not found", id)
b, err := errors.Marshal(http.StatusNotFound, m, "")
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(&sublog, w, b, err)
return
}
Expand Down
18 changes: 9 additions & 9 deletions internal/http/services/owncloud/ocdav/move.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)

if r.Body != http.NoBody {
w.WriteHeader(http.StatusUnsupportedMediaType)
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand All @@ -55,21 +55,21 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)
dstPath, err := net.ParseDestination(baseURI, dh)
if err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "")
b, err := errors.Marshal(http.StatusBadRequest, "failed to extract destination", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}

if err := ValidateName(filename(srcPath), s.nameValidators); err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "")
b, err := errors.Marshal(http.StatusBadRequest, "source failed naming rules", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}

if err := ValidateDestination(filename(dstPath), s.nameValidators); err != nil {
w.WriteHeader(http.StatusBadRequest)
b, err := errors.Marshal(http.StatusBadRequest, "destination naming rules", "")
b, err := errors.Marshal(http.StatusBadRequest, "destination naming rules", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand Down Expand Up @@ -108,7 +108,7 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI

if r.Body != http.NoBody {
w.WriteHeader(http.StatusUnsupportedMediaType)
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "")
b, err := errors.Marshal(http.StatusUnsupportedMediaType, "body must be empty", "", "")
errors.HandleWebdavError(appctx.GetLogger(ctx), w, b, err)
return
}
Expand Down Expand Up @@ -157,7 +157,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
}
if isChild {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into one of its children", "")
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into one of its children", "", "")
errors.HandleWebdavError(&log, w, b, err)
return
}
Expand All @@ -179,7 +179,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
}
if isParent {
w.WriteHeader(http.StatusConflict)
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into its parent", "")
b, err := errors.Marshal(http.StatusBadRequest, "can not move a folder into its parent", "", "")
errors.HandleWebdavError(&log, w, b, err)
return

Expand Down Expand Up @@ -213,7 +213,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req
if srcStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND {
w.WriteHeader(http.StatusNotFound)
m := fmt.Sprintf("Resource %v not found", srcStatReq.Ref.Path)
b, err := errors.Marshal(http.StatusNotFound, m, "")
b, err := errors.Marshal(http.StatusNotFound, m, "", "")
errors.HandleWebdavError(&log, w, b, err)
}
errors.HandleErrorStatus(&log, w, srcStatRes.Status)
Expand Down Expand Up @@ -321,7 +321,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req

w.WriteHeader(status)

b, err := errors.Marshal(status, m, "")
b, err := errors.Marshal(status, m, "", "")
errors.HandleWebdavError(&log, w, b, err)
return
}
Expand Down
Loading

0 comments on commit fdd92bd

Please sign in to comment.