Addressed review comments

cs3org · Feb 11, 2022 · 3450ac2 · 3450ac2
1 parent df15d13
commit 3450ac2
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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
 		}