diff --git a/changelog/unreleased/ocdav-putchunked.md b/changelog/unreleased/ocdav-putchunked.md new file mode 100644 index 00000000000..69e50bf424a --- /dev/null +++ b/changelog/unreleased/ocdav-putchunked.md @@ -0,0 +1,7 @@ +Bugfix: Upload file to storage provider after assembling chunks + +In the PUT handler for chunked uploads in ocdav, we store the individual +chunks in temporary file but do not write the assembled file to storage. +This PR fixes that. + +https://github.com/cs3org/reva/pull/1253 diff --git a/cmd/reva/upload.go b/cmd/reva/upload.go index ddcdbe9ee02..9dd5a2619ae 100644 --- a/cmd/reva/upload.go +++ b/cmd/reva/upload.go @@ -84,7 +84,6 @@ func uploadCommand() *command { if err != nil { return err } - defer fd.Close() fmt.Printf("Local file size: %d bytes\n", md.Size()) diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index 51fae4d9a7a..7048a44c2c7 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -19,6 +19,7 @@ package ocdav import ( + "io" "net/http" "path" "regexp" @@ -157,6 +158,23 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } } + length, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) + if err != nil { + // Fallback to Upload-Length + length, err = strconv.ParseInt(r.Header.Get("Upload-Length"), 10, 64) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + } + + s.handlePutHelper(w, r, r.Body, fn, length) +} + +func (s *svc) handlePutHelper(w http.ResponseWriter, r *http.Request, content io.Reader, fn string, length int64) { + ctx := r.Context() + log := appctx.GetLogger(ctx) + client, err := s.getClient() if err != nil { log.Error().Err(err).Msg("error getting grpc client") @@ -164,11 +182,10 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { return } - sReq := &provider.StatRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fn}, - }, + ref := &provider.Reference{ + Spec: &provider.Reference_Path{Path: fn}, } + sReq := &provider.StatRequest{Ref: ref} sRes, err := client.Stat(ctx, sReq) if err != nil { log.Error().Err(err).Msg("error sending grpc stat request") @@ -207,16 +224,6 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } } - length, err := strconv.ParseInt(r.Header.Get("Content-Length"), 10, 64) - if err != nil { - // Fallback to Upload-Length - length, err = strconv.ParseInt(r.Header.Get("Upload-Length"), 10, 64) - if err != nil { - w.WriteHeader(http.StatusBadRequest) - return - } - } - opaqueMap := map[string]*typespb.OpaqueEntry{ "Upload-Length": { Decoder: "plain", @@ -236,9 +243,7 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { } uReq := &provider.InitiateFileUploadRequest{ - Ref: &provider.Reference{ - Spec: &provider.Reference_Path{Path: fn}, - }, + Ref: ref, Opaque: &typespb.Opaque{ Map: opaqueMap, }, @@ -306,7 +311,7 @@ func (s *svc) handlePut(w http.ResponseWriter, r *http.Request, ns string) { //"checksum": fmt.Sprintf("%s %s", storageprovider.GRPC2PKGXS(xsType).String(), xs), } - upload := tus.NewUpload(r.Body, length, metadata, "") + upload := tus.NewUpload(content, length, metadata, "") // create the uploader. c.Store.Set(upload.Fingerprint, dataServerURL) diff --git a/internal/http/services/owncloud/ocdav/putchunked.go b/internal/http/services/owncloud/ocdav/putchunked.go index 80ec093edf6..b5c71e0ba8b 100644 --- a/internal/http/services/owncloud/ocdav/putchunked.go +++ b/internal/http/services/owncloud/ocdav/putchunked.go @@ -29,8 +29,6 @@ import ( "strconv" "strings" - rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" ) @@ -215,8 +213,7 @@ func (s *svc) saveChunk(ctx context.Context, path string, r io.ReadCloser) (bool return false, "", err } - tempFileName := assembledFileName - return true, tempFileName, nil + return true, assembledFileName, nil } func (s *svc) handlePutChunked(w http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -250,57 +247,17 @@ func (s *svc) handlePutChunked(w http.ResponseWriter, r *http.Request) { } defer fd.Close() - chunkInfo, _ := getChunkBLOBInfo(fn) - - client, err := s.getClient() - if err != nil { - log.Error().Err(err).Msg("error getting grpc client") - w.WriteHeader(http.StatusInternalServerError) - return - } - - ref := &provider.Reference{ - Spec: &provider.Reference_Path{Path: chunkInfo.path}, - } - req := &provider.StatRequest{Ref: ref} - res, err := client.Stat(ctx, req) + md, err := fd.Stat() if err != nil { - log.Error().Err(err).Msg("error sending grpc stat request") + log.Error().Err(err).Msg("error statting chunk") w.WriteHeader(http.StatusInternalServerError) return } - if res.Status.Code != rpc.Code_CODE_OK && res.Status.Code != rpc.Code_CODE_NOT_FOUND { - switch res.Status.Code { - case rpc.Code_CODE_PERMISSION_DENIED: - log.Debug().Str("path", fn).Interface("status", res.Status).Msg("permission denied") - w.WriteHeader(http.StatusForbidden) - default: - log.Error().Str("path", fn).Interface("status", res.Status).Msg("grpc stat request failed") - w.WriteHeader(http.StatusInternalServerError) - } - return - } - - info := res.Info - if info != nil && info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - log.Warn().Msg("resource is not a file") - w.WriteHeader(http.StatusConflict) - return - } + chunkInfo, _ := getChunkBLOBInfo(fn) + fn = chunkInfo.path - if info != nil { - clientETag := r.Header.Get("If-Match") - serverETag := info.Etag - if clientETag != "" { - if clientETag != serverETag { - log.Warn().Str("client-etag", clientETag).Str("server-etag", serverETag).Msg("etags mismatch") - w.WriteHeader(http.StatusPreconditionFailed) - return - } - } - } - w.WriteHeader(http.StatusCreated) + s.handlePutHelper(w, r, fd, fn, md.Size()) // TODO(labkode): implement old chunking diff --git a/pkg/auth/manager/json/json.go b/pkg/auth/manager/json/json.go index 377ba6ddad5..5f5b2f69be2 100644 --- a/pkg/auth/manager/json/json.go +++ b/pkg/auth/manager/json/json.go @@ -24,6 +24,7 @@ import ( "io/ioutil" user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" + typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/pkg/auth" "github.com/cs3org/reva/pkg/auth/manager/registry" "github.com/cs3org/reva/pkg/errtypes" @@ -37,13 +38,14 @@ func init() { // Credentials holds a pair of secret and userid type Credentials struct { - ID *user.UserId `mapstructure:"id" json:"id"` - Username string `mapstructure:"username" json:"username"` - Mail string `mapstructure:"mail" json:"mail"` - MailVerified bool `mapstructure:"mail_verified" json:"mail_verified"` - DisplayName string `mapstructure:"display_name" json:"display_name"` - Secret string `mapstructure:"secret" json:"secret"` - Groups []string `mapstructure:"groups" json:"groups"` + ID *user.UserId `mapstructure:"id" json:"id"` + Username string `mapstructure:"username" json:"username"` + Mail string `mapstructure:"mail" json:"mail"` + MailVerified bool `mapstructure:"mail_verified" json:"mail_verified"` + DisplayName string `mapstructure:"display_name" json:"display_name"` + Secret string `mapstructure:"secret" json:"secret"` + Groups []string `mapstructure:"groups" json:"groups"` + Opaque *typespb.Opaque `mapstructure:"opaque" json:"opaque"` } type manager struct { @@ -109,6 +111,7 @@ func (m *manager) Authenticate(ctx context.Context, username string, secret stri MailVerified: c.MailVerified, DisplayName: c.DisplayName, Groups: c.Groups, + Opaque: c.Opaque, // TODO add arbitrary keys as opaque data }, nil }