From 9cb36a1f92f644edb4dd43bc6baa1360bc74aa15 Mon Sep 17 00:00:00 2001 From: "A.Unger" Date: Wed, 10 Jun 2020 13:37:14 +0200 Subject: [PATCH 01/10] first working draft inline return working upload spare a few allocations provide missing parameter on InitiateUpload single file publicly shared download working filename on single shared file apply @PVince81 patch take into account depth improve base logic resolve append not assigned to same destination variable disable gocritic for long ifElse chains --- .golangci.yaml | 4 + .../services/gateway/publicshareprovider.go | 1 - .../grpc/services/gateway/storageprovider.go | 1 - .../publicstorageprovider.go | 304 ++++++++++++++++-- .../http/services/owncloud/ocdav/propfind.go | 75 ++++- internal/http/services/owncloud/ocdav/put.go | 70 ++-- .../http/services/owncloud/ocdav/versions.go | 2 +- 7 files changed, 389 insertions(+), 68 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6d59182f54..2530f8e60b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -12,6 +12,10 @@ issues: text: "SA1019:" linters: - staticcheck + - path: internal/http/services/owncloud/ocdav/propfind.go + text: "ifElseChain:" + linters: + - gocritic linters: enable: - bodyclose diff --git a/internal/grpc/services/gateway/publicshareprovider.go b/internal/grpc/services/gateway/publicshareprovider.go index d2fab6ff76..d005c3a390 100644 --- a/internal/grpc/services/gateway/publicshareprovider.go +++ b/internal/grpc/services/gateway/publicshareprovider.go @@ -64,7 +64,6 @@ func (s *svc) GetPublicShareByToken(ctx context.Context, req *link.GetPublicShar return nil, err } - // TODO the double call is not here res, err := driver.GetPublicShareByToken(ctx, req) if err != nil { return nil, err diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go index 55a197c896..578aad6c50 100644 --- a/internal/grpc/services/gateway/storageprovider.go +++ b/internal/grpc/services/gateway/storageprovider.go @@ -828,7 +828,6 @@ func (s *svc) UnsetArbitraryMetadata(ctx context.Context, req *provider.UnsetArb } func (s *svc) stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { - // TODO(refs) do we need to append home to every stat request? c, err := s.find(ctx, req.Ref) if err != nil { if _, ok := err.(errtypes.IsNotFound); ok { diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index ef4eae2bbf..fbd65bf8ff 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -21,17 +21,24 @@ package publicstorageprovider import ( "context" "fmt" + "net/url" "path" + "path/filepath" + "strconv" "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rgrpc" + "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" + "github.com/cs3org/reva/pkg/storage" + "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" - "github.com/rs/zerolog/log" "go.opencensus.io/trace" "google.golang.org/grpc" codes "google.golang.org/grpc/codes" @@ -43,16 +50,21 @@ func init() { } type config struct { - MountPath string `mapstructure:"mount_path"` - MountID string `mapstructure:"mount_id"` - GatewayAddr string `mapstructure:"gateway_addr"` - DriverAddr string `mapstructure:"driver_addr"` + MountPath string `mapstructure:"mount_path"` + MountID string `mapstructure:"mount_id"` + GatewayAddr string `mapstructure:"gateway_addr"` + Driver string `mapstructure:"driver"` + Drivers map[string]map[string]interface{} `mapstructure:"drivers"` + DriverAddr string `mapstructure:"driver_addr"` + DataServerURL string `mapstructure:"data_server_url"` + DataServerPrefix string `mapstructure:"data_server_prefix"` } type service struct { conf *config mountPath, mountID string gateway gateway.GatewayAPIClient + storage storage.FS } func (s *service) Close() error { @@ -60,7 +72,6 @@ func (s *service) Close() error { } func (s *service) UnprotectedEndpoints() []string { - // return []string{"/cs3.sharing.link.v1beta1.LinkAPI/GetPublicShareByToken"} return []string{} } @@ -87,6 +98,11 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { mountPath := c.MountPath mountID := c.MountID + fs, err := getFS(c) + if err != nil { + return nil, err + } + gateway, err := pool.GetGatewayServiceClient(c.GatewayAddr) if err != nil { return nil, err @@ -97,6 +113,7 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { mountPath: mountPath, mountID: mountID, gateway: gateway, + storage: fs, } return service, nil @@ -111,11 +128,182 @@ func (s *service) UnsetArbitraryMetadata(ctx context.Context, req *provider.Unse } func (s *service) InitiateFileDownload(ctx context.Context, req *provider.InitiateFileDownloadRequest) (*provider.InitiateFileDownloadResponse, error) { - return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") + statReq := &provider.StatRequest{Ref: req.Ref} + statRes, err := s.Stat(ctx, statReq) + if err != nil { + return &provider.InitiateFileDownloadResponse{ + Status: status.NewInternal(ctx, err, "gateway: error stating ref:"+req.Ref.String()), + }, nil + } + if statRes.Status.Code != rpc.Code_CODE_OK { + if statRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + return &provider.InitiateFileDownloadResponse{ + Status: status.NewNotFound(ctx, "gateway: file not found"), + }, nil + } + err := status.NewErrorFromCode(statRes.Status.Code, "gateway") + return &provider.InitiateFileDownloadResponse{ + Status: status.NewInternal(ctx, err, "gateway: error stating ref"), + }, nil + } + + req.Opaque = statRes.Info.Opaque + return s.initiateFileDownload(ctx, req) +} + +// PathTranslator encapsulates behavior that requires translation between external paths into internal paths. Internal +// path depend on the storage provider in use, hence a translation is needed. In the case of Public Links, the translation +// goes beyond a simple path conversion but instead, the token needs to be "expanded" to an internal path. Essentially +// transforming: +// "YzUTlrKrpswo/foldera/folderb/file.txt" +// into: +// "shared-folder-path/internal-folder/foldera/folderb/file.txt". +type PathTranslator struct { + dir string + base string + token string +} + +// NewPathTranslator creates a new PathTranslator. +func NewPathTranslator(p string) *PathTranslator { + return &PathTranslator{ + dir: filepath.Dir(p), + base: filepath.Base(p), + token: strings.Split(p, "/")[2], + } +} + +// Both, t.dir and tokenPath paths need to be merged: +// tokenPath = /oc/einstein/public-links +// t.dir = /public/ausGxuUePCOi/foldera/folderb/ +// res = /public-links/foldera/folderb/ +// this `res` will get then expanded taking into account the authenticated user and the storage: +// end = /einstein/files/public-links/foldera/folderb/ + +// TODO how would this behave with a storage other than owncloud? +// the current OC namespace looks like "/oc/einstein/public-links" +// and operations on it would be hardcoded string operations bound to +// the storage path. Either we need clearly defined paths from each storage, +// or else this would remain black magic. +func (s *service) initiateFileDownload(ctx context.Context, req *provider.InitiateFileDownloadRequest) (*provider.InitiateFileDownloadResponse, error) { + t := NewPathTranslator(req.Ref.GetPath()) + tokenPath, err := s.pathFromToken(ctx, t.token) + if err != nil { + return nil, err + } + + var destURL *url.URL + + // handle the case where a single file is shared publicly, for such case the url.Path might only be: "/public/ItYJjJNdiuEn" + // TODO the "/public" prefix might be configurable, although a prefix, nested paths should not be allowed, as in: `/public/level1/level2` values as prefix. + + _, ok := s.isSharedFile(ctx, tokenPath) + // check if the ref exists, if not, perhaps it is a publicly shared file + if ok { + base := strings.Join(strings.Split(tokenPath, "/")[3:], "/") + destURL, err = url.Parse(strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix, base}, "/")) + if err != nil { + return nil, err + } + } else if isOCStorage(tokenPath) { + base := strings.Join(strings.Split(tokenPath, "/")[3:], "/") + request := strings.Join(strings.Split(t.dir, "/")[3:], "/") + t.dir = strings.Join([]string{base, request}, "/") + + destURL, err = url.Parse(strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix, t.dir, t.base}, "/")) + if err != nil { + return nil, err + } + } + + return &provider.InitiateFileDownloadResponse{ + Opaque: req.Opaque, + Status: &rpc.Status{Code: rpc.Code_CODE_OK}, + DownloadEndpoint: destURL.String(), + Expose: true, + }, nil +} + +func isOCStorage(q string) bool { + return strings.HasPrefix(q, "/oc/") +} + +func (s *service) dataURL() (*url.URL, error) { + target := strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix}, "/") + targetURL, err := url.Parse(target) + if err != nil { + return nil, err + } + + return targetURL, nil +} + +func (s *service) uploadFullPath(ctx context.Context, refPath string) (string, error) { + var fullPath []string + // req.Ref.GetPath() = "/public/{token}/subfolderA/subfolderB/file.txt" + token := strings.Split(refPath, "/")[2] + + ref, err := s.refFromToken(ctx, token) + if err != nil { + return "", err + } + + sharedRootPath := strings.Split(ref.GetPath(), "/")[3:] // internal paths have the storage prefixed, i.e: "/oc/einstein/asdf/"" + fullPath = append(fullPath, append(sharedRootPath, strings.Split(refPath, "/")[3:]...)...) + // i.e: fullPath = /sharedFolder/subfolder/file.txt + return strings.Join(fullPath, "/"), nil } func (s *service) InitiateFileUpload(ctx context.Context, req *provider.InitiateFileUploadRequest) (*provider.InitiateFileUploadResponse, error) { - return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") + logger := appctx.GetLogger(ctx) + + fullPath, err := s.uploadFullPath(ctx, req.Ref.GetPath()) + if err != nil { + return nil, err + } + + ref := &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: fullPath, + }, + } + + targetURL, err := s.dataURL() + if err != nil { + return nil, err + } + + var uploadLength int64 + if req.Opaque != nil && req.Opaque.Map != nil && req.Opaque.Map["Upload-Length"] != nil { + var err error + uploadLength, err = strconv.ParseInt(string(req.Opaque.Map["Upload-Length"].Value), 10, 64) + if err != nil { + return &provider.InitiateFileUploadResponse{ + Status: status.NewInternal(ctx, err, "error parsing upload length"), + }, nil + } + } + + uploadID, err := s.storage.InitiateUpload(ctx, ref, uploadLength, nil) + if err != nil { + return &provider.InitiateFileUploadResponse{ + Status: status.NewInternal(ctx, err, "error getting upload id"), + }, nil + } + targetURL.Path = path.Join("/", targetURL.Path, uploadID) + + logger.Info().Str("data-server", targetURL.String()). + Str("fn", req.Ref.GetPath()). + Str("xs", fmt.Sprintf("%+v", "false")). + Msg("file upload") + res := &provider.InitiateFileUploadResponse{ + UploadEndpoint: targetURL.String(), + Status: status.NewOK(ctx), + Expose: true, + // AvailableChecksums: , + } + + return res, nil } func (s *service) GetPath(ctx context.Context, req *provider.GetPathRequest) (*provider.GetPathResponse, error) { @@ -160,32 +348,50 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide return nil, err } - statResponse, err := s.gateway.Stat( - ctx, - &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", pathFromToken, relativePath), - }, + var statResponse *provider.StatResponse + var ok bool + // the call has to be made to the gateway instead of the storage. + statResponse, err = s.gateway.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", pathFromToken, relativePath), }, - }) - if err != nil { - log.Error().Err(err).Msg("error during stat call") - return nil, err + }, + }) + if statResponse.Status.Code == rpc.Code_CODE_INTERNAL { + // the shared resource is a file, return the original error + // or ovewrite statResponse + if statResponse, ok = s.isSharedFile(ctx, pathFromToken); !ok { + return nil, err + } } - // we don't want to leak the path - statResponse.Info.Path = path.Join("/", tkn, relativePath) - - // if statResponse.Status.Code != rpc.Code_CODE_OK { - // if statResponse.Status.Code == rpc.Code_CODE_NOT_FOUND { - // // log.Warn().Str("path", refFromToken.GetPath()).Msgf("resource: `%v` not found", refFromToken.GetId()) - // } - // } + // prevent leaking internal paths + if statResponse.Info != nil { + statResponse.Info.Path = path.Join("/", tkn, relativePath) + } return statResponse, nil } +func (s *service) isSharedFile(ctx context.Context, loc string) (*provider.StatResponse, bool) { + statResponse, err := s.gateway.Stat(ctx, &provider.StatRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", loc), + }, + }, + }) + if err != nil { + return nil, false + } + if statResponse.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { + return nil, false + } + + return statResponse, true +} + func (s *service) ListContainerStream(req *provider.ListContainerStreamRequest, ss provider.ProviderAPI_ListContainerStreamServer) error { return gstatus.Errorf(codes.Unimplemented, "method not implemented") } @@ -304,7 +510,7 @@ func (s *service) trimMountPrefix(fn string) (string, error) { return "", errors.New(fmt.Sprintf("path=%q does not belong to this storage provider mount path=%q"+fn, s.mountPath)) } -// pathFromToken returns a reference from a public share token. +// pathFromToken returns the path for the publicly shared resource. func (s *service) pathFromToken(ctx context.Context, token string) (string, error) { driver, err := pool.GetPublicShareProviderClient(s.conf.DriverAddr) if err != nil { @@ -334,3 +540,45 @@ func (s *service) pathFromToken(ctx context.Context, token string) (string, erro return pathRes.Path, nil } + +// refFromToken returns the path for the publicly shared resource. +func (s *service) refFromToken(ctx context.Context, token string) (*provider.Reference, error) { + driver, err := pool.GetPublicShareProviderClient(s.conf.DriverAddr) + if err != nil { + return nil, err + } + + publicShareResponse, err := driver.GetPublicShare( + ctx, + &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Token{ + Token: token, + }, + }, + }, + ) + if err != nil { + return nil, err + } + + pathRes, err := s.gateway.GetPath(ctx, &provider.GetPathRequest{ + ResourceId: publicShareResponse.GetShare().GetResourceId(), + }) + if err != nil { + return nil, err + } + + return &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: pathRes.Path, + }, + }, nil +} + +func getFS(c *config) (storage.FS, error) { + if f, ok := registry.NewFuncs[c.Driver]; ok { + return f(c.Drivers[c.Driver]) + } + return nil, fmt.Errorf("driver not found: %s", c.Driver) +} diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 315f1bc0eb..42b2718a22 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -27,6 +27,7 @@ import ( "net/http" "net/url" "path" + "path/filepath" "strconv" "strings" "time" @@ -87,6 +88,8 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } + publiclySharedFile := (res.Info.Type == provider.ResourceType_RESOURCE_TYPE_FILE && strings.Contains(ctx.Value(ctxKeyBaseURI).(string), "public-files")) + if res.Status.Code != rpc.Code_CODE_OK { if res.Status.Code == rpc.Code_CODE_NOT_FOUND { log.Warn().Str("path", fn).Msg("resource not found") @@ -97,9 +100,9 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } - info := res.Info - infos := []*provider.ResourceInfo{info} - if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1" { + infos := []*provider.ResourceInfo{res.Info} + + if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1" { req := &provider.ListContainerRequest{ Ref: ref, } @@ -116,10 +119,10 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } infos = append(infos, res.Infos...) - } else if depth == "infinity" { + } else if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "infinity" { // FIXME: doesn't work cross-storage as the results will have the wrong paths! // use a stack to explore sub-containers breadth-first - stack := []string{info.Path} + stack := []string{res.Info.Path} for len(stack) > 0 { // retrieve path on top of stack path := stack[len(stack)-1] @@ -154,24 +157,37 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) // check sub-containers in reverse order and add them to the stack // the reversed order here will produce a more logical sorting of results for i := len(res.Infos) - 1; i >= 0; i-- { - //for i := range res.Infos { if res.Infos[i].Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { stack = append(stack, res.Infos[i].Path) } } } + } else if publiclySharedFile && depth == "1" { + infos = []*provider.ResourceInfo{} + // if the request is to a public link, we need to add yet another value for the file entry. + infos = append(infos, &provider.ResourceInfo{ + // append the shared as a container. Annex to OC10 standards. + Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, + Mtime: res.Info.Mtime, + }) + infos = append(infos, res.Info) + } else if publiclySharedFile && depth == "0" { + // this logic runs when uploading a file on a publicly shared folder. + infos = []*provider.ResourceInfo{} + infos = append(infos, res.Info) } - propRes, err := s.formatPropfind(ctx, &pf, infos, ns) + propRes, err := s.formatPropfind(ctx, &pf, infos, ns, publiclySharedFile) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } + w.Header().Set("DAV", "1, 3, extended-mkcol") w.Header().Set("Content-Type", "application/xml; charset=utf-8") // let clients know this collection supports tus.io POST requests to start uploads - if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && !s.c.DisableTus { + if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && !s.c.DisableTus { w.Header().Add("Access-Control-Expose-Headers", "Tus-Resumable, Tus-Version, Tus-Extension") w.Header().Set("Tus-Resumable", "1.0.0") w.Header().Set("Tus-Version", "1.0.0") @@ -214,10 +230,14 @@ func readPropfind(r io.Reader) (pf propfindXML, status int, err error) { return pf, 0, nil } -func (s *svc) formatPropfind(ctx context.Context, pf *propfindXML, mds []*provider.ResourceInfo, ns string) (string, error) { +func (s *svc) formatPropfind(ctx context.Context, pf *propfindXML, mds []*provider.ResourceInfo, ns string, publiclySharedFile bool) (string, error) { responses := make([]*responseXML, 0, len(mds)) + public := false for i := range mds { - res, err := s.mdToPropResponse(ctx, pf, mds[i], ns) + if publiclySharedFile && mds[i].Type == provider.ResourceType_RESOURCE_TYPE_FILE { + public = true + } + res, err := s.mdToPropResponse(ctx, pf, mds[i], ns, public) if err != nil { return "", err } @@ -259,10 +279,8 @@ func (s *svc) newProp(key, val string) *propertyXML { // mdToPropResponse converts the CS3 metadata into a webdav propesponse // ns is the CS3 namespace that needs to be removed from the CS3 path before // prefixing it with the baseURI -func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provider.ResourceInfo, ns string) (*responseXML, error) { - +func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provider.ResourceInfo, ns string, publiclySharedFile bool) (*responseXML, error) { md.Path = strings.TrimPrefix(md.Path, ns) - baseURI := ctx.Value(ctxKeyBaseURI).(string) ref := path.Join(baseURI, md.Path) @@ -270,8 +288,35 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide ref += "/" } - response := responseXML{ - Href: (&url.URL{Path: ref}).EscapedPath(), // url encode response.Href + base := "" + var response responseXML + + if publiclySharedFile { + // do a stat request in order to get the file name and append it to the request + client, err := s.getClient() + if err != nil { + return nil, err + } + + pathRes, err := client.GetPath(ctx, &provider.GetPathRequest{ + ResourceId: md.GetId(), + }) + if err != nil { + return nil, err + } + + // check whether md contains the basepath of the shared file + basePathRes := filepath.Base(pathRes.Path) + baseMD := filepath.Base(md.Path) + + // append the filename only if base is missing + if basePathRes != baseMD { + base = "/" + filepath.Base(pathRes.Path) + } + } + + response = responseXML{ + Href: (&url.URL{Path: ref + base}).EscapedPath(), // url encode response.Href Propstat: []propstatXML{}, } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 5a9a2be457..be6ff44f43 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -182,16 +182,15 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } } - info := sRes.Info - if info != nil && info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - log.Warn().Msg("resource is not a file") - w.WriteHeader(http.StatusConflict) - return - } + if sRes.Info != nil { + if sRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { + log.Warn().Msg("resource is not a file") + w.WriteHeader(http.StatusConflict) + return + } - if info != nil { clientETag := r.Header.Get("If-Match") - serverETag := info.Etag + serverETag := sRes.Info.Etag if clientETag != "" { if clientETag != serverETag { log.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") @@ -292,32 +291,59 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { return } + // apply mtime to the metadata if specified + if r.Header.Get("X-OC-Mtime") != "" { + sreq := &provider.SetArbitraryMetadataRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: fn}, + }, + ArbitraryMetadata: &provider.ArbitraryMetadata{ + Metadata: map[string]string{}, + }, + } + sreq.ArbitraryMetadata.Metadata["mtime"] = r.Header.Get("X-OC-Mtime") + res, err := client.SetArbitraryMetadata(ctx, sreq) + if err != nil { + log.Error().Err(err). + Str("mtime", r.Header.Get("X-OC-Mtime")). + Msg("error sending a grpc SetArbitraryMetadata request for setting mtime") + w.WriteHeader(http.StatusInternalServerError) + return + } + + if res.Status.Code != rpc.Code_CODE_OK { + log.Error().Err(err). + Str("mtime", r.Header.Get("X-OC-Mtime")). + Msgf("error sending a grpc SetArbitraryMetadata request for setting mtime, status %d", res.Status.Code) + w.WriteHeader(http.StatusInternalServerError) + return + } + + w.Header().Set("X-OC-Mtime", "accepted") + } + // stat again to check the new file's metadata - sRes, err = client.Stat(ctx, sReq) + sRes2, err := client.Stat(ctx, sReq) if err != nil { log.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } - if sRes.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes.Status.Code) + if sRes2.Status.Code != rpc.Code_CODE_OK { + log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes2.Status.Code) w.WriteHeader(http.StatusInternalServerError) return } - info2 := sRes.Info - - w.Header().Add("Content-Type", info2.MimeType) - w.Header().Set("ETag", info2.Etag) - w.Header().Set("OC-FileId", wrapResourceID(info2.Id)) - w.Header().Set("OC-ETag", info2.Etag) - t := utils.TSToTime(info2.Mtime) - lastModifiedString := t.Format(time.RFC1123Z) - w.Header().Set("Last-Modified", lastModifiedString) + w.Header().Add("Content-Type", sRes2.Info.MimeType) + w.Header().Set("ETag", sRes2.Info.Etag) + w.Header().Set("OC-FileId", wrapResourceID(sRes2.Info.Id)) + w.Header().Set("OC-ETag", sRes2.Info.Etag) + w.Header().Set("Last-Modified", utils.TSToTime(sRes2.Info.Mtime).Format(time.RFC1123Z)) - // file was new - if info == nil { + // file was new as it didn't exist during the first stat + if sRes.Info == nil { w.WriteHeader(http.StatusCreated) return } diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index ae3716e0c0..f54c466d6e 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -166,7 +166,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, infos = append(infos, vi) } - propRes, err := s.formatPropfind(ctx, &pf, infos, "") + propRes, err := s.formatPropfind(ctx, &pf, infos, "", false) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) From fab207783187a4875e7bb9f9dad332468b001673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 23 Jun 2020 16:53:05 +0200 Subject: [PATCH 02/10] refactor public links storageprovider to use gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../publicstorageprovider.go | 160 ++++++------------ 1 file changed, 48 insertions(+), 112 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index fbd65bf8ff..9494e35e53 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -24,7 +24,6 @@ import ( "net/url" "path" "path/filepath" - "strconv" "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -35,8 +34,6 @@ import ( "github.com/cs3org/reva/pkg/rgrpc" "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" - "github.com/cs3org/reva/pkg/storage" - "github.com/cs3org/reva/pkg/storage/fs/registry" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "go.opencensus.io/trace" @@ -50,21 +47,17 @@ func init() { } type config struct { - MountPath string `mapstructure:"mount_path"` - MountID string `mapstructure:"mount_id"` - GatewayAddr string `mapstructure:"gateway_addr"` - Driver string `mapstructure:"driver"` - Drivers map[string]map[string]interface{} `mapstructure:"drivers"` - DriverAddr string `mapstructure:"driver_addr"` - DataServerURL string `mapstructure:"data_server_url"` - DataServerPrefix string `mapstructure:"data_server_prefix"` + MountPath string `mapstructure:"mount_path"` + MountID string `mapstructure:"mount_id"` + GatewayAddr string `mapstructure:"gateway_addr"` + DataServerURL string `mapstructure:"data_server_url"` + DataServerPrefix string `mapstructure:"data_server_prefix"` } type service struct { conf *config mountPath, mountID string gateway gateway.GatewayAPIClient - storage storage.FS } func (s *service) Close() error { @@ -98,11 +91,6 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { mountPath := c.MountPath mountID := c.MountID - fs, err := getFS(c) - if err != nil { - return nil, err - } - gateway, err := pool.GetGatewayServiceClient(c.GatewayAddr) if err != nil { return nil, err @@ -113,7 +101,6 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) { mountPath: mountPath, mountID: mountID, gateway: gateway, - storage: fs, } return service, nil @@ -180,54 +167,44 @@ func NewPathTranslator(p string) *PathTranslator { // this `res` will get then expanded taking into account the authenticated user and the storage: // end = /einstein/files/public-links/foldera/folderb/ -// TODO how would this behave with a storage other than owncloud? -// the current OC namespace looks like "/oc/einstein/public-links" -// and operations on it would be hardcoded string operations bound to -// the storage path. Either we need clearly defined paths from each storage, -// or else this would remain black magic. func (s *service) initiateFileDownload(ctx context.Context, req *provider.InitiateFileDownloadRequest) (*provider.InitiateFileDownloadResponse, error) { - t := NewPathTranslator(req.Ref.GetPath()) - tokenPath, err := s.pathFromToken(ctx, t.token) + tkn, relativePath, err := s.unwrap(ctx, req.Ref) if err != nil { return nil, err } - var destURL *url.URL + originalPath, err := s.pathFromToken(ctx, tkn) + if err != nil { + return nil, err + } - // handle the case where a single file is shared publicly, for such case the url.Path might only be: "/public/ItYJjJNdiuEn" - // TODO the "/public" prefix might be configurable, although a prefix, nested paths should not be allowed, as in: `/public/level1/level2` values as prefix. + dReq := &provider.InitiateFileDownloadRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: path.Join("/", originalPath, relativePath)}, + }, + } - _, ok := s.isSharedFile(ctx, tokenPath) - // check if the ref exists, if not, perhaps it is a publicly shared file - if ok { - base := strings.Join(strings.Split(tokenPath, "/")[3:], "/") - destURL, err = url.Parse(strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix, base}, "/")) - if err != nil { - return nil, err - } - } else if isOCStorage(tokenPath) { - base := strings.Join(strings.Split(tokenPath, "/")[3:], "/") - request := strings.Join(strings.Split(t.dir, "/")[3:], "/") - t.dir = strings.Join([]string{base, request}, "/") + dRes, err := s.gateway.InitiateFileDownload(ctx, dReq) + if err != nil { + return &provider.InitiateFileDownloadResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling InitiateFileDownload"), + }, nil + } - destURL, err = url.Parse(strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix, t.dir, t.base}, "/")) - if err != nil { - return nil, err - } + if dRes.Status.Code != rpc.Code_CODE_OK { + return &provider.InitiateFileDownloadResponse{ + Status: dRes.Status, + }, nil } return &provider.InitiateFileDownloadResponse{ Opaque: req.Opaque, - Status: &rpc.Status{Code: rpc.Code_CODE_OK}, - DownloadEndpoint: destURL.String(), - Expose: true, + Status: dRes.Status, + DownloadEndpoint: dRes.DownloadEndpoint, + Expose: true, // TODO set to false, leave data provider lookup to the datagateway }, nil } -func isOCStorage(q string) bool { - return strings.HasPrefix(q, "/oc/") -} - func (s *service) dataURL() (*url.URL, error) { target := strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix}, "/") targetURL, err := url.Parse(target) @@ -243,12 +220,12 @@ func (s *service) uploadFullPath(ctx context.Context, refPath string) (string, e // req.Ref.GetPath() = "/public/{token}/subfolderA/subfolderB/file.txt" token := strings.Split(refPath, "/")[2] - ref, err := s.refFromToken(ctx, token) + originalPath, err := s.pathFromToken(ctx, token) if err != nil { return "", err } - sharedRootPath := strings.Split(ref.GetPath(), "/")[3:] // internal paths have the storage prefixed, i.e: "/oc/einstein/asdf/"" + sharedRootPath := strings.Split(originalPath, "/")[3:] // internal paths have the storage prefixed, i.e: "/oc/einstein/asdf/"" fullPath = append(fullPath, append(sharedRootPath, strings.Split(refPath, "/")[3:]...)...) // i.e: fullPath = /sharedFolder/subfolder/file.txt return strings.Join(fullPath, "/"), nil @@ -273,34 +250,35 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate return nil, err } - var uploadLength int64 - if req.Opaque != nil && req.Opaque.Map != nil && req.Opaque.Map["Upload-Length"] != nil { - var err error - uploadLength, err = strconv.ParseInt(string(req.Opaque.Map["Upload-Length"].Value), 10, 64) - if err != nil { - return &provider.InitiateFileUploadResponse{ - Status: status.NewInternal(ctx, err, "error parsing upload length"), - }, nil - } + uReq := &provider.InitiateFileUploadRequest{ + Ref: ref, + Opaque: req.Opaque, } - uploadID, err := s.storage.InitiateUpload(ctx, ref, uploadLength, nil) + uRes, err := s.gateway.InitiateFileUpload(ctx, uReq) if err != nil { return &provider.InitiateFileUploadResponse{ - Status: status.NewInternal(ctx, err, "error getting upload id"), + Status: status.NewInternal(ctx, err, "gateway: error calling InitiateFileUpload"), + }, nil + } + + if uRes.Status.Code != rpc.Code_CODE_OK { + return &provider.InitiateFileUploadResponse{ + Status: uRes.Status, }, nil } - targetURL.Path = path.Join("/", targetURL.Path, uploadID) logger.Info().Str("data-server", targetURL.String()). Str("fn", req.Ref.GetPath()). Str("xs", fmt.Sprintf("%+v", "false")). Msg("file upload") + res := &provider.InitiateFileUploadResponse{ - UploadEndpoint: targetURL.String(), - Status: status.NewOK(ctx), - Expose: true, - // AvailableChecksums: , + UploadEndpoint: uRes.UploadEndpoint, + Status: uRes.Status, + AvailableChecksums: uRes.AvailableChecksums, + Opaque: uRes.Opaque, + Expose: true, // TODO set to false, leave data provider lookup to the datagateway } return res, nil @@ -512,7 +490,7 @@ func (s *service) trimMountPrefix(fn string) (string, error) { // pathFromToken returns the path for the publicly shared resource. func (s *service) pathFromToken(ctx context.Context, token string) (string, error) { - driver, err := pool.GetPublicShareProviderClient(s.conf.DriverAddr) + driver, err := pool.GetGatewayServiceClient(s.conf.GatewayAddr) if err != nil { return "", err } @@ -540,45 +518,3 @@ func (s *service) pathFromToken(ctx context.Context, token string) (string, erro return pathRes.Path, nil } - -// refFromToken returns the path for the publicly shared resource. -func (s *service) refFromToken(ctx context.Context, token string) (*provider.Reference, error) { - driver, err := pool.GetPublicShareProviderClient(s.conf.DriverAddr) - if err != nil { - return nil, err - } - - publicShareResponse, err := driver.GetPublicShare( - ctx, - &link.GetPublicShareRequest{ - Ref: &link.PublicShareReference{ - Spec: &link.PublicShareReference_Token{ - Token: token, - }, - }, - }, - ) - if err != nil { - return nil, err - } - - pathRes, err := s.gateway.GetPath(ctx, &provider.GetPathRequest{ - ResourceId: publicShareResponse.GetShare().GetResourceId(), - }) - if err != nil { - return nil, err - } - - return &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: pathRes.Path, - }, - }, nil -} - -func getFS(c *config) (storage.FS, error) { - if f, ok := registry.NewFuncs[c.Driver]; ok { - return f(c.Drivers[c.Driver]) - } - return nil, fmt.Errorf("driver not found: %s", c.Driver) -} From b1bbca2e53f40b03643a9982fad4a6fd4f0d5ace Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Jun 2020 17:25:19 +0200 Subject: [PATCH 03/10] Fix public link upload --- .../publicstorageprovider.go | 38 +++---------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 9494e35e53..5c30a7134b 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -30,7 +30,6 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rgrpc" "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" @@ -215,43 +214,21 @@ func (s *service) dataURL() (*url.URL, error) { return targetURL, nil } -func (s *service) uploadFullPath(ctx context.Context, refPath string) (string, error) { - var fullPath []string - // req.Ref.GetPath() = "/public/{token}/subfolderA/subfolderB/file.txt" - token := strings.Split(refPath, "/")[2] - - originalPath, err := s.pathFromToken(ctx, token) - if err != nil { - return "", err - } - - sharedRootPath := strings.Split(originalPath, "/")[3:] // internal paths have the storage prefixed, i.e: "/oc/einstein/asdf/"" - fullPath = append(fullPath, append(sharedRootPath, strings.Split(refPath, "/")[3:]...)...) - // i.e: fullPath = /sharedFolder/subfolder/file.txt - return strings.Join(fullPath, "/"), nil -} - func (s *service) InitiateFileUpload(ctx context.Context, req *provider.InitiateFileUploadRequest) (*provider.InitiateFileUploadResponse, error) { - logger := appctx.GetLogger(ctx) - - fullPath, err := s.uploadFullPath(ctx, req.Ref.GetPath()) + tkn, relativePath, err := s.unwrap(ctx, req.Ref) if err != nil { return nil, err } - ref := &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: fullPath, - }, - } - - targetURL, err := s.dataURL() + originalPath, err := s.pathFromToken(ctx, tkn) if err != nil { return nil, err } uReq := &provider.InitiateFileUploadRequest{ - Ref: ref, + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{Path: path.Join("/", originalPath, relativePath)}, + }, Opaque: req.Opaque, } @@ -268,11 +245,6 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate }, nil } - logger.Info().Str("data-server", targetURL.String()). - Str("fn", req.Ref.GetPath()). - Str("xs", fmt.Sprintf("%+v", "false")). - Msg("file upload") - res := &provider.InitiateFileUploadResponse{ UploadEndpoint: uRes.UploadEndpoint, Status: uRes.Status, From a7fee225564fa382361837f6dd857306b23dce9d Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Jun 2020 17:28:00 +0200 Subject: [PATCH 04/10] Remove path translators in public provider --- .../publicstorageprovider.go | 42 ++----------------- 1 file changed, 3 insertions(+), 39 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 5c30a7134b..d7b28b7265 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -21,9 +21,7 @@ package publicstorageprovider import ( "context" "fmt" - "net/url" "path" - "path/filepath" "strings" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -46,11 +44,9 @@ func init() { } type config struct { - MountPath string `mapstructure:"mount_path"` - MountID string `mapstructure:"mount_id"` - GatewayAddr string `mapstructure:"gateway_addr"` - DataServerURL string `mapstructure:"data_server_url"` - DataServerPrefix string `mapstructure:"data_server_prefix"` + MountPath string `mapstructure:"mount_path"` + MountID string `mapstructure:"mount_id"` + GatewayAddr string `mapstructure:"gateway_addr"` } type service struct { @@ -137,28 +133,6 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia return s.initiateFileDownload(ctx, req) } -// PathTranslator encapsulates behavior that requires translation between external paths into internal paths. Internal -// path depend on the storage provider in use, hence a translation is needed. In the case of Public Links, the translation -// goes beyond a simple path conversion but instead, the token needs to be "expanded" to an internal path. Essentially -// transforming: -// "YzUTlrKrpswo/foldera/folderb/file.txt" -// into: -// "shared-folder-path/internal-folder/foldera/folderb/file.txt". -type PathTranslator struct { - dir string - base string - token string -} - -// NewPathTranslator creates a new PathTranslator. -func NewPathTranslator(p string) *PathTranslator { - return &PathTranslator{ - dir: filepath.Dir(p), - base: filepath.Base(p), - token: strings.Split(p, "/")[2], - } -} - // Both, t.dir and tokenPath paths need to be merged: // tokenPath = /oc/einstein/public-links // t.dir = /public/ausGxuUePCOi/foldera/folderb/ @@ -204,16 +178,6 @@ func (s *service) initiateFileDownload(ctx context.Context, req *provider.Initia }, nil } -func (s *service) dataURL() (*url.URL, error) { - target := strings.Join([]string{s.conf.DataServerURL, s.conf.DataServerPrefix}, "/") - targetURL, err := url.Parse(target) - if err != nil { - return nil, err - } - - return targetURL, nil -} - func (s *service) InitiateFileUpload(ctx context.Context, req *provider.InitiateFileUploadRequest) (*provider.InitiateFileUploadResponse, error) { tkn, relativePath, err := s.unwrap(ctx, req.Ref) if err != nil { From bb3a921e0eb3e4620e8b67b8df975b08f0768198 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Jun 2020 17:43:43 +0200 Subject: [PATCH 05/10] Implement public provider CreateContainer --- .../publicstorageprovider.go | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index d7b28b7265..a5936db269 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -233,7 +233,37 @@ func (s *service) CreateHome(ctx context.Context, req *provider.CreateHomeReques } func (s *service) CreateContainer(ctx context.Context, req *provider.CreateContainerRequest) (*provider.CreateContainerResponse, error) { - return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") + ctx, span := trace.StartSpan(ctx, "CreateContainer") + defer span.End() + + span.AddAttributes( + trace.StringAttribute("ref", req.Ref.String()), + ) + + tkn, relativePath, err := s.unwrap(ctx, req.Ref) + if err != nil { + return nil, err + } + + pathFromToken, err := s.pathFromToken(ctx, tkn) + if err != nil { + return nil, err + } + + var res *provider.CreateContainerResponse + // the call has to be made to the gateway instead of the storage. + res, err = s.gateway.CreateContainer(ctx, &provider.CreateContainerRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", pathFromToken, relativePath), + }, + }, + }) + if res.Status.Code == rpc.Code_CODE_INTERNAL { + return res, nil + } + + return res, nil } func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*provider.DeleteResponse, error) { From c2d199c7649c7f78f372a3a5efe3aa7dd42bda9a Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Jun 2020 17:45:26 +0200 Subject: [PATCH 06/10] Implement public provider delete method --- .../publicstorageprovider.go | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index a5936db269..0fef7c8c87 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -267,7 +267,37 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta } func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*provider.DeleteResponse, error) { - return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") + ctx, span := trace.StartSpan(ctx, "Delete") + defer span.End() + + span.AddAttributes( + trace.StringAttribute("ref", req.Ref.String()), + ) + + tkn, relativePath, err := s.unwrap(ctx, req.Ref) + if err != nil { + return nil, err + } + + pathFromToken, err := s.pathFromToken(ctx, tkn) + if err != nil { + return nil, err + } + + var res *provider.DeleteResponse + // the call has to be made to the gateway instead of the storage. + res, err = s.gateway.Delete(ctx, &provider.DeleteRequest{ + Ref: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", pathFromToken, relativePath), + }, + }, + }) + if res.Status.Code == rpc.Code_CODE_INTERNAL { + return res, nil + } + + return res, nil } func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) { From bebec8b917662277e1abc36b3de21396c98bd2dc Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Tue, 23 Jun 2020 17:53:46 +0200 Subject: [PATCH 07/10] Implement public storage move operation --- .../publicstorageprovider.go | 55 ++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 0fef7c8c87..ea00b7311a 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -301,7 +301,60 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro } func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) { - return nil, gstatus.Errorf(codes.Unimplemented, "method not implemented") + ctx, span := trace.StartSpan(ctx, "Move") + defer span.End() + + span.AddAttributes( + trace.StringAttribute("source", req.Source.String()), + trace.StringAttribute("destination", req.Destination.String()), + ) + + tknSource, relativePathSource, err := s.unwrap(ctx, req.Source) + if err != nil { + return nil, err + } + + tknDest, relativePathDest, err := s.unwrap(ctx, req.Destination) + if err != nil { + return nil, err + } + + if tknSource != tknDest { + return &provider.MoveResponse{ + Status: status.NewInvalidArg(ctx, "Source and destination token must be the same"), + }, nil + } + + originalPathSource, err := s.pathFromToken(ctx, tknSource) + if err != nil { + return nil, err + } + + // FIXME: maybe there's a shortcut possible here using the source path + originalPathDest, err := s.pathFromToken(ctx, tknDest) + if err != nil { + return nil, err + } + + var res *provider.MoveResponse + // the call has to be made to the gateway instead of the storage. + res, err = s.gateway.Move(ctx, &provider.MoveRequest{ + Source: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", originalPathSource, relativePathSource), + }, + }, + Destination: &provider.Reference{ + Spec: &provider.Reference_Path{ + Path: path.Join("/", originalPathDest, relativePathDest), + }, + }, + }) + if res.Status.Code == rpc.Code_CODE_INTERNAL { + return res, nil + } + + return res, nil } func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) { From f412c6330542840f8453b09fe0a8ec7d0dd7f035 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 24 Jun 2020 11:37:47 +0200 Subject: [PATCH 08/10] Refactor public provider path translation Move all path translation logic into a single private method to make it easier to reuse. --- .../publicstorageprovider.go | 105 +++++++----------- 1 file changed, 39 insertions(+), 66 deletions(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index ea00b7311a..31acd41b1c 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -28,6 +28,7 @@ import ( rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rgrpc" "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" @@ -133,6 +134,31 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia return s.initiateFileDownload(ctx, req) } +func (s *service) translatePublicRefToCS3Ref(ctx context.Context, ref *provider.Reference) (*provider.Reference, string, error) { + log := appctx.GetLogger(ctx) + tkn, relativePath, err := s.unwrap(ctx, ref) + if err != nil { + return nil, "", err + } + + originalPath, err := s.pathFromToken(ctx, tkn) + if err != nil { + return nil, "", err + } + + cs3Ref := &provider.Reference{ + Spec: &provider.Reference_Path{Path: path.Join("/", originalPath, relativePath)}, + } + log.Debug(). + Interface("sourceRef", ref). + Interface("cs3Ref", cs3Ref). + Str("tkn", tkn). + Str("originalPath", originalPath). + Str("relativePath", relativePath). + Msg("translatePublicRefToCS3Ref") + return cs3Ref, tkn, nil +} + // Both, t.dir and tokenPath paths need to be merged: // tokenPath = /oc/einstein/public-links // t.dir = /public/ausGxuUePCOi/foldera/folderb/ @@ -141,20 +167,12 @@ func (s *service) InitiateFileDownload(ctx context.Context, req *provider.Initia // end = /einstein/files/public-links/foldera/folderb/ func (s *service) initiateFileDownload(ctx context.Context, req *provider.InitiateFileDownloadRequest) (*provider.InitiateFileDownloadResponse, error) { - tkn, relativePath, err := s.unwrap(ctx, req.Ref) - if err != nil { - return nil, err - } - - originalPath, err := s.pathFromToken(ctx, tkn) + cs3Ref, _, err := s.translatePublicRefToCS3Ref(ctx, req.Ref) if err != nil { return nil, err } - dReq := &provider.InitiateFileDownloadRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: path.Join("/", originalPath, relativePath)}, - }, + Ref: cs3Ref, } dRes, err := s.gateway.InitiateFileDownload(ctx, dReq) @@ -179,20 +197,12 @@ func (s *service) initiateFileDownload(ctx context.Context, req *provider.Initia } func (s *service) InitiateFileUpload(ctx context.Context, req *provider.InitiateFileUploadRequest) (*provider.InitiateFileUploadResponse, error) { - tkn, relativePath, err := s.unwrap(ctx, req.Ref) + cs3Ref, _, err := s.translatePublicRefToCS3Ref(ctx, req.Ref) if err != nil { return nil, err } - - originalPath, err := s.pathFromToken(ctx, tkn) - if err != nil { - return nil, err - } - uReq := &provider.InitiateFileUploadRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: path.Join("/", originalPath, relativePath)}, - }, + Ref: cs3Ref, Opaque: req.Opaque, } @@ -240,12 +250,7 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta trace.StringAttribute("ref", req.Ref.String()), ) - tkn, relativePath, err := s.unwrap(ctx, req.Ref) - if err != nil { - return nil, err - } - - pathFromToken, err := s.pathFromToken(ctx, tkn) + cs3Ref, _, err := s.translatePublicRefToCS3Ref(ctx, req.Ref) if err != nil { return nil, err } @@ -253,11 +258,7 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta var res *provider.CreateContainerResponse // the call has to be made to the gateway instead of the storage. res, err = s.gateway.CreateContainer(ctx, &provider.CreateContainerRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", pathFromToken, relativePath), - }, - }, + Ref: cs3Ref, }) if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil @@ -274,12 +275,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro trace.StringAttribute("ref", req.Ref.String()), ) - tkn, relativePath, err := s.unwrap(ctx, req.Ref) - if err != nil { - return nil, err - } - - pathFromToken, err := s.pathFromToken(ctx, tkn) + cs3Ref, _, err := s.translatePublicRefToCS3Ref(ctx, req.Ref) if err != nil { return nil, err } @@ -287,11 +283,7 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro var res *provider.DeleteResponse // the call has to be made to the gateway instead of the storage. res, err = s.gateway.Delete(ctx, &provider.DeleteRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", pathFromToken, relativePath), - }, - }, + Ref: cs3Ref, }) if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil @@ -309,12 +301,12 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide trace.StringAttribute("destination", req.Destination.String()), ) - tknSource, relativePathSource, err := s.unwrap(ctx, req.Source) + cs3RefSource, tknSource, err := s.translatePublicRefToCS3Ref(ctx, req.Source) if err != nil { return nil, err } - - tknDest, relativePathDest, err := s.unwrap(ctx, req.Destination) + // FIXME: maybe there's a shortcut possible here using the source path + cs3RefDestination, tknDest, err := s.translatePublicRefToCS3Ref(ctx, req.Destination) if err != nil { return nil, err } @@ -325,30 +317,11 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide }, nil } - originalPathSource, err := s.pathFromToken(ctx, tknSource) - if err != nil { - return nil, err - } - - // FIXME: maybe there's a shortcut possible here using the source path - originalPathDest, err := s.pathFromToken(ctx, tknDest) - if err != nil { - return nil, err - } - var res *provider.MoveResponse // the call has to be made to the gateway instead of the storage. res, err = s.gateway.Move(ctx, &provider.MoveRequest{ - Source: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", originalPathSource, relativePathSource), - }, - }, - Destination: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", originalPathDest, relativePathDest), - }, - }, + Source: cs3RefSource, + Destination: cs3RefDestination, }) if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil From 916d1274f6c66a00f96b1821baf4fcafc848144d Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 24 Jun 2020 12:22:44 +0200 Subject: [PATCH 09/10] Add public provider error response to gateway calls Whenever the actual storage operation on the gateway fails, return a response to the caller including the error. --- .../publicstorageprovider.go | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 31acd41b1c..481f0b0f23 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -260,6 +260,11 @@ func (s *service) CreateContainer(ctx context.Context, req *provider.CreateConta res, err = s.gateway.CreateContainer(ctx, &provider.CreateContainerRequest{ Ref: cs3Ref, }) + if err != nil { + return &provider.CreateContainerResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling CreateContainer for ref:"+req.Ref.String()), + }, nil + } if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil } @@ -285,6 +290,11 @@ func (s *service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro res, err = s.gateway.Delete(ctx, &provider.DeleteRequest{ Ref: cs3Ref, }) + if err != nil { + return &provider.DeleteResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling Delete for ref:"+req.Ref.String()), + }, nil + } if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil } @@ -323,6 +333,11 @@ func (s *service) Move(ctx context.Context, req *provider.MoveRequest) (*provide Source: cs3RefSource, Destination: cs3RefDestination, }) + if err != nil { + return &provider.MoveResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling Move for source ref "+req.Source.String()+" to destination ref "+req.Destination.String()), + }, nil + } if res.Status.Code == rpc.Code_CODE_INTERNAL { return res, nil } @@ -358,6 +373,11 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide }, }, }) + if err != nil { + return &provider.StatResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling Stat for ref:"+req.Ref.String()), + }, nil + } if statResponse.Status.Code == rpc.Code_CODE_INTERNAL { // the shared resource is a file, return the original error // or ovewrite statResponse @@ -418,7 +438,9 @@ func (s *service) ListContainer(ctx context.Context, req *provider.ListContainer }, ) if err != nil { - return nil, err + return &provider.ListContainerResponse{ + Status: status.NewInternal(ctx, err, "gateway: error calling ListContainer for ref:"+req.Ref.String()), + }, nil } for i := range listContainerR.Infos { From a2095f003a15e3779607b291f611c9dadbaeac3c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 24 Jun 2020 16:56:04 +0200 Subject: [PATCH 10/10] Remove single file share code Remove code for non-working single file share implementation --- .golangci.yaml | 4 - .../publicstorageprovider.go | 30 +------- .../http/services/owncloud/ocdav/propfind.go | 75 ++++--------------- internal/http/services/owncloud/ocdav/put.go | 70 ++++++----------- .../http/services/owncloud/ocdav/versions.go | 2 +- 5 files changed, 40 insertions(+), 141 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 2530f8e60b..6d59182f54 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -12,10 +12,6 @@ issues: text: "SA1019:" linters: - staticcheck - - path: internal/http/services/owncloud/ocdav/propfind.go - text: "ifElseChain:" - linters: - - gocritic linters: enable: - bodyclose diff --git a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go index 481f0b0f23..3bafe7fae9 100644 --- a/internal/grpc/services/publicstorageprovider/publicstorageprovider.go +++ b/internal/grpc/services/publicstorageprovider/publicstorageprovider.go @@ -358,18 +358,17 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide return nil, err } - pathFromToken, err := s.pathFromToken(ctx, tkn) + originalPath, err := s.pathFromToken(ctx, tkn) if err != nil { return nil, err } var statResponse *provider.StatResponse - var ok bool // the call has to be made to the gateway instead of the storage. statResponse, err = s.gateway.Stat(ctx, &provider.StatRequest{ Ref: &provider.Reference{ Spec: &provider.Reference_Path{ - Path: path.Join("/", pathFromToken, relativePath), + Path: path.Join("/", originalPath, relativePath), }, }, }) @@ -378,13 +377,6 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide Status: status.NewInternal(ctx, err, "gateway: error calling Stat for ref:"+req.Ref.String()), }, nil } - if statResponse.Status.Code == rpc.Code_CODE_INTERNAL { - // the shared resource is a file, return the original error - // or ovewrite statResponse - if statResponse, ok = s.isSharedFile(ctx, pathFromToken); !ok { - return nil, err - } - } // prevent leaking internal paths if statResponse.Info != nil { @@ -394,24 +386,6 @@ func (s *service) Stat(ctx context.Context, req *provider.StatRequest) (*provide return statResponse, nil } -func (s *service) isSharedFile(ctx context.Context, loc string) (*provider.StatResponse, bool) { - statResponse, err := s.gateway.Stat(ctx, &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{ - Path: path.Join("/", loc), - }, - }, - }) - if err != nil { - return nil, false - } - if statResponse.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { - return nil, false - } - - return statResponse, true -} - func (s *service) ListContainerStream(req *provider.ListContainerStreamRequest, ss provider.ProviderAPI_ListContainerStreamServer) error { return gstatus.Errorf(codes.Unimplemented, "method not implemented") } diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 42b2718a22..315f1bc0eb 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -27,7 +27,6 @@ import ( "net/http" "net/url" "path" - "path/filepath" "strconv" "strings" "time" @@ -88,8 +87,6 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } - publiclySharedFile := (res.Info.Type == provider.ResourceType_RESOURCE_TYPE_FILE && strings.Contains(ctx.Value(ctxKeyBaseURI).(string), "public-files")) - if res.Status.Code != rpc.Code_CODE_OK { if res.Status.Code == rpc.Code_CODE_NOT_FOUND { log.Warn().Str("path", fn).Msg("resource not found") @@ -100,9 +97,9 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } - infos := []*provider.ResourceInfo{res.Info} - - if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1" { + info := res.Info + infos := []*provider.ResourceInfo{info} + if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1" { req := &provider.ListContainerRequest{ Ref: ref, } @@ -119,10 +116,10 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } infos = append(infos, res.Infos...) - } else if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "infinity" { + } else if depth == "infinity" { // FIXME: doesn't work cross-storage as the results will have the wrong paths! // use a stack to explore sub-containers breadth-first - stack := []string{res.Info.Path} + stack := []string{info.Path} for len(stack) > 0 { // retrieve path on top of stack path := stack[len(stack)-1] @@ -157,37 +154,24 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) // check sub-containers in reverse order and add them to the stack // the reversed order here will produce a more logical sorting of results for i := len(res.Infos) - 1; i >= 0; i-- { + //for i := range res.Infos { if res.Infos[i].Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { stack = append(stack, res.Infos[i].Path) } } } - } else if publiclySharedFile && depth == "1" { - infos = []*provider.ResourceInfo{} - // if the request is to a public link, we need to add yet another value for the file entry. - infos = append(infos, &provider.ResourceInfo{ - // append the shared as a container. Annex to OC10 standards. - Type: provider.ResourceType_RESOURCE_TYPE_CONTAINER, - Mtime: res.Info.Mtime, - }) - infos = append(infos, res.Info) - } else if publiclySharedFile && depth == "0" { - // this logic runs when uploading a file on a publicly shared folder. - infos = []*provider.ResourceInfo{} - infos = append(infos, res.Info) } - propRes, err := s.formatPropfind(ctx, &pf, infos, ns, publiclySharedFile) + propRes, err := s.formatPropfind(ctx, &pf, infos, ns) if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError) return } - w.Header().Set("DAV", "1, 3, extended-mkcol") w.Header().Set("Content-Type", "application/xml; charset=utf-8") // let clients know this collection supports tus.io POST requests to start uploads - if res.Info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && !s.c.DisableTus { + if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && !s.c.DisableTus { w.Header().Add("Access-Control-Expose-Headers", "Tus-Resumable, Tus-Version, Tus-Extension") w.Header().Set("Tus-Resumable", "1.0.0") w.Header().Set("Tus-Version", "1.0.0") @@ -230,14 +214,10 @@ func readPropfind(r io.Reader) (pf propfindXML, status int, err error) { return pf, 0, nil } -func (s *svc) formatPropfind(ctx context.Context, pf *propfindXML, mds []*provider.ResourceInfo, ns string, publiclySharedFile bool) (string, error) { +func (s *svc) formatPropfind(ctx context.Context, pf *propfindXML, mds []*provider.ResourceInfo, ns string) (string, error) { responses := make([]*responseXML, 0, len(mds)) - public := false for i := range mds { - if publiclySharedFile && mds[i].Type == provider.ResourceType_RESOURCE_TYPE_FILE { - public = true - } - res, err := s.mdToPropResponse(ctx, pf, mds[i], ns, public) + res, err := s.mdToPropResponse(ctx, pf, mds[i], ns) if err != nil { return "", err } @@ -279,8 +259,10 @@ func (s *svc) newProp(key, val string) *propertyXML { // mdToPropResponse converts the CS3 metadata into a webdav propesponse // ns is the CS3 namespace that needs to be removed from the CS3 path before // prefixing it with the baseURI -func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provider.ResourceInfo, ns string, publiclySharedFile bool) (*responseXML, error) { +func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provider.ResourceInfo, ns string) (*responseXML, error) { + md.Path = strings.TrimPrefix(md.Path, ns) + baseURI := ctx.Value(ctxKeyBaseURI).(string) ref := path.Join(baseURI, md.Path) @@ -288,35 +270,8 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide ref += "/" } - base := "" - var response responseXML - - if publiclySharedFile { - // do a stat request in order to get the file name and append it to the request - client, err := s.getClient() - if err != nil { - return nil, err - } - - pathRes, err := client.GetPath(ctx, &provider.GetPathRequest{ - ResourceId: md.GetId(), - }) - if err != nil { - return nil, err - } - - // check whether md contains the basepath of the shared file - basePathRes := filepath.Base(pathRes.Path) - baseMD := filepath.Base(md.Path) - - // append the filename only if base is missing - if basePathRes != baseMD { - base = "/" + filepath.Base(pathRes.Path) - } - } - - response = responseXML{ - Href: (&url.URL{Path: ref + base}).EscapedPath(), // url encode response.Href + response := responseXML{ + Href: (&url.URL{Path: ref}).EscapedPath(), // url encode response.Href Propstat: []propstatXML{}, } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index be6ff44f43..5a9a2be457 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -182,15 +182,16 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } } - if sRes.Info != nil { - if sRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - log.Warn().Msg("resource is not a file") - w.WriteHeader(http.StatusConflict) - return - } + info := sRes.Info + if info != nil && info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { + log.Warn().Msg("resource is not a file") + w.WriteHeader(http.StatusConflict) + return + } + if info != nil { clientETag := r.Header.Get("If-Match") - serverETag := sRes.Info.Etag + serverETag := info.Etag if clientETag != "" { if clientETag != serverETag { log.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") @@ -291,59 +292,32 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { return } - // apply mtime to the metadata if specified - if r.Header.Get("X-OC-Mtime") != "" { - sreq := &provider.SetArbitraryMetadataRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fn}, - }, - ArbitraryMetadata: &provider.ArbitraryMetadata{ - Metadata: map[string]string{}, - }, - } - sreq.ArbitraryMetadata.Metadata["mtime"] = r.Header.Get("X-OC-Mtime") - res, err := client.SetArbitraryMetadata(ctx, sreq) - if err != nil { - log.Error().Err(err). - Str("mtime", r.Header.Get("X-OC-Mtime")). - Msg("error sending a grpc SetArbitraryMetadata request for setting mtime") - w.WriteHeader(http.StatusInternalServerError) - return - } - - if res.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err). - Str("mtime", r.Header.Get("X-OC-Mtime")). - Msgf("error sending a grpc SetArbitraryMetadata request for setting mtime, status %d", res.Status.Code) - w.WriteHeader(http.StatusInternalServerError) - return - } - - w.Header().Set("X-OC-Mtime", "accepted") - } - // stat again to check the new file's metadata - sRes2, err := client.Stat(ctx, sReq) + sRes, err = client.Stat(ctx, sReq) if err != nil { log.Error().Err(err).Msg("error sending grpc stat request") w.WriteHeader(http.StatusInternalServerError) return } - if sRes2.Status.Code != rpc.Code_CODE_OK { - log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes2.Status.Code) + if sRes.Status.Code != rpc.Code_CODE_OK { + log.Error().Err(err).Msgf("error status %d when sending grpc stat request", sRes.Status.Code) w.WriteHeader(http.StatusInternalServerError) return } - w.Header().Add("Content-Type", sRes2.Info.MimeType) - w.Header().Set("ETag", sRes2.Info.Etag) - w.Header().Set("OC-FileId", wrapResourceID(sRes2.Info.Id)) - w.Header().Set("OC-ETag", sRes2.Info.Etag) - w.Header().Set("Last-Modified", utils.TSToTime(sRes2.Info.Mtime).Format(time.RFC1123Z)) + info2 := sRes.Info + + w.Header().Add("Content-Type", info2.MimeType) + w.Header().Set("ETag", info2.Etag) + w.Header().Set("OC-FileId", wrapResourceID(info2.Id)) + w.Header().Set("OC-ETag", info2.Etag) + t := utils.TSToTime(info2.Mtime) + lastModifiedString := t.Format(time.RFC1123Z) + w.Header().Set("Last-Modified", lastModifiedString) - // file was new as it didn't exist during the first stat - if sRes.Info == nil { + // file was new + if info == nil { w.WriteHeader(http.StatusCreated) return } diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index f54c466d6e..ae3716e0c0 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -166,7 +166,7 @@ func (h *VersionsHandler) doListVersions(w http.ResponseWriter, r *http.Request, infos = append(infos, vi) } - propRes, err := s.formatPropfind(ctx, &pf, infos, "", false) + propRes, err := s.formatPropfind(ctx, &pf, infos, "") if err != nil { log.Error().Err(err).Msg("error formatting propfind") w.WriteHeader(http.StatusInternalServerError)