From f374ded926f65c3f21b22d87b2bcbf925a1c756d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 2 Dec 2020 08:09:43 +0000 Subject: [PATCH] OCDAV: map bad request and unimplemented codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/ocdav-handle-bad-request.md | 5 + internal/http/services/owncloud/ocdav/copy.go | 53 +++----- .../http/services/owncloud/ocdav/delete.go | 21 +-- .../http/services/owncloud/ocdav/error.go | 27 ++++ internal/http/services/owncloud/ocdav/get.go | 44 ++---- internal/http/services/owncloud/ocdav/head.go | 22 ++- .../http/services/owncloud/ocdav/mkcol.go | 34 ++--- internal/http/services/owncloud/ocdav/move.go | 89 +++--------- .../http/services/owncloud/ocdav/propfind.go | 55 ++------ .../http/services/owncloud/ocdav/proppatch.go | 53 ++------ .../services/owncloud/ocdav/publicfile.go | 68 ++++------ internal/http/services/owncloud/ocdav/put.go | 63 +++------ .../http/services/owncloud/ocdav/trashbin.go | 127 +++++------------- internal/http/services/owncloud/ocdav/tus.go | 57 +++----- .../http/services/owncloud/ocdav/versions.go | 67 ++++----- 15 files changed, 267 insertions(+), 518 deletions(-) create mode 100644 changelog/unreleased/ocdav-handle-bad-request.md diff --git a/changelog/unreleased/ocdav-handle-bad-request.md b/changelog/unreleased/ocdav-handle-bad-request.md new file mode 100644 index 00000000000..021eeff6a72 --- /dev/null +++ b/changelog/unreleased/ocdav-handle-bad-request.md @@ -0,0 +1,5 @@ +Enhancement: Map bad request and unimplement to http status codes + +We now return a 400 bad request when a grpc call fails with an invalid argument status and a 501 not implemented when it fails with an unimplemented status. This prevents 500 errors when a user tries to add resources to the Share folder or a storage does not implement an action. + +https://github.com/cs3org/reva/pull/1354 diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index f8a350bf5c1..aad7fdeb4a9 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -32,11 +32,13 @@ import ( "github.com/cs3org/reva/internal/http/services/datagateway" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rhttp" + "go.opencensus.io/trace" ) func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "head") + defer span.End() ns = applyLayout(ctx, ns) @@ -55,8 +57,8 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { } dst = path.Join(ns, dst) - log.Info().Str("source", src).Str("destination", dst). - Str("overwrite", overwrite).Str("depth", depth).Msg("copy") + sublog := appctx.GetLogger(ctx).With().Str("src", src).Str("dst", dst).Logger() + sublog.Debug().Str("overwrite", overwrite).Str("depth", depth).Msg("copy") overwrite = strings.ToUpper(overwrite) if overwrite == "" { @@ -75,7 +77,7 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -87,23 +89,13 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { srcStatReq := &provider.StatRequest{Ref: ref} srcStatRes, err := client.Stat(ctx, srcStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if srcStatRes.Status.Code != rpc.Code_CODE_OK { - switch srcStatRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("src", src).Interface("status", srcStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, srcStatRes.Status) return } @@ -114,19 +106,12 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { dstStatReq := &provider.StatRequest{Ref: ref} dstStatRes, err := client.Stat(ctx, dstStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch dstStatRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, srcStatRes.Status) return } @@ -135,7 +120,7 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.8.5 if overwrite == "F" { - log.Warn().Str("dst", dst).Msg("dst already exists") + sublog.Warn().Str("overwrite", overwrite).Msg("dst already exists") w.WriteHeader(http.StatusPreconditionFailed) // 412, see https://tools.ietf.org/html/rfc4918#section-9.8.5 return } @@ -149,21 +134,17 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { intStatReq := &provider.StatRequest{Ref: ref} intStatRes, err := client.Stat(ctx, intStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if intStatRes.Status.Code != rpc.Code_CODE_OK { - switch intStatRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: + if intStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.8.5 + sublog.Debug().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("conflict") w.WriteHeader(http.StatusConflict) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("dst", dst).Interface("status", intStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("dst", dst).Interface("status", intStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) + } else { + handleErrorStatus(&sublog, w, srcStatRes.Status) } return } @@ -172,7 +153,7 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) { err = s.descend(ctx, client, srcStatRes.Info, dst, depth == "infinity") if err != nil { - log.Error().Err(err).Msg("error descending directory") + sublog.Error().Err(err).Str("depth", depth).Msg("error descending directory") w.WriteHeader(http.StatusInternalServerError) return } diff --git a/internal/http/services/owncloud/ocdav/delete.go b/internal/http/services/owncloud/ocdav/delete.go index 4cbeb94cb13..b606b108012 100644 --- a/internal/http/services/owncloud/ocdav/delete.go +++ b/internal/http/services/owncloud/ocdav/delete.go @@ -25,19 +25,24 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" + "github.com/rs/zerolog/log" + "go.opencensus.io/trace" ) func (s *svc) handleDelete(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "head") + defer span.End() ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -48,7 +53,7 @@ func (s *svc) handleDelete(w http.ResponseWriter, r *http.Request, ns string) { req := &provider.DeleteRequest{Ref: ref} res, err := client.Delete(ctx, req) if err != nil { - log.Error().Err(err).Msg("error performing delete grpc request") + sublog.Error().Err(err).Msg("error performing delete grpc request") w.WriteHeader(http.StatusInternalServerError) return } @@ -57,13 +62,9 @@ func (s *svc) handleDelete(w http.ResponseWriter, r *http.Request, ns string) { case rpc.Code_CODE_OK: w.WriteHeader(http.StatusNoContent) case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) + log.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("resource not found") + w.WriteHeader(http.StatusConflict) default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc delete request failed") - w.WriteHeader(http.StatusInternalServerError) + handleErrorStatus(&sublog, w, res.Status) } } diff --git a/internal/http/services/owncloud/ocdav/error.go b/internal/http/services/owncloud/ocdav/error.go index ac993f4d7d1..4d97c97e32c 100644 --- a/internal/http/services/owncloud/ocdav/error.go +++ b/internal/http/services/owncloud/ocdav/error.go @@ -20,6 +20,10 @@ package ocdav import ( "encoding/xml" + "net/http" + + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + "github.com/rs/zerolog" ) type code int @@ -49,3 +53,26 @@ func Marshal(e exception) ([]byte, error) { Message: e.message, }) } + +func handleErrorStatus(log *zerolog.Logger, w http.ResponseWriter, s *rpc.Status) { + switch s.Code { + case rpc.Code_CODE_OK: + log.Debug().Interface("status", s).Msg("ok") + w.WriteHeader(http.StatusOK) + case rpc.Code_CODE_NOT_FOUND: + log.Debug().Interface("status", s).Msg("resource not found") + w.WriteHeader(http.StatusNotFound) + case rpc.Code_CODE_PERMISSION_DENIED: + log.Debug().Interface("status", s).Msg("permission denied") + w.WriteHeader(http.StatusForbidden) + case rpc.Code_CODE_INVALID_ARGUMENT: + log.Debug().Interface("status", s).Msg("bad request") + w.WriteHeader(http.StatusBadRequest) + case rpc.Code_CODE_UNIMPLEMENTED: + log.Debug().Interface("status", s).Msg("not implemented") + w.WriteHeader(http.StatusNotImplemented) + default: + log.Error().Interface("status", s).Msg("grpc request failed") + w.WriteHeader(http.StatusInternalServerError) + } +} diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index 806dede12f1..04b294b5caa 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/internal/http/services/owncloud/ocdav/get.go @@ -26,6 +26,7 @@ import ( "time" "github.com/cs3org/reva/internal/http/services/datagateway" + "go.opencensus.io/trace" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -36,15 +37,18 @@ import ( func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "head") + defer span.End() ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -56,29 +60,19 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { } sRes, err := client.Stat(ctx, sReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK { - switch sRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } info := sRes.Info if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { - log.Warn().Msg("resource is a folder and cannot be downloaded") + sublog.Warn().Msg("resource is a folder and cannot be downloaded") w.WriteHeader(http.StatusNotImplemented) return } @@ -91,23 +85,13 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { dRes, err := client.InitiateFileDownload(ctx, dReq) if err != nil { - log.Error().Err(err).Msg("error initiating file download") + sublog.Error().Err(err).Msg("error initiating file download") w.WriteHeader(http.StatusInternalServerError) return } if dRes.Status.Code != rpc.Code_CODE_OK { - switch dRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", dRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", dRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", dRes.Status).Msg("grpc initiate file download request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, dRes.Status) return } @@ -120,7 +104,7 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { httpReq, err := rhttp.NewRequest(ctx, "GET", ep, nil) if err != nil { - log.Error().Err(err).Msg("error creating http request") + sublog.Error().Err(err).Msg("error creating http request") w.WriteHeader(http.StatusInternalServerError) return } @@ -129,7 +113,7 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { httpRes, err := httpClient.Do(httpReq) if err != nil { - log.Error().Err(err).Msg("error performing http request") + sublog.Error().Err(err).Msg("error performing http request") w.WriteHeader(http.StatusInternalServerError) return } @@ -156,6 +140,6 @@ func (s *svc) handleGet(w http.ResponseWriter, r *http.Request, ns string) { } */ if _, err := io.Copy(w, httpRes.Body); err != nil { - log.Error().Err(err).Msg("error finishing copying data to response") + sublog.Error().Err(err).Msg("error finishing copying data to response") } } diff --git a/internal/http/services/owncloud/ocdav/head.go b/internal/http/services/owncloud/ocdav/head.go index d81452b5774..a0444bf0b06 100644 --- a/internal/http/services/owncloud/ocdav/head.go +++ b/internal/http/services/owncloud/ocdav/head.go @@ -28,19 +28,23 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/utils" + "go.opencensus.io/trace" ) func (s *svc) handleHead(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "head") + defer span.End() ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -51,23 +55,13 @@ func (s *svc) handleHead(w http.ResponseWriter, r *http.Request, ns string) { req := &provider.StatRequest{Ref: ref} res, err := client.Stat(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index f167ea88225..a5cc97f331a 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -26,27 +26,31 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" + "go.opencensus.io/trace" ) func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "mkcol") + defer span.End() ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + buf := make([]byte, 1) _, err := r.Body.Read(buf) if err != io.EOF { - log.Error().Err(err).Msg("error reading request body") + sublog.Error().Err(err).Msg("error reading request body") w.WriteHeader(http.StatusUnsupportedMediaType) return } client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -58,21 +62,16 @@ func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { statReq := &provider.StatRequest{Ref: ref} statRes, err := client.Stat(ctx, statReq) if err != nil { - log.Error().Err(err).Msg("error sending a grpc stat request") + sublog.Error().Err(err).Msg("error sending a grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if statRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch statRes.Status.Code { - case rpc.Code_CODE_OK: + if statRes.Status.Code == rpc.Code_CODE_OK { w.WriteHeader(http.StatusMethodNotAllowed) // 405 if it already exists - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) + } else { + handleErrorStatus(&sublog, w, statRes.Status) } return } @@ -80,22 +79,17 @@ func (s *svc) handleMkcol(w http.ResponseWriter, r *http.Request, ns string) { req := &provider.CreateContainerRequest{Ref: ref} res, err := client.CreateContainer(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending create container grpc request") + sublog.Error().Err(err).Msg("error sending create container grpc request") w.WriteHeader(http.StatusInternalServerError) return } - switch res.Status.Code { case rpc.Code_CODE_OK: w.WriteHeader(http.StatusCreated) case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("resource not found") + sublog.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("conflict") w.WriteHeader(http.StatusConflict) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) default: - log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc create container request failed") - w.WriteHeader(http.StatusInternalServerError) + handleErrorStatus(&sublog, w, res.Status) } } diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index cc94cdd2859..75e4d0d5f41 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -26,11 +26,13 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" + "go.opencensus.io/trace" ) func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "move") + defer span.End() ns = applyLayout(ctx, ns) @@ -45,7 +47,8 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { } dst = path.Join(ns, dst) - log.Info().Str("src", src).Str("dst", dst).Str("overwrite", overwrite).Msg("move") + sublog := appctx.GetLogger(ctx).With().Str("src", src).Str("dst", dst).Logger() + sublog.Debug().Str("overwrite", overwrite).Msg("move") overwrite = strings.ToUpper(overwrite) if overwrite == "" { @@ -59,7 +62,7 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -72,22 +75,12 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { } srcStatRes, err := client.Stat(ctx, srcStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if srcStatRes.Status.Code != rpc.Code_CODE_OK { - switch srcStatRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("src", src).Interface("status", srcStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("src", src).Interface("status", srcStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, srcStatRes.Status) return } @@ -98,19 +91,12 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { dstStatReq := &provider.StatRequest{Ref: dstStatRef} dstStatRes, err := client.Stat(ctx, dstStatReq) if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } if dstStatRes.Status.Code != rpc.Code_CODE_OK && dstStatRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch dstStatRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, srcStatRes.Status) return } @@ -119,7 +105,7 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { successCode = http.StatusNoContent // 204 if target already existed, see https://tools.ietf.org/html/rfc4918#section-9.9.4 if overwrite == "F" { - log.Warn().Str("dst", dst).Msg("dst already exists") + sublog.Warn().Str("overwrite", overwrite).Msg("dst already exists") w.WriteHeader(http.StatusPreconditionFailed) // 412, see https://tools.ietf.org/html/rfc4918#section-9.9.4 return } @@ -128,20 +114,13 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { delReq := &provider.DeleteRequest{Ref: dstStatRef} delRes, err := client.Delete(ctx, delReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc delete request") + sublog.Error().Err(err).Msg("error sending grpc delete request") w.WriteHeader(http.StatusInternalServerError) return } if delRes.Status.Code != rpc.Code_CODE_OK && delRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch delRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("dst", dst).Interface("status", delRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("dst", dst).Interface("status", delRes.Status).Msg("grpc delete request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, delRes.Status) return } } else { @@ -153,21 +132,17 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { intStatReq := &provider.StatRequest{Ref: ref2} intStatRes, err := client.Stat(ctx, intStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if intStatRes.Status.Code != rpc.Code_CODE_OK { - switch intStatRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: + if intStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND { // 409 if intermediate dir is missing, see https://tools.ietf.org/html/rfc4918#section-9.8.5 + sublog.Debug().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("conflict") w.WriteHeader(http.StatusConflict) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("parent", intermediateDir).Interface("status", intStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) + } else { + handleErrorStatus(&sublog, w, intStatRes.Status) } return } @@ -183,45 +158,25 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) { mReq := &provider.MoveRequest{Source: sourceRef, Destination: dstRef} mRes, err := client.Move(ctx, mReq) if err != nil { - log.Error().Err(err).Msg("error sending move grpc request") + sublog.Error().Err(err).Msg("error sending move grpc request") w.WriteHeader(http.StatusInternalServerError) return } if mRes.Status.Code != rpc.Code_CODE_OK { - switch mRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("src", src).Str("dst", dst).Interface("status", mRes.Status).Msg("grpc move request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, mRes.Status) return } dstStatRes, err = client.Stat(ctx, dstStatReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if dstStatRes.Status.Code != rpc.Code_CODE_OK { - switch dstStatRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("dst", dst).Interface("status", dstStatRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, dstStatRes.Status) return } diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 57fecfebf48..2dc321e79ec 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -46,7 +46,6 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) ctx := r.Context() ctx, span := trace.StartSpan(ctx, "propfind") defer span.End() - log := appctx.GetLogger(ctx) ns = applyLayout(ctx, ns) @@ -56,23 +55,25 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) depth = "1" } + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + // see https://tools.ietf.org/html/rfc4918#section-9.1 if depth != "0" && depth != "1" && depth != "infinity" { - log.Error().Msgf("invalid Depth header value %s", depth) + sublog.Debug().Str("depth", depth).Msgf("invalid Depth header value") w.WriteHeader(http.StatusBadRequest) return } pf, status, err := readPropfind(r.Body) if err != nil { - log.Error().Err(err).Msg("error reading propfind request") + sublog.Debug().Err(err).Msg("error reading propfind request") w.WriteHeader(status) return } client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -83,23 +84,13 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) req := &provider.StatRequest{Ref: ref} res, err := client.Stat(ctx, req) if err != nil { - log.Error().Err(err).Msgf("error sending a grpc stat request to ref: %v", ref) + sublog.Error().Err(err).Msgf("error sending a grpc stat request to ref: %v", ref) w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusMultiStatus) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } @@ -111,23 +102,13 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } res, err := client.ListContainer(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending list container grpc request") + sublog.Error().Err(err).Msg("error sending list container grpc request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc list container request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } infos = append(infos, res.Infos...) @@ -146,22 +127,12 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } res, err := client.ListContainer(ctx, req) if err != nil { - log.Error().Err(err).Str("path", path).Msg("error sending list container grpc request") + sublog.Error().Err(err).Str("path", path).Msg("error sending list container grpc request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc list container request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } @@ -188,7 +159,7 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) propRes, err := s.formatPropfind(ctx, &pf, infos, ns) if err != nil { - log.Error().Err(err).Msg("error formatting propfind") + sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } @@ -203,7 +174,7 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } w.WriteHeader(http.StatusMultiStatus) if _, err := w.Write([]byte(propRes)); err != nil { - log.Err(err).Msg("error writing response") + sublog.Err(err).Msg("error writing response") } } diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 60e2492e772..d957881f98f 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -40,7 +40,6 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) ctx := r.Context() ctx, span := trace.StartSpan(ctx, "proppatch") defer span.End() - log := appctx.GetLogger(ctx) acceptedProps := []xml.Name{} removedProps := []xml.Name{} @@ -49,16 +48,18 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + pp, status, err := readProppatch(r.Body) if err != nil { - log.Error().Err(err).Msg("error reading proppatch") + sublog.Debug().Err(err).Msg("error reading proppatch") w.WriteHeader(status) return } c, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -71,23 +72,13 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) } statRes, err := c.Stat(ctx, statReq) if err != nil { - log.Error().Err(err).Msg("error sending a grpc stat request") + sublog.Error().Err(err).Msg("error sending a grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if statRes.Status.Code != rpc.Code_CODE_OK { - switch statRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", statRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", statRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, statRes.Status) return } @@ -131,23 +122,13 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) rreq.ArbitraryMetadataKeys[0] = key res, err := c.UnsetArbitraryMetadata(ctx, rreq) if err != nil { - log.Error().Err(err).Msg("error sending a grpc UnsetArbitraryMetadata request") + sublog.Error().Err(err).Msg("error sending a grpc UnsetArbitraryMetadata request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc unset arbitrary metadata request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } removedProps = append(removedProps, propNameXML) @@ -155,23 +136,13 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) sreq.ArbitraryMetadata.Metadata[key] = value res, err := c.SetArbitraryMetadata(ctx, sreq) if err != nil { - log.Error().Err(err).Str("key", key).Str("value", value).Msg("error sending a grpc SetArbitraryMetadata request") + sublog.Error().Err(err).Str("key", key).Str("value", value).Msg("error sending a grpc SetArbitraryMetadata request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc set arbitrary metadata request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } @@ -192,7 +163,7 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) propRes, err := s.formatProppatchResponse(ctx, acceptedProps, removedProps, ref) if err != nil { - log.Error().Err(err).Msg("error formatting proppatch response") + sublog.Error().Err(err).Msg("error formatting proppatch response") w.WriteHeader(http.StatusInternalServerError) return } @@ -200,7 +171,7 @@ func (s *svc) handleProppatch(w http.ResponseWriter, r *http.Request, ns string) w.Header().Set("Content-Type", "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) if _, err := w.Write([]byte(propRes)); err != nil { - log.Err(err).Msg("error writing response") + sublog.Err(err).Msg("error writing response") } } diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index 9dc0c228e2e..d13931e89f7 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -87,12 +87,16 @@ func (h *PublicFileHandler) Handler(s *svc) http.Handler { func (s *svc) adjustResourcePathInURL(w http.ResponseWriter, r *http.Request) bool { ctx := r.Context() + ctx, span := trace.StartSpan(ctx, "adjustResourcePathInURL") + defer span.End() + // find actual file name - log := appctx.GetLogger(ctx) tokenStatInfo := ctx.Value(tokenStatInfoKey{}).(*provider.ResourceInfo) + sublog := appctx.GetLogger(ctx).With().Interface("tokenStatInfo", tokenStatInfo).Logger() + client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return false } @@ -100,28 +104,20 @@ func (s *svc) adjustResourcePathInURL(w http.ResponseWriter, r *http.Request) bo ResourceId: tokenStatInfo.GetId(), }) if err != nil { - log.Warn(). - Str("tokenStatInfo.Id", tokenStatInfo.GetId().String()). - Str("tokenStatInfo.Path", tokenStatInfo.Path). - Msg("Could not get path of resource") - w.WriteHeader(http.StatusNotFound) + sublog.Error().Msg("Could not get path of resource") + w.WriteHeader(http.StatusInternalServerError) return false } if pathRes.Status.Code != rpc.Code_CODE_OK { - switch pathRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - w.WriteHeader(http.StatusNotFound) - return false - case rpc.Code_CODE_PERMISSION_DENIED: - w.WriteHeader(http.StatusForbidden) - return false - default: - w.WriteHeader(http.StatusInternalServerError) - return false - } + handleErrorStatus(&sublog, w, pathRes.Status) + return false } if path.Base(r.URL.Path) != path.Base(pathRes.Path) { - w.WriteHeader(http.StatusNotFound) + sublog.Debug(). + Str("requestbase", path.Base(r.URL.Path)). + Str("pathbase", path.Base(pathRes.Path)). + Msg("base paths don't match") + w.WriteHeader(http.StatusConflict) return false } @@ -136,10 +132,10 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s ctx := r.Context() ctx, span := trace.StartSpan(ctx, "propfind") defer span.End() - log := appctx.GetLogger(ctx) tokenStatInfo := ctx.Value(tokenStatInfoKey{}).(*provider.ResourceInfo) - log.Debug().Interface("tokenStatInfo", tokenStatInfo).Msg("handlePropfindOnToken") + sublog := appctx.GetLogger(ctx).With().Interface("tokenStatInfo", tokenStatInfo).Logger() + sublog.Debug().Msg("handlePropfindOnToken") depth := r.Header.Get("Depth") if depth == "" { @@ -148,21 +144,21 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s // see https://tools.ietf.org/html/rfc4918#section-10.2 if depth != "0" && depth != "1" && depth != "infinity" { - log.Error().Msgf("invalid Depth header value %s", depth) + sublog.Debug().Msgf("invalid Depth header value %s", depth) w.WriteHeader(http.StatusBadRequest) return } pf, status, err := readPropfind(r.Body) if err != nil { - log.Error().Err(err).Msg("error reading propfind request") + sublog.Debug().Err(err).Msg("error reading propfind request") w.WriteHeader(status) return } client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -172,25 +168,13 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s ResourceId: tokenStatInfo.GetId(), }) if err != nil { - log.Warn(). - Str("tokenStatInfo.Id", tokenStatInfo.GetId().String()). - Str("tokenStatInfo.Path", tokenStatInfo.Path). - Msg("Could not get path of resource") - w.WriteHeader(http.StatusNotFound) + sublog.Warn().Msg("Could not get path of resource") + w.WriteHeader(http.StatusInternalServerError) return } if pathRes.Status.Code != rpc.Code_CODE_OK { - switch pathRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - w.WriteHeader(http.StatusNotFound) - return - case rpc.Code_CODE_PERMISSION_DENIED: - w.WriteHeader(http.StatusForbidden) - return - default: - w.WriteHeader(http.StatusInternalServerError) - return - } + handleErrorStatus(&sublog, w, pathRes.Status) + return } infos := []*provider.ResourceInfo{} @@ -229,7 +213,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s propRes, err := s.formatPropfind(ctx, &pf, infos, ns) if err != nil { - log.Error().Err(err).Msg("error formatting propfind") + sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } @@ -238,6 +222,6 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s w.Header().Set("Content-Type", "application/xml; charset=utf-8") w.WriteHeader(http.StatusMultiStatus) if _, err := w.Write([]byte(propRes)); err != nil { - log.Err(err).Msg("error writing response") + sublog.Err(err).Msg("error writing response") } } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 935a67bae00..7fb5fe813be 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -33,6 +33,7 @@ import ( "github.com/cs3org/reva/pkg/rhttp" "github.com/cs3org/reva/pkg/storage/utils/chunking" "github.com/cs3org/reva/pkg/utils" + "go.opencensus.io/trace" ) func sufferMacOSFinder(r *http.Request) bool { @@ -102,19 +103,20 @@ func isContentRange(r *http.Request) bool { func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) ns = applyLayout(ctx, ns) fn := path.Join(ns, r.URL.Path) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() + if r.Body == nil { - log.Warn().Msg("body is nil") + sublog.Debug().Msg("body is nil") w.WriteHeader(http.StatusBadRequest) return } if isContentRange(r) { - log.Warn().Msg("Content-Range not supported for PUT") + sublog.Debug().Msg("Content-Range not supported for PUT") w.WriteHeader(http.StatusNotImplemented) return } @@ -122,7 +124,7 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { if sufferMacOSFinder(r) { err := handleMacOSFinder(w, r) if err != nil { - log.Error().Err(err).Msg("error handling Mac OS corner-case") + sublog.Debug().Err(err).Msg("error handling Mac OS corner-case") w.WriteHeader(http.StatusInternalServerError) return } @@ -143,11 +145,13 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io.Reader, fn string, length int64) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "put") + defer span.End() + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -158,26 +162,19 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io sReq := &provider.StatRequest{Ref: ref} sRes, err := client.Stat(ctx, sReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch sRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } info := sRes.Info if info != nil { if info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - log.Warn().Msg("resource is not a file") + sublog.Debug().Msg("resource is not a file") w.WriteHeader(http.StatusConflict) return } @@ -185,7 +182,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io serverETag := info.Etag if clientETag != "" { if clientETag != serverETag { - log.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") + sublog.Debug().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") w.WriteHeader(http.StatusPreconditionFailed) return } @@ -217,23 +214,13 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io // where to upload the file? uRes, err := client.InitiateFileUpload(ctx, uReq) if err != nil { - log.Error().Err(err).Msg("error initiating file upload") + sublog.Error().Err(err).Msg("error initiating file upload") w.WriteHeader(http.StatusInternalServerError) return } if uRes.Status.Code != rpc.Code_CODE_OK { - switch uRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", uRes.Status).Msg("grpc initiate file upload request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, uRes.Status) return } @@ -254,7 +241,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io httpRes, err := s.client.Do(httpReq) if err != nil { - log.Err(err).Msg("error doing PUT request to data service") + sublog.Error().Err(err).Msg("error doing PUT request to data service") w.WriteHeader(http.StatusInternalServerError) return } @@ -264,7 +251,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io w.WriteHeader(http.StatusPartialContent) return } - log.Err(err).Msg("PUT request to data server failed") + sublog.Error().Err(err).Msg("PUT request to data server failed") w.WriteHeader(http.StatusInternalServerError) return } @@ -293,23 +280,13 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io // stat again to check the new file's metadata sRes, err = client.Stat(ctx, sReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK { - switch sRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index 6ffe67fe423..1f663f69de5 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/internal/http/services/owncloud/ocdav/trashbin.go @@ -37,6 +37,7 @@ import ( "github.com/cs3org/reva/pkg/rhttp/router" ctxuser "github.com/cs3org/reva/pkg/user" "github.com/cs3org/reva/pkg/utils" + "go.opencensus.io/trace" ) // TrashbinHandler handles trashbin requests @@ -127,11 +128,14 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler { func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "listTrashbin") + defer span.End() + + sublog := appctx.GetLogger(ctx).With().Logger() pf, status, err := readPropfind(r.Body) if err != nil { - log.Error().Err(err).Msg("error reading propfind request") + sublog.Debug().Err(err).Msg("error reading propfind request") w.WriteHeader(status) return } @@ -141,29 +145,19 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s // TODO(jfd) how do we make the user aware that some storages are not available? // opaque response property? Or a list of errors? // add a recycle entry with the path to the storage that produced the error? - log.Error().Err(err).Msg("error getting gateway client") + sublog.Error().Err(err).Msg("error getting gateway client") w.WriteHeader(http.StatusInternalServerError) return } getHomeRes, err := gc.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - log.Error().Err(err).Msg("error calling GetHome") + sublog.Error().Err(err).Msg("error calling GetHome") w.WriteHeader(http.StatusInternalServerError) return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - switch getHomeRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, getHomeRes.Status) return } @@ -178,29 +172,19 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s }) if err != nil { - log.Error().Err(err).Msg("error calling ListRecycle") + sublog.Error().Err(err).Msg("error calling ListRecycle") w.WriteHeader(http.StatusInternalServerError) return } if getRecycleRes.Status.Code != rpc.Code_CODE_OK { - switch getRecycleRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", getHomeRes.Path).Interface("status", getRecycleRes.Status).Msg("grpc list recycle request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, getHomeRes.Status) return } propRes, err := h.formatTrashPropfind(ctx, s, u, &pf, getRecycleRes.RecycleItems) if err != nil { - log.Error().Err(err).Msg("error formatting propfind") + sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } @@ -209,7 +193,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s w.WriteHeader(http.StatusMultiStatus) _, err = w.Write([]byte(propRes)) if err != nil { - log.Error().Err(err).Msg("error writing body") + sublog.Error().Err(err).Msg("error writing body") return } } @@ -368,33 +352,26 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, pf *pr // restore has a destination and a key func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, dst string, key string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "restore") + defer span.End() + + sublog := appctx.GetLogger(ctx).With().Logger() client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } getHomeRes, err := client.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - log.Error().Err(err).Msg("error calling GetHome") + sublog.Error().Err(err).Msg("error calling GetHome") w.WriteHeader(http.StatusInternalServerError) return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - switch getHomeRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, getHomeRes.Status) return } @@ -414,57 +391,41 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc res, err := client.RestoreRecycleItem(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending a grpc restore recycle item request") + sublog.Error().Err(err).Msg("error sending a grpc restore recycle item request") w.WriteHeader(http.StatusInternalServerError) return } - switch res.Status.Code { - case rpc.Code_CODE_OK: - w.WriteHeader(http.StatusNoContent) - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("key", key).Str("dst", dst).Interface("status", res.Status).Msg("grpc restore recycle item request failed") - w.WriteHeader(http.StatusInternalServerError) + if res.Status.Code != rpc.Code_CODE_OK { + handleErrorStatus(&sublog, w, res.Status) + return } + w.WriteHeader(http.StatusNoContent) } // delete has only a key func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key string) { - ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "erase") + defer span.End() + + sublog := appctx.GetLogger(ctx).With().Str("key", key).Logger() client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } getHomeRes, err := client.GetHome(ctx, &provider.GetHomeRequest{}) if err != nil { - log.Error().Err(err).Msg("error calling GetHomeProvider") + sublog.Error().Err(err).Msg("error calling GetHomeProvider") w.WriteHeader(http.StatusInternalServerError) return } if getHomeRes.Status.Code != rpc.Code_CODE_OK { - switch getHomeRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", getHomeRes.Path).Interface("status", getHomeRes.Status).Msg("grpc get home request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, getHomeRes.Status) return } sRes, err := client.Stat(ctx, &provider.StatRequest{ @@ -475,22 +436,12 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, }, }) if err != nil { - log.Error().Err(err).Msg("error calling Stat") + sublog.Error().Err(err).Msg("error calling Stat") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK { - switch sRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", getHomeRes.Path).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } @@ -510,7 +461,7 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, res, err := client.PurgeRecycle(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending a grpc restore recycle item request") + sublog.Error().Err(err).Msg("error sending a grpc restore recycle item request") w.WriteHeader(http.StatusInternalServerError) return } @@ -518,13 +469,9 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, case rpc.Code_CODE_OK: w.WriteHeader(http.StatusNoContent) case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("resource not found") + sublog.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("resource not found") w.WriteHeader(http.StatusConflict) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) default: - log.Error().Str("storageid", sRes.Info.Id.StorageId).Str("key", key).Interface("status", res.Status).Msg("grpc purge recycle request failed") - w.WriteHeader(http.StatusInternalServerError) + handleErrorStatus(&sublog, w, res.Status) } } diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 8ef8d8ddf8b..ccdbefd1aa8 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -32,11 +32,13 @@ import ( "github.com/cs3org/reva/pkg/rhttp" "github.com/cs3org/reva/pkg/utils" tusd "github.com/tus/tusd/pkg/handler" + "go.opencensus.io/trace" ) func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "tus-post") + defer span.End() w.Header().Add("Access-Control-Allow-Headers", "Tus-Resumable, Upload-Length, Upload-Metadata, If-Match") w.Header().Add("Access-Control-Expose-Headers", "Tus-Resumable, Location") @@ -68,12 +70,13 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { // append filename to current dir fn := path.Join(ns, r.URL.Path, meta["filename"]) + sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() // check tus headers? // check if destination exists or is a file client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -85,26 +88,19 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { } sRes, err := client.Stat(ctx, sReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch sRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } info := sRes.Info if info != nil && info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - log.Warn().Msg("resource is not a file") + sublog.Warn().Msg("resource is not a file") w.WriteHeader(http.StatusConflict) return } @@ -114,7 +110,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { serverETag := info.Etag if clientETag != "" { if clientETag != serverETag { - log.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") + sublog.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") w.WriteHeader(http.StatusPreconditionFailed) return } @@ -148,23 +144,13 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { uRes, err := client.InitiateFileUpload(ctx, uReq) if err != nil { - log.Error().Err(err).Msg("error initiating file upload") + sublog.Error().Err(err).Msg("error initiating file upload") w.WriteHeader(http.StatusInternalServerError) return } if uRes.Status.Code != rpc.Code_CODE_OK { - switch uRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", uRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", uRes.Status).Msg("grpc initiate file upload request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, uRes.Status) return } @@ -192,7 +178,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { length, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) if err != nil { - log.Err(err).Msg("wrong request") + sublog.Debug().Err(err).Msg("wrong request") w.WriteHeader(http.StatusBadRequest) return } @@ -202,7 +188,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { if length != 0 { httpReq, err := rhttp.NewRequest(ctx, "PATCH", ep, r.Body) if err != nil { - log.Err(err).Msg("wrong request") + sublog.Debug().Err(err).Msg("wrong request") w.WriteHeader(http.StatusInternalServerError) return } @@ -218,7 +204,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { httpRes, err = s.client.Do(httpReq) if err != nil { - log.Err(err).Msg("error doing GET request to data service") + sublog.Error().Err(err).Msg("error doing GET request to data service") w.WriteHeader(http.StatusInternalServerError) return } @@ -231,7 +217,7 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { return } } else { - log.Info().Msg("Skipping sending a Patch request as body is empty") + sublog.Debug().Msg("Skipping sending a Patch request as body is empty") } // check if upload was fully completed @@ -239,26 +225,19 @@ func (s *svc) handleTusPost(w http.ResponseWriter, r *http.Request, ns string) { // get uploaded file metadata sRes, err := client.Stat(ctx, sReq) if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + sublog.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK && sRes.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch sRes.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", sRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", sRes.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, sRes.Status) return } info := sRes.Info if info == nil { - log.Error().Str("fn", fn).Msg("No info found for uploaded file") + sublog.Error().Msg("No info found for uploaded file") w.WriteHeader(http.StatusInternalServerError) return } diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index ca9f1432e0c..e8cff38e856 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -28,6 +28,7 @@ import ( types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rhttp/router" + "go.opencensus.io/trace" ) // VersionsHandler handles version requests @@ -74,18 +75,21 @@ func (h *VersionsHandler) Handler(s *svc, rid *provider.ResourceId) http.Handler func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, s *svc, rid *provider.ResourceId) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "listVersions") + defer span.End() + + sublog := appctx.GetLogger(ctx).With().Interface("resourceid", rid).Logger() pf, status, err := readPropfind(r.Body) if err != nil { - log.Error().Err(err).Msg("error reading propfind request") + sublog.Debug().Err(err).Msg("error reading propfind request") w.WriteHeader(status) return } client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -96,22 +100,12 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, req := &provider.StatRequest{Ref: ref} res, err := client.Stat(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending a grpc stat request") + sublog.Error().Err(err).Msg("error sending a grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if res.Status.Code != rpc.Code_CODE_OK { - switch res.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Interface("resourceid", rid).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Interface("resourceid", rid).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Interface("resourceid", rid).Interface("status", res.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, res.Status) return } @@ -122,22 +116,12 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, } lvRes, err := client.ListFileVersions(ctx, lvReq) if err != nil { - log.Error().Err(err).Msg("error sending list container grpc request") + sublog.Error().Err(err).Msg("error sending list container grpc request") w.WriteHeader(http.StatusInternalServerError) return } if lvRes.Status.Code != rpc.Code_CODE_OK { - switch lvRes.Status.Code { - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Interface("resourceid", rid).Interface("status", lvRes.Status).Msg("grpc list file revisions request failed") - w.WriteHeader(http.StatusInternalServerError) - } + handleErrorStatus(&sublog, w, lvRes.Status) return } @@ -185,7 +169,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, propRes, err := s.formatPropfind(ctx, &pf, infos, "") if err != nil { - log.Error().Err(err).Msg("error formatting propfind") + sublog.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } @@ -194,7 +178,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, w.WriteHeader(http.StatusMultiStatus) _, err = w.Write([]byte(propRes)) if err != nil { - log.Error().Err(err).Msg("error writing body") + sublog.Error().Err(err).Msg("error writing body") return } @@ -202,11 +186,14 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, func (h *VersionsHandler) doRestore(w http.ResponseWriter, r *http.Request, s *svc, rid *provider.ResourceId, key string) { ctx := r.Context() - log := appctx.GetLogger(ctx) + ctx, span := trace.StartSpan(ctx, "restore") + defer span.End() + + sublog := appctx.GetLogger(ctx).With().Interface("resourceid", rid).Str("key", key).Logger() client, err := s.getClient() if err != nil { - log.Error().Err(err).Msg("error getting grpc client") + sublog.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } @@ -220,21 +207,13 @@ func (h *VersionsHandler) doRestore(w http.ResponseWriter, r *http.Request, s *s res, err := client.RestoreFileVersion(ctx, req) if err != nil { - log.Error().Err(err).Msg("error sending a grpc restore version request") + sublog.Error().Err(err).Msg("error sending a grpc restore version request") w.WriteHeader(http.StatusInternalServerError) return } - switch res.Status.Code { - case rpc.Code_CODE_OK: - w.WriteHeader(http.StatusNoContent) - case rpc.Code_CODE_NOT_FOUND: - log.Debug().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("resource not found") - w.WriteHeader(http.StatusNotFound) - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Interface("resourceid", rid).Str("key", key).Interface("status", res.Status).Msg("grpc restore file revision request failed") - w.WriteHeader(http.StatusInternalServerError) + if res.Status.Code != rpc.Code_CODE_OK { + handleErrorStatus(&sublog, w, res.Status) + return } + w.WriteHeader(http.StatusNoContent) }