From 965ef683b6491ce46f21379f521f61421a3b2857 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Tue, 13 Jul 2021 17:34:57 +0200 Subject: [PATCH] refactor webdav put --- .../services/owncloud/ocdav/publicfile.go | 2 +- internal/http/services/owncloud/ocdav/put.go | 150 +++++++++--------- .../http/services/owncloud/ocdav/webdav.go | 2 +- 3 files changed, 80 insertions(+), 74 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/publicfile.go b/internal/http/services/owncloud/ocdav/publicfile.go index 3085f1cf91e..240c5d3b49a 100644 --- a/internal/http/services/owncloud/ocdav/publicfile.go +++ b/internal/http/services/owncloud/ocdav/publicfile.go @@ -66,7 +66,7 @@ func (h *PublicFileHandler) Handler(s *svc) http.Handler { case http.MethodHead: s.handlePathHead(w, r, h.namespace) case http.MethodPut: - s.handlePut(w, r, h.namespace) + s.handlePathPut(w, r, h.namespace) default: w.WriteHeader(http.StatusMethodNotAllowed) } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index f767adfa8eb..3873f748265 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -19,7 +19,7 @@ package ocdav import ( - "io" + "context" "net/http" "path" "strconv" @@ -35,11 +35,11 @@ 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" + "github.com/rs/zerolog" ) func sufferMacOSFinder(r *http.Request) bool { - return r.Header.Get("X-Expected-Entity-Length") != "" + return r.Header.Get(HeaderExpectedEntityLength) != "" } func handleMacOSFinder(w http.ResponseWriter, r *http.Request) error { @@ -61,8 +61,8 @@ func handleMacOSFinder(w http.ResponseWriter, r *http.Request) error { */ log := appctx.GetLogger(r.Context()) - content := r.Header.Get("Content-Length") - expected := r.Header.Get("X-Expected-Entity-Length") + content := r.Header.Get(HeaderContentLength) + expected := r.Header.Get(HeaderExpectedEntityLength) log.Warn().Str("content-length", content).Str("x-expected-entity-length", expected).Msg("Mac OS Finder corner-case detected") // The best mitigation to this problem is to tell users to not use crappy Finder. @@ -100,87 +100,63 @@ func isContentRange(r *http.Request) bool { in unexpected behaviour (cf PEAR::HTTP_WebDAV_Client 1.0.1), we reject all PUT requests with a Content-Range for now. */ - return r.Header.Get("Content-Range") != "" + return r.Header.Get(HeaderContentRange) != "" } -func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { +func (s *svc) handlePathPut(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() fn := path.Join(ns, r.URL.Path) sublog := appctx.GetLogger(ctx).With().Str("path", fn).Logger() - if r.Body == nil { - sublog.Debug().Msg("body is nil") - w.WriteHeader(http.StatusBadRequest) - return - } + ref := &provider.Reference{Path: fn} - if isContentRange(r) { - sublog.Debug().Msg("Content-Range not supported for PUT") - w.WriteHeader(http.StatusNotImplemented) - return - } + s.handlePut(ctx, w, r, ref, sublog) +} - if sufferMacOSFinder(r) { - err := handleMacOSFinder(w, r) - if err != nil { - sublog.Debug().Err(err).Msg("error handling Mac OS corner-case") - w.WriteHeader(http.StatusInternalServerError) - return - } +func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Request, ref *provider.Reference, log zerolog.Logger) { + if !checkPreconditions(w, r, log) { + // checkPreconditions handles error returns + return } - length, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) + length, err := getContentLength(w, r) if err != nil { - // Fallback to Upload-Length - length, err = strconv.ParseInt(r.Header.Get("Upload-Length"), 10, 64) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - return - } + w.WriteHeader(http.StatusBadRequest) + return } - s.handlePutHelper(w, r, r.Body, fn, length) -} - -func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io.Reader, fn string, length int64) { - ctx := r.Context() - 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 { - sublog.Error().Err(err).Msg("error getting grpc client") + log.Error().Err(err).Msg("error getting grpc client") w.WriteHeader(http.StatusInternalServerError) return } - ref := &provider.Reference{Path: fn} sReq := &provider.StatRequest{Ref: ref} sRes, err := client.Stat(ctx, sReq) if err != nil { - sublog.Error().Err(err).Msg("error sending grpc stat request") + log.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 { - HandleErrorStatus(&sublog, w, sRes.Status) + HandleErrorStatus(&log, w, sRes.Status) return } info := sRes.Info if info != nil { if info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - sublog.Debug().Msg("resource is not a file") + log.Debug().Msg("resource is not a file") w.WriteHeader(http.StatusConflict) return } - clientETag := r.Header.Get("If-Match") + clientETag := r.Header.Get(HeaderIfMatch) serverETag := info.Etag if clientETag != "" { if clientETag != serverETag { - sublog.Debug().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") + log.Debug().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") w.WriteHeader(http.StatusPreconditionFailed) return } @@ -188,38 +164,38 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io } opaqueMap := map[string]*typespb.OpaqueEntry{ - "Upload-Length": { + HeaderUploadLength: { Decoder: "plain", Value: []byte(strconv.FormatInt(length, 10)), }, } - if mtime := r.Header.Get("X-OC-Mtime"); mtime != "" { - opaqueMap["X-OC-Mtime"] = &typespb.OpaqueEntry{ + if mtime := r.Header.Get(HeaderOCMtime); mtime != "" { + opaqueMap[HeaderOCMtime] = &typespb.OpaqueEntry{ Decoder: "plain", Value: []byte(mtime), } // TODO: find a way to check if the storage really accepted the value - w.Header().Set("X-OC-Mtime", "accepted") + w.Header().Set(HeaderOCMtime, "accepted") } // curl -X PUT https://demo.owncloud.com/remote.php/webdav/testcs.bin -u demo:demo -d '123' -v -H 'OC-Checksum: SHA1:40bd001563085fc35165329ea1ff5c5ecbdbbeef' var cparts []string // TUS Upload-Checksum header takes precedence - if checksum := r.Header.Get("Upload-Checksum"); checksum != "" { + if checksum := r.Header.Get(HeaderUploadChecksum); checksum != "" { cparts = strings.SplitN(checksum, " ", 2) if len(cparts) != 2 { - sublog.Debug().Str("upload-checksum", checksum).Msg("invalid Upload-Checksum format, expected '[algorithm] [checksum]'") + log.Debug().Str("upload-checksum", checksum).Msg("invalid Upload-Checksum format, expected '[algorithm] [checksum]'") w.WriteHeader(http.StatusBadRequest) return } // Then try owncloud header - } else if checksum := r.Header.Get("OC-Checksum"); checksum != "" { + } else if checksum := r.Header.Get(HeaderOCChecksum); checksum != "" { cparts = strings.SplitN(checksum, ":", 2) if len(cparts) != 2 { - sublog.Debug().Str("oc-checksum", checksum).Msg("invalid OC-Checksum format, expected '[algorithm]:[checksum]'") + log.Debug().Str("oc-checksum", checksum).Msg("invalid OC-Checksum format, expected '[algorithm]:[checksum]'") w.WriteHeader(http.StatusBadRequest) return } @@ -227,7 +203,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io // we do not check the algorithm here, because it might depend on the storage if len(cparts) == 2 { // Translate into TUS style Upload-Checksum header - opaqueMap["Upload-Checksum"] = &typespb.OpaqueEntry{ + opaqueMap[HeaderUploadChecksum] = &typespb.OpaqueEntry{ Decoder: "plain", // algorithm is always lowercase, checksum is separated by space Value: []byte(strings.ToLower(cparts[0]) + " " + cparts[1]), @@ -242,7 +218,7 @@ 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 { - sublog.Error().Err(err).Msg("error initiating file upload") + log.Error().Err(err).Msg("error initiating file upload") w.WriteHeader(http.StatusInternalServerError) return } @@ -254,9 +230,9 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io code: SabredavPermissionDenied, message: "permission denied: you have no permission to upload content", }) - HandleWebdavError(&sublog, w, b, err) + HandleWebdavError(&log, w, b, err) } - HandleErrorStatus(&sublog, w, uRes.Status) + HandleErrorStatus(&log, w, uRes.Status) return } @@ -268,7 +244,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io } if length > 0 { - httpReq, err := rhttp.NewRequest(ctx, "PUT", ep, content) + httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, ep, r.Body) if err != nil { w.WriteHeader(http.StatusInternalServerError) return @@ -277,7 +253,7 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io httpRes, err := s.client.Do(httpReq) if err != nil { - sublog.Error().Err(err).Msg("error doing PUT request to data service") + log.Error().Err(err).Msg("error doing PUT request to data service") w.WriteHeader(http.StatusInternalServerError) return } @@ -293,21 +269,21 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io code: SabredavBadRequest, message: "The computed checksum does not match the one received from the client.", }) - HandleWebdavError(&sublog, w, b, err) + HandleWebdavError(&log, w, b, err) } - sublog.Error().Err(err).Msg("PUT request to data server failed") + log.Error().Err(err).Msg("PUT request to data server failed") w.WriteHeader(httpRes.StatusCode) return } } - ok, err := chunking.IsChunked(fn) + ok, err := chunking.IsChunked(ref.Path) if err != nil { w.WriteHeader(http.StatusInternalServerError) return } if ok { - chunk, err := chunking.GetChunkBLOBInfo(fn) + chunk, err := chunking.GetChunkBLOBInfo(ref.Path) if err != nil { w.WriteHeader(http.StatusInternalServerError) return @@ -318,25 +294,25 @@ 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 { - sublog.Error().Err(err).Msg("error sending grpc stat request") + log.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } if sRes.Status.Code != rpc.Code_CODE_OK { - HandleErrorStatus(&sublog, w, sRes.Status) + HandleErrorStatus(&log, w, sRes.Status) return } newInfo := sRes.Info - w.Header().Add("Content-Type", newInfo.MimeType) - w.Header().Set("ETag", newInfo.Etag) - w.Header().Set("OC-FileId", wrapResourceID(newInfo.Id)) - w.Header().Set("OC-ETag", newInfo.Etag) + w.Header().Add(HeaderContentType, newInfo.MimeType) + w.Header().Set(HeaderETag, newInfo.Etag) + w.Header().Set(HeaderOCFileID, wrapResourceID(newInfo.Id)) + w.Header().Set(HeaderOCETag, newInfo.Etag) t := utils.TSToTime(newInfo.Mtime).UTC() lastModifiedString := t.Format(time.RFC1123Z) - w.Header().Set("Last-Modified", lastModifiedString) + w.Header().Set(HeaderLastModified, lastModifiedString) // file was new if info == nil { @@ -347,3 +323,33 @@ func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io // overwrite w.WriteHeader(http.StatusNoContent) } + +func checkPreconditions(w http.ResponseWriter, r *http.Request, log zerolog.Logger) bool { + if isContentRange(r) { + log.Debug().Msg("Content-Range not supported for PUT") + w.WriteHeader(http.StatusNotImplemented) + return false + } + + if sufferMacOSFinder(r) { + err := handleMacOSFinder(w, r) + if err != nil { + log.Debug().Err(err).Msg("error handling Mac OS corner-case") + w.WriteHeader(http.StatusInternalServerError) + return false + } + } + return true +} + +func getContentLength(w http.ResponseWriter, r *http.Request) (int64, error) { + length, err := strconv.ParseInt(r.Header.Get(HeaderContentLength), 10, 64) + if err != nil { + // Fallback to Upload-Length + length, err = strconv.ParseInt(r.Header.Get(HeaderUploadLength), 10, 64) + if err != nil { + return 0, err + } + } + return length, nil +} diff --git a/internal/http/services/owncloud/ocdav/webdav.go b/internal/http/services/owncloud/ocdav/webdav.go index 3c00b6ee0b1..b956a55c001 100644 --- a/internal/http/services/owncloud/ocdav/webdav.go +++ b/internal/http/services/owncloud/ocdav/webdav.go @@ -109,7 +109,7 @@ func (h *WebDavHandler) Handler(s *svc) http.Handler { case http.MethodGet: s.handlePathGet(w, r, ns) case http.MethodPut: - s.handlePut(w, r, ns) + s.handlePathPut(w, r, ns) case http.MethodPost: s.handleTusPost(w, r, ns) case http.MethodOptions: