From 3450ac20bccf601672ede2302030e644f876b23d Mon Sep 17 00:00:00 2001 From: Giuseppe Lo Presti Date: Thu, 10 Feb 2022 16:07:14 +0100 Subject: [PATCH] Addressed review comments --- internal/http/services/owncloud/ocdav/copy.go | 18 +++-- internal/http/services/owncloud/ocdav/tpc.go | 73 +++++++++---------- pkg/cbox/user/rest/rest.go | 2 +- 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index d988bc0df2..55a6ea6fc0 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/internal/http/services/owncloud/ocdav/copy.go @@ -52,14 +52,16 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string) ctx, span := rtrace.Provider.Tracer("reva").Start(r.Context(), "copy") defer span.End() - if r.Header.Get("Source") != "" && s.c.EnableHTTPTpc { - // HTTP Third-Party Copy Pull mode - s.handleTPCPull(ctx, w, r, ns) - return - } else if r.Header.Get("Destination") != "" && s.c.EnableHTTPTpc { - // HTTP Third-Party Copy Push mode - s.handleTPCPush(ctx, w, r, ns) - return + if s.c.EnableHTTPTpc { + if r.Header.Get("Source") != "" { + // HTTP Third-Party Copy Pull mode + s.handleTPCPull(ctx, w, r, ns) + return + } else if r.Header.Get("Destination") != "" { + // HTTP Third-Party Copy Push mode + s.handleTPCPush(ctx, w, r, ns) + return + } } // Local copy: in this case Destination is mandatory diff --git a/internal/http/services/owncloud/ocdav/tpc.go b/internal/http/services/owncloud/ocdav/tpc.go index d85d4c3cab..c31b6066a4 100644 --- a/internal/http/services/owncloud/ocdav/tpc.go +++ b/internal/http/services/owncloud/ocdav/tpc.go @@ -34,8 +34,8 @@ import ( typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/internal/http/services/datagateway" "github.com/cs3org/reva/pkg/appctx" + "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/rhttp" - "github.com/pkg/errors" ) const ( @@ -161,7 +161,6 @@ func (s *svc) handleTPCPull(ctx context.Context, w http.ResponseWriter, r *http. return } - w.WriteHeader(http.StatusAccepted) err = s.performHTTPPull(ctx, client, r, w, ns) if err != nil { sublog.Error().Err(err).Msg("error performing TPC Pull") @@ -176,6 +175,36 @@ func (s *svc) performHTTPPull(ctx context.Context, client gateway.GatewayAPIClie sublog := appctx.GetLogger(ctx) sublog.Debug().Str("src", src).Str("dst", dst).Msg("Performing HTTP Pull") + // get http client for remote + httpClient := &http.Client{} + + req, err := http.NewRequest("GET", src, nil) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return err + } + + // add authentication header + bearerHeader := r.Header.Get(HeaderTransferAuth) + req.Header.Add("Authorization", bearerHeader) + + // do download + httpDownloadRes, err := httpClient.Do(req) // lgtm[go/request-forgery] + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + return err + } + defer httpDownloadRes.Body.Close() + + if httpDownloadRes.StatusCode == http.StatusNotImplemented { + w.WriteHeader(http.StatusBadRequest) + return errtypes.NotSupported("Third-Party copy not supported, source might be a folder") + } + if httpDownloadRes.StatusCode != http.StatusOK { + w.WriteHeader(httpDownloadRes.StatusCode) + return errtypes.InternalError(fmt.Sprintf("Remote GET returned status code %d", httpDownloadRes.StatusCode)) + } + // get upload url uReq := &provider.InitiateFileUploadRequest{ Ref: &provider.Reference{Path: dst}, @@ -205,39 +234,8 @@ func (s *svc) performHTTPPull(ctx context.Context, client gateway.GatewayAPIClie } } - // get http client for remote - httpClient := &http.Client{} - - req, err := http.NewRequest("GET", src, nil) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return err - } - - // add authentication header - bearerHeader := r.Header.Get(HeaderTransferAuth) - req.Header.Add("Authorization", bearerHeader) - - // do download - httpDownloadRes, err := httpClient.Do(req) // lgtm[go/request-forgery] - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return err - } - defer httpDownloadRes.Body.Close() - - if httpDownloadRes.StatusCode == http.StatusNotImplemented { - // source is a folder, fail - w.WriteHeader(http.StatusBadRequest) - return errors.New("Third-Party copy of a folder is not supported") - } - if httpDownloadRes.StatusCode != http.StatusOK { - w.WriteHeader(http.StatusInternalServerError) - return fmt.Errorf("Remote GET returned status code %d", httpDownloadRes.StatusCode) - } - - // send performance markers periodically every PerfMarkerResponseTime (5 seconds unless configured). - // seconds as transfer progresses + // send performance markers periodically every PerfMarkerResponseTime (5 seconds unless configured) + w.WriteHeader(http.StatusAccepted) wc := WriteCounter{0, time.Now(), w} tempReader := io.TeeReader(httpDownloadRes.Body, &wc) @@ -325,7 +323,6 @@ func (s *svc) handleTPCPush(ctx context.Context, w http.ResponseWriter, r *http. return } - w.WriteHeader(http.StatusAccepted) err = s.performHTTPPush(ctx, client, r, w, srcStatRes.Info, ns) if err != nil { sublog.Error().Err(err).Msg("error performing TPC Push") @@ -383,8 +380,8 @@ func (s *svc) performHTTPPush(ctx context.Context, client gateway.GatewayAPIClie return fmt.Errorf("Remote PUT returned status code %d", httpDownloadRes.StatusCode) } - // send performance markers periodically ever $PerfMarkerResponseTime - // seconds as transfer progreses + // send performance markers periodically every PerfMarkerResponseTime (5 seconds unless configured) + w.WriteHeader(http.StatusAccepted) wc := WriteCounter{0, time.Now(), w} tempReader := io.TeeReader(httpDownloadRes.Body, &wc) diff --git a/pkg/cbox/user/rest/rest.go b/pkg/cbox/user/rest/rest.go index b6ac931c59..7fc1c3c427 100644 --- a/pkg/cbox/user/rest/rest.go +++ b/pkg/cbox/user/rest/rest.go @@ -291,7 +291,7 @@ func (m *manager) GetUserByClaim(ctx context.Context, claim, value string, skipF var userData map[string]interface{} if strings.HasPrefix(value, "guest:") { - // Lightweight accounts need to be fetched by email, regardless the demanded claim + // Lightweight accounts need to be fetched by email, regardless of the demanded claim if userData, err = m.getLightweightUser(ctx, strings.TrimPrefix(value, "guest:")); err != nil { return nil, err }