ocdav: handle redirection prefixes when extracting destination from U…

…RL (#1111)

changelog/unreleased/eosfs-recycle-purge.md

@@ -1,6 +1,5 @@
 Bugfix: do not restore recycle entry on purge
 
-
 This PR fixes a bug in the EOSFS driver that was restoring a deleted entry
 when asking for its permanent purge.
 EOS does not have the functionality to purge individual files, but the whole recycle of the user.

changelog/unreleased/webdav-move-prefix.md

@@ -0,0 +1,8 @@
+Bugfix: Handle redirection prefixes when extracting destination from URL
+
+The move function handler in ocdav extracts the destination path from the URL by
+removing the base URL prefix from the URL path. This would fail in case there is
+a redirection prefix. This PR takes care of that and it also allows zero-size
+uploads for localfs.
+
+https://github.com/cs3org/reva/pull/1111

cmd/reva/ocm-share-create.go

@@ -117,8 +117,6 @@ func ocmShareCreateCommand() *command {
 			},
 		}
 
-		fmt.Println("res.Info.Path" + res.Info.Path)
-
 		opaqueObj := &types.Opaque{
 			Map: map[string]*types.OpaqueEntry{
 				"permissions": &types.OpaqueEntry{

internal/http/services/owncloud/ocdav/copy.go

@@ -23,7 +23,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"path"
 	"strings"
 	"time"
@@ -54,13 +53,15 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) {
 		depth = "infinity"
 	}
 
-	log.Info().Str("source", src).Str("destination", dstHeader).
-		Str("overwrite", overwrite).Str("depth", depth).Msg("copy")
-
-	if dstHeader == "" {
+	dst, err := extractDestination(dstHeader, r.Context().Value(ctxKeyBaseURI).(string))
+	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
+	dst = path.Join(ns, dst)
+
+	log.Info().Str("source", src).Str("destination", dst).
+		Str("overwrite", overwrite).Str("depth", depth).Msg("copy")
 
 	overwrite = strings.ToUpper(overwrite)
 	if overwrite == "" {
@@ -84,23 +85,6 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 
-	// strip baseURL from destination
-	dstURL, err := url.ParseRequestURI(dstHeader)
-	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		return
-	}
-
-	urlPath := dstURL.Path
-	baseURI := r.Context().Value(ctxKeyBaseURI).(string)
-	log.Info().Str("url-path", urlPath).Str("base-uri", baseURI).Msg("copy")
-	// TODO replace with HasPrefix:
-	i := strings.Index(urlPath, baseURI)
-	if i == -1 {
-		w.WriteHeader(http.StatusBadRequest)
-		return
-	}
-
 	// check src exists
 	ref := &provider.Reference{
 		Spec: &provider.Reference_Path{Path: src},
@@ -122,10 +106,6 @@ func (s *svc) handleCopy(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 
-	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
-	// prefix to namespace
-	dst := path.Join(ns, urlPath[len(baseURI):])
-
 	// check dst exists
 	ref = &provider.Reference{
 		Spec: &provider.Reference_Path{Path: dst},

internal/http/services/owncloud/ocdav/move.go

@@ -20,7 +20,6 @@ package ocdav
 
 import (
 	"net/http"
-	"net/url"
 	"path"
 	"strings"
 
@@ -39,12 +38,14 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) {
 	dstHeader := r.Header.Get("Destination")
 	overwrite := r.Header.Get("Overwrite")
 
-	log.Info().Str("src", src).Str("dst", dstHeader).Str("overwrite", overwrite).Msg("move")
-
-	if dstHeader == "" {
+	dst, err := extractDestination(dstHeader, r.Context().Value(ctxKeyBaseURI).(string))
+	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
 	}
+	dst = path.Join(ns, dst)
+
+	log.Info().Str("src", src).Str("dst", dst).Str("overwrite", overwrite).Msg("move")
 
 	overwrite = strings.ToUpper(overwrite)
 	if overwrite == "" {
@@ -63,23 +64,6 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 
-	// strip baseURL from destination
-	dstURL, err := url.ParseRequestURI(dstHeader)
-	if err != nil {
-		w.WriteHeader(http.StatusBadRequest)
-		return
-	}
-
-	urlPath := dstURL.Path
-	baseURI := r.Context().Value(ctxKeyBaseURI).(string)
-	log.Info().Str("url_path", urlPath).Str("base_uri", baseURI).Msg("move urls")
-	// TODO replace with HasPrefix:
-	i := strings.Index(urlPath, baseURI)
-	if i == -1 {
-		w.WriteHeader(http.StatusBadRequest)
-		return
-	}
-
 	// check src exists
 	srcStatReq := &provider.StatRequest{
 		Ref: &provider.Reference{
@@ -102,10 +86,6 @@ func (s *svc) handleMove(w http.ResponseWriter, r *http.Request, ns string) {
 		return
 	}
 
-	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
-	// prefix to namespace
-	dst := path.Join(ns, urlPath[len(baseURI):])
-
 	// check dst exists
 	dstStatRef := &provider.Reference{
 		Spec: &provider.Reference_Path{Path: dst},

internal/http/services/owncloud/ocdav/ocdav.go

@@ -23,6 +23,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"net/http"
+	"net/url"
 	"os"
 	"path"
 	"strings"
@@ -37,6 +38,7 @@ import (
 	"github.com/cs3org/reva/pkg/storage/utils/templates"
 	ctxuser "github.com/cs3org/reva/pkg/user"
 	"github.com/mitchellh/mapstructure"
+	"github.com/pkg/errors"
 	"github.com/rs/zerolog"
 )
 
@@ -244,3 +246,22 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
 		headers.Set("Strict-Transport-Security", "max-age=63072000")
 	}
 }
+
+func extractDestination(dstHeader, baseURI string) (string, error) {
+	if dstHeader == "" {
+		return "", errors.New("destination header is empty")
+	}
+	dstURL, err := url.ParseRequestURI(dstHeader)
+	if err != nil {
+		return "", err
+	}
+
+	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
+	// Strip the base URI from the destination. The destination might contain redirection prefixes which need to be handled
+	urlSplit := strings.Split(dstURL.Path, baseURI)
+	if len(urlSplit) != 2 {
+		return "", errors.New("destination path does not contain base URI")
+	}
+
+	return urlSplit[1], nil
+}

internal/http/services/owncloud/ocdav/trashbin.go

@@ -96,37 +96,22 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 			return
 		}
 		if key != "" && r.Method == "MOVE" {
-			dstHeader := r.Header.Get("Destination")
-
-			log.Debug().Str("key", key).Str("dst", dstHeader).Msg("restore")
-
-			if dstHeader == "" {
-				w.WriteHeader(http.StatusBadRequest)
-				return
-			}
-			// strip baseURL from destination
-			dstURL, err := url.ParseRequestURI(dstHeader)
-			if err != nil {
-				w.WriteHeader(http.StatusBadRequest)
-				return
-			}
-
-			urlPath := dstURL.Path
-
 			// find path in url relative to trash base
 			trashBase := ctx.Value(ctxKeyBaseURI).(string)
 			baseURI := path.Join(path.Dir(trashBase), "files", userid)
 			ctx = context.WithValue(ctx, ctxKeyBaseURI, baseURI)
 			r = r.WithContext(ctx)
 
-			log.Debug().Str("url_path", urlPath).Str("base_uri", baseURI).Msg("move urls")
 			// TODO make request.php optional in destination header
-			i := strings.Index(urlPath, baseURI)
-			if i == -1 {
+			dstHeader := r.Header.Get("Destination")
+			dst, err := extractDestination(dstHeader, baseURI)
+			if err != nil {
 				w.WriteHeader(http.StatusBadRequest)
 				return
 			}
-			dst := path.Clean(urlPath[len(baseURI):])
+			dst = path.Clean(dst)
+
+			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
 
 			h.restore(w, r, s, u, dst, key)
 			return

pkg/ocm/invite/manager/json/json.go

@@ -250,14 +250,19 @@ func (m *manager) AcceptInvite(ctx context.Context, invite *invitepb.InviteToken
 		return err
 	}
 
-	// Add to the list of accepted users
-	userKey := inviteToken.GetUserId().GetOpaqueId()
-	for _, acceptedUser := range m.model.AcceptedUsers[userKey] {
+	currUser := inviteToken.GetUserId()
+
+	// do not allow the user who created the token to accept it
+	if remoteUser.Id.Idp == currUser.Idp && remoteUser.Id.OpaqueId == currUser.OpaqueId {
+		return errors.New("json: token creator and recipient are the same")
+	}
+
+	for _, acceptedUser := range m.model.AcceptedUsers[currUser.GetOpaqueId()] {
 		if acceptedUser.Id.GetOpaqueId() == remoteUser.Id.OpaqueId && acceptedUser.Id.GetIdp() == remoteUser.Id.Idp {
 			return errors.New("json: user already added to accepted users")
 		}
 	}
-	m.model.AcceptedUsers[userKey] = append(m.model.AcceptedUsers[userKey], remoteUser)
+	m.model.AcceptedUsers[currUser.GetOpaqueId()] = append(m.model.AcceptedUsers[currUser.GetOpaqueId()], remoteUser)
 	if err := m.model.Save(); err != nil {
 		err = errors.Wrap(err, "json: error saving model")
 		return err

pkg/ocm/invite/manager/memory/memory.go

@@ -143,21 +143,27 @@ func (m *manager) AcceptInvite(ctx context.Context, invite *invitepb.InviteToken
 		return err
 	}
 
-	currUser := inviteToken.GetUserId().GetOpaqueId()
+	currUser := inviteToken.GetUserId()
+
+	// do not allow the user who created the token to accept it
+	if remoteUser.Id.Idp == currUser.Idp && remoteUser.Id.OpaqueId == currUser.OpaqueId {
+		return errors.New("memory: token creator and recipient are the same")
+	}
+
 	usersList, ok := m.AcceptedUsers.Load(currUser)
+	acceptedUsers := usersList.([]*userpb.User)
 	if ok {
-		acceptedUsers := usersList.([]*userpb.User)
 		for _, acceptedUser := range acceptedUsers {
 			if acceptedUser.Id.GetOpaqueId() == remoteUser.Id.OpaqueId && acceptedUser.Id.GetIdp() == remoteUser.Id.Idp {
 				return errors.New("memory: user already added to accepted users")
 			}
 		}
 
 		acceptedUsers = append(acceptedUsers, remoteUser)
-		m.AcceptedUsers.Store(currUser, acceptedUsers)
+		m.AcceptedUsers.Store(currUser.GetOpaqueId(), acceptedUsers)
 	} else {
 		acceptedUsers := []*userpb.User{remoteUser}
-		m.AcceptedUsers.Store(currUser, acceptedUsers)
+		m.AcceptedUsers.Store(currUser.GetOpaqueId(), acceptedUsers)
 	}
 	return nil
 }

pkg/storage/utils/localfs/upload.go

@@ -171,6 +171,16 @@ func (fs *localfs) NewUpload(ctx context.Context, info tusd.FileInfo) (upload tu
 		fs:       fs,
 	}
 
+	if !info.SizeIsDeferred && info.Size == 0 {
+		log.Debug().Interface("info", info).Msg("localfs: finishing upload for empty file")
+		// no need to create info file and finish directly
+		err := u.FinishUpload(ctx)
+		if err != nil {
+			return nil, err
+		}
+		return u, nil
+	}
+
 	// writeInfo creates the file by itself if necessary
 	err = u.writeInfo()
 	if err != nil {
@@ -307,10 +317,10 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) error {
 
 	err := os.Rename(upload.binPath, np)
 
-	// only delete the upload if it was successfully written to eos
+	// only delete the upload if it was successfully written to the fs
 	if err := os.Remove(upload.infoPath); err != nil {
 		log := appctx.GetLogger(ctx)
-		log.Err(err).Interface("info", upload.info).Msg("eos: could not delete upload info")
+		log.Err(err).Interface("info", upload.info).Msg("localfs: could not delete upload info")
 	}
 
 	// TODO: set mtime if specified in metadata

pkg/user/manager/rest/rest.go

@@ -235,6 +235,10 @@ func (m *manager) getUserByParam(ctx context.Context, param, val string) (map[st
 	if err != nil {
 		return nil, err
 	}
+	if len(responseData) == 0 {
+		return nil, errors.New("rest: no user found")
+	}
+
 	userData, ok := responseData[0].(map[string]interface{})
 	if !ok {
 		return nil, errors.New("rest: error in type assertion")