diff --git a/changelog/unreleased/app-provider-new-file.md b/changelog/unreleased/app-provider-new-file.md new file mode 100644 index 0000000000..223070bfac --- /dev/null +++ b/changelog/unreleased/app-provider-new-file.md @@ -0,0 +1,8 @@ +Change: Fix app provider new file creation and improved error codes + +We've fixed the behavior for the app provider when creating new files. +Previously the app provider would overwrite already existing files when creating a new file, this is now handled and prevented. +The new file endpoint accepted a path to a file, but this does not work for spaces. Therefore we now use the resource id of the folder where the file should be created and a filename to create the new file. +Also the app provider returns more useful error codes in a lot of cases. + +https://github.com/cs3org/reva/pull/2210 diff --git a/internal/grpc/services/gateway/appprovider.go b/internal/grpc/services/gateway/appprovider.go index 74e8a5adec..cbcad414c2 100644 --- a/internal/grpc/services/gateway/appprovider.go +++ b/internal/grpc/services/gateway/appprovider.go @@ -242,7 +242,7 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI // we did not find a default provider if res.Status.Code == rpc.Code_CODE_NOT_FOUND { - err := errtypes.NotFound(fmt.Sprintf("gateway: default app rovider for mime type:%s not found", ri.MimeType)) + err := errtypes.NotFound(fmt.Sprintf("gateway: default app provider for mime type:%s not found", ri.MimeType)) return nil, err } @@ -285,7 +285,10 @@ func (s *svc) findAppProvider(ctx context.Context, ri *storageprovider.ResourceI } res.Providers = filteredProviders - // if we only have one app provider we verify that it matches the requested app name + if len(res.Providers) == 0 { + return nil, errtypes.NotFound(fmt.Sprintf("app '%s' not found", app)) + } + if len(res.Providers) == 1 { return res.Providers[0], nil } diff --git a/internal/http/services/appprovider/appprovider.go b/internal/http/services/appprovider/appprovider.go index 9795b1302e..ed101a0f3c 100644 --- a/internal/http/services/appprovider/appprovider.go +++ b/internal/http/services/appprovider/appprovider.go @@ -19,13 +19,9 @@ package appprovider import ( - "context" - "encoding/base64" "encoding/json" - "fmt" "net/http" - "strings" - "unicode/utf8" + "path" appregistry "github.com/cs3org/go-cs3apis/cs3/app/registry/v1beta1" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" @@ -33,8 +29,6 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" "github.com/cs3org/reva/internal/http/services/datagateway" - "github.com/cs3org/reva/internal/http/services/ocmd" - "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/rgrpc/status" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/rhttp" @@ -42,15 +36,11 @@ import ( "github.com/cs3org/reva/pkg/rhttp/router" "github.com/cs3org/reva/pkg/sharedconf" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" ua "github.com/mileusna/useragent" "github.com/mitchellh/mapstructure" "github.com/pkg/errors" "github.com/rs/zerolog" - "github.com/rs/zerolog/log" -) - -const ( - idDelimiter string = ":" ) func init() { @@ -61,6 +51,7 @@ func init() { type Config struct { Prefix string `mapstructure:"prefix"` GatewaySvc string `mapstructure:"gatewaysvc"` + Insecure bool `mapstructure:"insecure"` } func (c *Config) init() { @@ -115,17 +106,17 @@ func (s *svc) Handler() http.Handler { case "open": s.handleOpen(w, r) default: - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "unsupported POST endpoint", nil) + writeError(w, r, appErrorUnimplemented, "unsupported POST endpoint", nil) } case "GET": switch head { case "list": s.handleList(w, r) default: - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "unsupported GET endpoint", nil) + writeError(w, r, appErrorUnimplemented, "unsupported GET endpoint", nil) } default: - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "unsupported method", nil) + writeError(w, r, appErrorUnimplemented, "unsupported method", nil) } }) } @@ -135,34 +126,86 @@ func (s *svc) handleNew(w http.ResponseWriter, r *http.Request) { client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error getting grpc gateway client", err) + writeError(w, r, appErrorServerError, "error getting grpc gateway client", err) return } if r.URL.Query().Get("template") != "" { // TODO in the future we want to create a file out of the given template - ocmd.WriteError(w, r, ocmd.APIErrorInvalidParameter, "Template not implemented", - errtypes.NotSupported("Templates are not yet supported")) + writeError(w, r, appErrorUnimplemented, "template is not implemented", nil) + return + } + + parentContainerID := r.URL.Query().Get("parent_container_id") + if parentContainerID == "" { + writeError(w, r, appErrorInvalidParameter, "missing parent container ID", nil) + return + } + + parentContainerRef := resourceid.OwnCloudResourceIDUnwrap(parentContainerID) + if parentContainerRef == nil { + writeError(w, r, appErrorInvalidParameter, "invalid parent container ID", nil) + return + } + + filename := r.URL.Query().Get("filename") + if filename == "" { + writeError(w, r, appErrorInvalidParameter, "missing filename", nil) + return + } + + dirPart, filePart := path.Split(filename) + if dirPart != "" || filePart != filename { + writeError(w, r, appErrorInvalidParameter, "the filename must not contain a path segment", nil) + return + } + + statParentContainerReq := &provider.StatRequest{ + Ref: &provider.Reference{ + ResourceId: parentContainerRef, + }, + } + parentContainer, err := client.Stat(ctx, statParentContainerReq) + if err != nil { + writeError(w, r, appErrorServerError, "error sending a grpc stat request", err) + return + } + + if parentContainer.Status.Code != rpc.Code_CODE_OK { + writeError(w, r, appErrorNotFound, "the parent container is not accessible or does not exist", err) return } - target := r.URL.Query().Get("filename") - if target == "" { - ocmd.WriteError(w, r, ocmd.APIErrorInvalidParameter, "Missing filename", - errtypes.UserRequired("Missing filename")) + if parentContainer.Info.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER { + writeError(w, r, appErrorInvalidParameter, "the parent container id does not point to a container", nil) return } - // TODO(lopresti) if target is relative, currently the gateway fails to identify a storage provider (?) - // and just returns a CODE_INTERNAL error on InitiateFileUpload. - // Therefore for now make sure the target is absolute. - if target[0] != '/' { - target = "/" + target + fileRef := &provider.Reference{ + Path: path.Join(parentContainer.Info.Path, utils.MakeRelativePath(filename)), + } + + statFileReq := &provider.StatRequest{ + Ref: fileRef, + } + statFileRes, err := client.Stat(ctx, statFileReq) + if err != nil { + writeError(w, r, appErrorServerError, "failed to stat the file", err) + return + } + + if statFileRes.Status.Code != rpc.Code_CODE_NOT_FOUND { + if statFileRes.Status.Code == rpc.Code_CODE_OK { + writeError(w, r, appErrorAlreadyExists, "the file already exists", nil) + return + } + writeError(w, r, appErrorServerError, "statting the file returned unexpected status code", err) + return } // Create empty file via storageprovider createReq := &provider.InitiateFileUploadRequest{ - Ref: &provider.Reference{Path: target}, + Ref: fileRef, Opaque: &typespb.Opaque{ Map: map[string]*typespb.OpaqueEntry{ "Upload-Length": { @@ -172,13 +215,16 @@ func (s *svc) handleNew(w http.ResponseWriter, r *http.Request) { }, }, } + + // having a client.CreateFile() function would come in handy here... + createRes, err := client.InitiateFileUpload(ctx, createReq) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error calling InitiateFileUpload", err) + writeError(w, r, appErrorServerError, "error calling InitiateFileUpload", err) return } if createRes.Status.Code != rpc.Code_CODE_OK { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error creating resource", status.NewErrorFromCode(createRes.Status.Code, "appprovider")) + writeError(w, r, appErrorServerError, "error calling InitiateFileUpload", nil) return } @@ -191,44 +237,58 @@ func (s *svc) handleNew(w http.ResponseWriter, r *http.Request) { } httpReq, err := rhttp.NewRequest(ctx, http.MethodPut, ep, nil) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error executing PUT", err) + writeError(w, r, appErrorServerError, "failed to create the file", err) return } httpReq.Header.Set(datagateway.TokenTransportHeader, token) - httpRes, err := rhttp.GetHTTPClient().Do(httpReq) + httpRes, err := rhttp.GetHTTPClient( + rhttp.Context(ctx), + rhttp.Insecure(s.conf.Insecure), + ).Do(httpReq) if err != nil { - log.Error().Err(err).Msg("error doing PUT request to data service") - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error executing PUT", err) + writeError(w, r, appErrorServerError, "failed to create the file", err) return } defer httpRes.Body.Close() - if httpRes.StatusCode != http.StatusOK { - log.Error().Msg("PUT request to data server failed") - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error executing PUT", - errtypes.InternalError(fmt.Sprint(httpRes.StatusCode))) + if httpRes.StatusCode == http.StatusForbidden { + // the file upload was already finished since it is a zero byte file + // TODO: why do we get a 401 then!? + } else if httpRes.StatusCode != http.StatusOK { + writeError(w, r, appErrorServerError, "failed to create the file", nil) return } // Stat the newly created file - statRes, ocmderr, err := statRef(ctx, provider.Reference{Path: target}, client) + statRes, err := client.Stat(ctx, statFileReq) if err != nil { - log.Error().Err(err).Msg("error statting created file") - ocmd.WriteError(w, r, ocmderr, "Created file not found", errtypes.NotFound("Created file not found")) + writeError(w, r, appErrorServerError, "statting the created file failed", err) + return + } + + if statRes.Status.Code != rpc.Code_CODE_OK { + writeError(w, r, appErrorServerError, "statting the created file failed", nil) + return + } + + if statRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { + writeError(w, r, appErrorInvalidParameter, "the given file id does not point to a file", nil) return } - // Base64-encode the fileid for the web to consume it - b64id := base64.StdEncoding.EncodeToString([]byte(statRes.Id.StorageId + idDelimiter + statRes.Id.OpaqueId)) - js, err := json.Marshal(map[string]interface{}{"file_id": b64id}) + js, err := json.Marshal( + map[string]interface{}{ + "file_id": resourceid.OwnCloudResourceIDWrap(statRes.Info.Id), + }, + ) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error marshalling JSON response", err) + writeError(w, r, appErrorServerError, "error marshalling JSON response", err) return } w.Header().Set("Content-Type", "application/json") if _, err = w.Write(js); err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error writing JSON response", err) + writeError(w, r, appErrorServerError, "error writing JSON response", err) return } } @@ -237,31 +297,30 @@ func (s *svc) handleList(w http.ResponseWriter, r *http.Request) { ctx := r.Context() client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error getting grpc gateway client", err) + writeError(w, r, appErrorServerError, "error getting grpc gateway client", err) return } listRes, err := client.ListSupportedMimeTypes(ctx, &appregistry.ListSupportedMimeTypesRequest{}) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error listing supported mime types", err) + writeError(w, r, appErrorServerError, "error listing supported mime types", err) return } if listRes.Status.Code != rpc.Code_CODE_OK { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error listing supported mime types", - status.NewErrorFromCode(listRes.Status.Code, "appprovider")) + writeError(w, r, appErrorServerError, "error listing supported mime types", nil) return } res := filterAppsByUserAgent(listRes.MimeTypes, r.UserAgent()) js, err := json.Marshal(map[string]interface{}{"mime-types": res}) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error marshalling JSON response", err) + writeError(w, r, appErrorServerError, "error marshalling JSON response", err) return } w.Header().Set("Content-Type", "application/json") if _, err = w.Write(js); err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "error writing JSON response", err) + writeError(w, r, appErrorServerError, "error writing JSON response", err) return } } @@ -271,43 +330,83 @@ func (s *svc) handleOpen(w http.ResponseWriter, r *http.Request) { client, err := pool.GetGatewayServiceClient(s.conf.GatewaySvc) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with the gateway, please try again later", err) + writeError(w, r, appErrorServerError, "Internal error with the gateway, please try again later", err) + return + } + + fileID := r.URL.Query().Get("file_id") + + if fileID == "" { + writeError(w, r, appErrorInvalidParameter, "missing file ID", nil) return } - info, errCode, err := s.getStatInfo(ctx, r.URL.Query().Get("file_id"), client) + resourceID := resourceid.OwnCloudResourceIDUnwrap(fileID) + if resourceID == nil { + writeError(w, r, appErrorInvalidParameter, "invalid file ID", nil) + return + } + + fileRef := &provider.Reference{ + ResourceId: resourceID, + } + + statRes, err := client.Stat(ctx, &provider.StatRequest{Ref: fileRef}) if err != nil { - ocmd.WriteError(w, r, errCode, "Internal error accessing the file, please try again later", err) + writeError(w, r, appErrorServerError, "Internal error accessing the file, please try again later", err) + return + } + + if statRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + writeError(w, r, appErrorNotFound, "file does not exist", nil) + return + } else if statRes.Status.Code != rpc.Code_CODE_OK { + writeError(w, r, appErrorServerError, "failed to stat the file", nil) + return + } + + if statRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { + writeError(w, r, appErrorInvalidParameter, "the given file id does not point to a file", nil) + return + } + + viewMode := getViewMode(statRes.Info, r.URL.Query().Get("view_mode")) + if viewMode == gateway.OpenInAppRequest_VIEW_MODE_INVALID { + writeError(w, r, appErrorInvalidParameter, "invalid view mode", err) return } openReq := gateway.OpenInAppRequest{ - Ref: &provider.Reference{ResourceId: info.Id}, - ViewMode: getViewMode(info, r.URL.Query().Get("view_mode")), + Ref: fileRef, + ViewMode: viewMode, App: r.URL.Query().Get("app_name"), } openRes, err := client.OpenInApp(ctx, &openReq) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, + writeError(w, r, appErrorServerError, "Error contacting the requested application, please use a different one or try again later", err) return } if openRes.Status.Code != rpc.Code_CODE_OK { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, openRes.Status.Message, - status.NewErrorFromCode(openRes.Status.Code, "Error calling OpenInApp")) + if openRes.Status.Code == rpc.Code_CODE_NOT_FOUND { + writeError(w, r, appErrorNotFound, openRes.Status.Message, nil) + return + } + writeError(w, r, appErrorServerError, openRes.Status.Message, + status.NewErrorFromCode(openRes.Status.Code, "error calling OpenInApp")) return } js, err := json.Marshal(openRes.AppUrl) if err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with JSON payload", + writeError(w, r, appErrorServerError, "Internal error with JSON payload", errors.Wrap(err, "error marshalling JSON response")) return } w.Header().Set("Content-Type", "application/json") if _, err = w.Write(js); err != nil { - ocmd.WriteError(w, r, ocmd.APIErrorServerError, "Internal error with JSON payload", + writeError(w, r, appErrorServerError, "Internal error with JSON payload", errors.Wrap(err, "error writing JSON response")) return } @@ -334,43 +433,6 @@ func filterAppsByUserAgent(mimeTypes []*appregistry.MimeTypeInfo, userAgent stri return res } -func (s *svc) getStatInfo(ctx context.Context, fileID string, client gateway.GatewayAPIClient) (*provider.ResourceInfo, ocmd.APIErrorCode, error) { - if fileID == "" { - return nil, ocmd.APIErrorInvalidParameter, errors.New("fileID parameter missing in request") - } - - decodedID, err := base64.URLEncoding.DecodeString(fileID) - if err != nil { - return nil, ocmd.APIErrorInvalidParameter, errors.Wrap(err, fmt.Sprintf("fileID %s doesn't follow the required format", fileID)) - } - - parts := strings.Split(string(decodedID), idDelimiter) - if !utf8.ValidString(parts[0]) || !utf8.ValidString(parts[1]) { - return nil, ocmd.APIErrorInvalidParameter, errtypes.BadRequest(fmt.Sprintf("fileID %s contains illegal characters", fileID)) - } - res := &provider.ResourceId{ - StorageId: parts[0], - OpaqueId: parts[1], - } - - return statRef(ctx, provider.Reference{ResourceId: res}, client) -} - -func statRef(ctx context.Context, ref provider.Reference, client gateway.GatewayAPIClient) (*provider.ResourceInfo, ocmd.APIErrorCode, error) { - statReq := provider.StatRequest{Ref: &ref} - statRes, err := client.Stat(ctx, &statReq) - if err != nil { - return nil, ocmd.APIErrorServerError, err - } - if statRes.Status.Code != rpc.Code_CODE_OK { - return nil, ocmd.APIErrorServerError, status.NewErrorFromCode(statRes.Status.Code, "appprovider") - } - if statRes.Info.Type != provider.ResourceType_RESOURCE_TYPE_FILE { - return nil, ocmd.APIErrorServerError, errors.New("unsupported resource type") - } - return statRes.Info, ocmd.APIErrorCode(""), nil -} - func getViewMode(res *provider.ResourceInfo, vm string) gateway.OpenInAppRequest_ViewMode { if vm != "" { return utils.GetViewMode(vm) diff --git a/internal/http/services/appprovider/errors.go b/internal/http/services/appprovider/errors.go new file mode 100644 index 0000000000..353925423d --- /dev/null +++ b/internal/http/services/appprovider/errors.go @@ -0,0 +1,78 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package appprovider + +import ( + "encoding/json" + "net/http" + + "github.com/cs3org/reva/pkg/appctx" +) + +// appErrorCode stores the type of error encountered +type appErrorCode string + +const ( + appErrorNotFound appErrorCode = "RESOURCE_NOT_FOUND" + appErrorAlreadyExists appErrorCode = "RESOURCE_ALREADY_EXISTS" + appErrorUnauthenticated appErrorCode = "UNAUTHENTICATED" + appErrorUnimplemented appErrorCode = "NOT_IMPLEMENTED" + appErrorInvalidParameter appErrorCode = "INVALID_PARAMETER" + appErrorServerError appErrorCode = "SERVER_ERROR" +) + +// appErrorCodeMapping stores the HTTP error code mapping for various APIErrorCodes +var appErrorCodeMapping = map[appErrorCode]int{ + appErrorNotFound: http.StatusNotFound, + appErrorAlreadyExists: http.StatusForbidden, + appErrorUnauthenticated: http.StatusUnauthorized, + appErrorUnimplemented: http.StatusNotImplemented, + appErrorInvalidParameter: http.StatusBadRequest, + appErrorServerError: http.StatusInternalServerError, +} + +// APIError encompasses the error type and message +type appError struct { + Code appErrorCode `json:"code"` + Message string `json:"message"` +} + +// writeError handles writing error responses +func writeError(w http.ResponseWriter, r *http.Request, code appErrorCode, message string, err error) { + if err != nil { + appctx.GetLogger(r.Context()).Error().Err(err).Msg(message) + } + + var encoded []byte + w.Header().Set("Content-Type", "application/json") + encoded, err = json.MarshalIndent(appError{Code: code, Message: message}, "", " ") + + if err != nil { + appctx.GetLogger(r.Context()).Error().Err(err).Msg("error encoding response") + w.WriteHeader(http.StatusInternalServerError) + return + } + + w.WriteHeader(appErrorCodeMapping[code]) + _, err = w.Write(encoded) + if err != nil { + appctx.GetLogger(r.Context()).Error().Err(err).Msg("error writing response") + w.WriteHeader(http.StatusInternalServerError) + } +} diff --git a/internal/http/services/archiver/handler.go b/internal/http/services/archiver/handler.go index ffa8df0356..ee2c05f111 100644 --- a/internal/http/services/archiver/handler.go +++ b/internal/http/services/archiver/handler.go @@ -20,12 +20,10 @@ package archiver import ( "context" - "encoding/base64" + "errors" "fmt" "net/http" - "strings" "time" - "unicode/utf8" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -41,6 +39,7 @@ import ( "github.com/cs3org/reva/pkg/sharedconf" "github.com/cs3org/reva/pkg/storage/utils/downloader" "github.com/cs3org/reva/pkg/storage/utils/walker" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/gdexlab/go-render/render" ua "github.com/mileusna/useragent" "github.com/mitchellh/mapstructure" @@ -130,17 +129,14 @@ func (s *svc) getFiles(ctx context.Context, files, ids []string) ([]string, erro for _, id := range ids { // id is base64 encoded and after decoding has the form : - storageID, opaqueID, err := decodeResourceID(id) - if err != nil { - return nil, err + ref := resourceid.OwnCloudResourceIDUnwrap(id) + if ref == nil { + return nil, errors.New("could not unwrap given file id") } resp, err := s.gtwClient.Stat(ctx, &provider.StatRequest{ Ref: &provider.Reference{ - ResourceId: &provider.ResourceId{ - StorageId: storageID, - OpaqueId: opaqueID, - }, + ResourceId: ref, }, }) @@ -279,19 +275,3 @@ func (s *svc) Close() error { func (s *svc) Unprotected() []string { return nil } - -func decodeResourceID(encodedID string) (string, string, error) { - decodedID, err := base64.URLEncoding.DecodeString(encodedID) - if err != nil { - return "", "", errtypes.BadRequest("resource ID does not follow the required format") - } - - parts := strings.Split(string(decodedID), ":") - if len(parts) != 2 { - return "", "", errtypes.BadRequest("resource ID does not follow the required format") - } - if !utf8.ValidString(parts[0]) || !utf8.ValidString(parts[1]) { - return "", "", errtypes.BadRequest("resourceID contains illegal characters") - } - return parts[0], parts[1], nil -} diff --git a/internal/http/services/owncloud/ocdav/get.go b/internal/http/services/owncloud/ocdav/get.go index cefc4da7df..a812365846 100644 --- a/internal/http/services/owncloud/ocdav/get.go +++ b/internal/http/services/owncloud/ocdav/get.go @@ -36,6 +36,7 @@ import ( "github.com/cs3org/reva/pkg/rhttp" rtrace "github.com/cs3org/reva/pkg/trace" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/rs/zerolog" ) @@ -126,7 +127,7 @@ func (s *svc) handleGet(ctx context.Context, w http.ResponseWriter, r *http.Requ w.Header().Set(HeaderContentDisposistion, "attachment; filename*=UTF-8''"+ path.Base(r.RequestURI)+"; filename=\""+path.Base(r.RequestURI)+"\"") w.Header().Set(HeaderETag, info.Etag) - w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) t := utils.TSToTime(info.Mtime).UTC() lastModifiedString := t.Format(time.RFC1123Z) diff --git a/internal/http/services/owncloud/ocdav/head.go b/internal/http/services/owncloud/ocdav/head.go index 7b3094595b..7acdca6a8e 100644 --- a/internal/http/services/owncloud/ocdav/head.go +++ b/internal/http/services/owncloud/ocdav/head.go @@ -28,6 +28,7 @@ import ( "time" rtrace "github.com/cs3org/reva/pkg/trace" + "github.com/cs3org/reva/pkg/utils/resourceid" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -73,7 +74,7 @@ func (s *svc) handleHead(ctx context.Context, w http.ResponseWriter, r *http.Req info := res.Info w.Header().Set(HeaderContentType, info.MimeType) w.Header().Set(HeaderETag, info.Etag) - w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) if info.Checksum != nil { w.Header().Set(HeaderOCChecksum, fmt.Sprintf("%s:%s", strings.ToUpper(string(storageprovider.GRPC2PKGXS(info.Checksum.Type))), info.Checksum.Sum)) diff --git a/internal/http/services/owncloud/ocdav/meta.go b/internal/http/services/owncloud/ocdav/meta.go index f40baa2396..7aaffea142 100644 --- a/internal/http/services/owncloud/ocdav/meta.go +++ b/internal/http/services/owncloud/ocdav/meta.go @@ -22,6 +22,7 @@ import ( "net/http" "github.com/cs3org/reva/pkg/rhttp/router" + "github.com/cs3org/reva/pkg/utils/resourceid" ) // MetaHandler handles meta requests @@ -45,7 +46,7 @@ func (h *MetaHandler) Handler(s *svc) http.Handler { return } - did := unwrap(id) + did := resourceid.OwnCloudResourceIDUnwrap(id) var head string head, r.URL.Path = router.ShiftPath(r.URL.Path) diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 376f57d72b..53d3852f8c 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/internal/http/services/owncloud/ocdav/move.go @@ -30,6 +30,7 @@ import ( "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rhttp/router" rtrace "github.com/cs3org/reva/pkg/trace" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/rs/zerolog" ) @@ -259,7 +260,7 @@ func (s *svc) handleMove(ctx context.Context, w http.ResponseWriter, r *http.Req info := dstStatRes.Info w.Header().Set(HeaderContentType, info.MimeType) w.Header().Set(HeaderETag, info.Etag) - w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) w.WriteHeader(successCode) } diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 27d11407b2..e53daa7ccf 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -20,7 +20,6 @@ package ocdav import ( "context" - "encoding/base64" "fmt" "net/http" "net/url" @@ -28,11 +27,9 @@ import ( "regexp" "strings" "time" - "unicode/utf8" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" ctxpkg "github.com/cs3org/reva/pkg/ctx" "github.com/cs3org/reva/pkg/errtypes" @@ -53,8 +50,6 @@ type ctxKey int const ( ctxKeyBaseURI ctxKey = iota - - idDelimiter string = ":" ) var ( @@ -271,39 +266,6 @@ func applyLayout(ctx context.Context, ns string, useLoggedInUserNS bool, request return templates.WithUser(u, ns) } -func wrapResourceID(r *provider.ResourceId) string { - return wrap(r.StorageId, r.OpaqueId) -} - -// The fileID must be encoded -// - XML safe, because it is going to be used in the propfind result -// - url safe, because the id might be used in a url, eg. the /dav/meta nodes -// which is why we base64 encode it -func wrap(sid string, oid string) string { - return base64.URLEncoding.EncodeToString([]byte(sid + idDelimiter + oid)) -} - -func unwrap(rid string) *provider.ResourceId { - decodedID, err := base64.URLEncoding.DecodeString(rid) - if err != nil { - return nil - } - - parts := strings.SplitN(string(decodedID), idDelimiter, 2) - if len(parts) != 2 { - return nil - } - - if !utf8.ValidString(parts[0]) || !utf8.ValidString(parts[1]) { - return nil - } - - return &provider.ResourceId{ - StorageId: parts[0], - OpaqueId: parts[1], - } -} - func addAccessHeaders(w http.ResponseWriter, r *http.Request) { headers := w.Header() // the webdav api is accessible from anywhere diff --git a/internal/http/services/owncloud/ocdav/ocdav_test.go b/internal/http/services/owncloud/ocdav/ocdav_test.go index 24326cb9f9..7d0de23167 100644 --- a/internal/http/services/owncloud/ocdav/ocdav_test.go +++ b/internal/http/services/owncloud/ocdav/ocdav_test.go @@ -25,7 +25,7 @@ import ( "testing" providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" ) /* @@ -41,69 +41,15 @@ func BenchmarkEncodePath(b *testing.B) { } } -func BenchmarkWrap(b *testing.B) { - for i := 0; i < b.N; i++ { - _ = wrap("storageid", "opaqueid") - } -} - -func TestWrap(t *testing.T) { - expected := "c3RvcmFnZWlkOm9wYXF1ZWlk" - wrapped := wrap("storageid", "opaqueid") - - if wrapped != expected { - t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected) - } -} - func TestWrapResourceID(t *testing.T) { expected := "c3RvcmFnZWlkOm9wYXF1ZWlk" - wrapped := wrapResourceID(&providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}) + wrapped := resourceid.OwnCloudResourceIDWrap(&providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}) if wrapped != expected { t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected) } } -func BenchmarkUnwrap(b *testing.B) { - for i := 0; i < b.N; i++ { - _ = unwrap("c3RvcmFnZWlkOm9wYXF1ZWlk") - } -} - -func TestUnwrap(t *testing.T) { - tests := []struct { - input string - expected *providerv1beta1.ResourceId - }{ - { - "c3RvcmFnZWlkOm9wYXF1ZWlk", - &providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, - }, - { - "", - nil, - }, - { - "c", - nil, - }, - } - - for _, tt := range tests { - rid := unwrap(tt.input) - - if tt.expected == nil { - if rid != nil { - t.Errorf("Expected unwrap to return nil, got %v", rid) - } - } else if !utils.ResourceIDEqual(rid, tt.expected) { - t.Error("StorageID or OpaqueID doesn't match") - } - } - -} - func TestExtractDestination(t *testing.T) { expected := "/dst" request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil) diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index eb0e0b197f..a9adf2848d 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -44,6 +44,7 @@ import ( "github.com/cs3org/reva/pkg/publicshare" rtrace "github.com/cs3org/reva/pkg/trace" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/rs/zerolog" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -550,7 +551,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide // return all known properties if md.Id != nil { - id := wrapResourceID(md.Id) + id := resourceid.OwnCloudResourceIDWrap(md.Id) propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:id", id), s.newProp("oc:fileid", id), @@ -649,13 +650,13 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide // I tested the desktop client and phoenix to annotate which properties are requestted, see below cases case "fileid": // phoenix only if md.Id != nil { - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:fileid", wrapResourceID(md.Id))) + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:fileid", resourceid.OwnCloudResourceIDWrap(md.Id))) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:fileid", "")) } case "id": // desktop client only if md.Id != nil { - propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:id", wrapResourceID(md.Id))) + propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:id", resourceid.OwnCloudResourceIDWrap(md.Id))) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:id", "")) } diff --git a/internal/http/services/owncloud/ocdav/put.go b/internal/http/services/owncloud/ocdav/put.go index f28a423404..1114d604f6 100644 --- a/internal/http/services/owncloud/ocdav/put.go +++ b/internal/http/services/owncloud/ocdav/put.go @@ -36,6 +36,7 @@ import ( "github.com/cs3org/reva/pkg/storage/utils/chunking" rtrace "github.com/cs3org/reva/pkg/trace" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/rs/zerolog" ) @@ -314,7 +315,7 @@ func (s *svc) handlePut(ctx context.Context, w http.ResponseWriter, r *http.Requ w.Header().Add(HeaderContentType, newInfo.MimeType) w.Header().Set(HeaderETag, newInfo.Etag) - w.Header().Set(HeaderOCFileID, wrapResourceID(newInfo.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(newInfo.Id)) w.Header().Set(HeaderOCETag, newInfo.Etag) t := utils.TSToTime(newInfo.Mtime).UTC() lastModifiedString := t.Format(time.RFC1123Z) diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index 5d7425d1bd..7d27aa0964 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/internal/http/services/owncloud/ocdav/trashbin.go @@ -30,6 +30,7 @@ import ( "time" rtrace "github.com/cs3org/reva/pkg/trace" + "github.com/cs3org/reva/pkg/utils/resourceid" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -580,7 +581,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc info := dstStatRes.Info w.Header().Set(HeaderContentType, info.MimeType) w.Header().Set(HeaderETag, info.Etag) - w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) w.WriteHeader(successCode) diff --git a/internal/http/services/owncloud/ocdav/tus.go b/internal/http/services/owncloud/ocdav/tus.go index 8dea0a0fce..b795d482d4 100644 --- a/internal/http/services/owncloud/ocdav/tus.go +++ b/internal/http/services/owncloud/ocdav/tus.go @@ -36,6 +36,7 @@ import ( "github.com/cs3org/reva/pkg/rhttp" rtrace "github.com/cs3org/reva/pkg/trace" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/rs/zerolog" tusd "github.com/tus/tusd/pkg/handler" ) @@ -313,7 +314,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http. ) w.Header().Set(HeaderContentType, info.MimeType) - w.Header().Set(HeaderOCFileID, wrapResourceID(info.Id)) + w.Header().Set(HeaderOCFileID, resourceid.OwnCloudResourceIDWrap(info.Id)) w.Header().Set(HeaderOCETag, info.Etag) w.Header().Set(HeaderETag, info.Etag) w.Header().Set(HeaderOCPermissions, permissions) diff --git a/internal/http/services/owncloud/ocdav/versions.go b/internal/http/services/owncloud/ocdav/versions.go index ad6620ccd9..75467dd39e 100644 --- a/internal/http/services/owncloud/ocdav/versions.go +++ b/internal/http/services/owncloud/ocdav/versions.go @@ -24,6 +24,7 @@ import ( "path" rtrace "github.com/cs3org/reva/pkg/trace" + "github.com/cs3org/reva/pkg/utils/resourceid" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -53,7 +54,7 @@ func (h *VersionsHandler) Handler(s *svc, rid *provider.ResourceId) http.Handler } // baseURI is encoded as part of the response payload in href field - baseURI := path.Join(ctx.Value(ctxKeyBaseURI).(string), wrapResourceID(rid)) + baseURI := path.Join(ctx.Value(ctxKeyBaseURI).(string), resourceid.OwnCloudResourceIDWrap(rid)) ctx = context.WithValue(ctx, ctxKeyBaseURI, baseURI) r = r.WithContext(ctx) diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index f1937658ca..53c859c7c0 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -21,7 +21,6 @@ package shares import ( "bytes" "context" - "encoding/base64" "encoding/json" "fmt" "mime" @@ -55,6 +54,7 @@ import ( "github.com/cs3org/reva/pkg/share/cache" "github.com/cs3org/reva/pkg/share/cache/registry" "github.com/cs3org/reva/pkg/utils" + "github.com/cs3org/reva/pkg/utils/resourceid" "github.com/pkg/errors" ) @@ -119,7 +119,7 @@ func (h *Handler) startCacheWarmup(c cache.Warmup) { return } for _, r := range infos { - key := wrapResourceID(r.Id) + key := resourceid.OwnCloudResourceIDWrap(r.Id) _ = h.resourceInfoCache.SetWithExpire(key, r, h.resourceInfoCacheTTL) } } @@ -856,18 +856,6 @@ func (h *Handler) addFilters(w http.ResponseWriter, r *http.Request, prefix stri return collaborationFilters, linkFilters, nil } -func wrapResourceID(r *provider.ResourceId) string { - return wrap(r.StorageId, r.OpaqueId) -} - -// The fileID must be encoded -// - XML safe, because it is going to be used in the propfind result -// - url safe, because the id might be used in a url, eg. the /dav/meta nodes -// which is why we base64 encode it -func wrap(sid string, oid string) string { - return base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", sid, oid))) -} - func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, info *provider.ResourceInfo) error { log := appctx.GetLogger(ctx) if info != nil { @@ -880,7 +868,7 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf s.MimeType = parsedMt // TODO STime: &types.Timestamp{Seconds: info.Mtime.Seconds, Nanos: info.Mtime.Nanos}, // TODO Storage: int - s.ItemSource = wrapResourceID(info.Id) + s.ItemSource = resourceid.OwnCloudResourceIDWrap(info.Id) s.FileSource = s.ItemSource switch { case h.sharePrefix == "/": @@ -1041,7 +1029,7 @@ func (h *Handler) getResourceInfoByPath(ctx context.Context, client gateway.Gate } func (h *Handler) getResourceInfoByID(ctx context.Context, client gateway.GatewayAPIClient, id *provider.ResourceId) (*provider.ResourceInfo, *rpc.Status, error) { - return h.getResourceInfo(ctx, client, wrapResourceID(id), &provider.Reference{ResourceId: id}) + return h.getResourceInfo(ctx, client, resourceid.OwnCloudResourceIDWrap(id), &provider.Reference{ResourceId: id}) } // getResourceInfo retrieves the resource info to a target. diff --git a/pkg/utils/resourceid/owncloud.go b/pkg/utils/resourceid/owncloud.go new file mode 100644 index 0000000000..39abc460a4 --- /dev/null +++ b/pkg/utils/resourceid/owncloud.go @@ -0,0 +1,77 @@ +// Copyright 2018-2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package resourceid + +import ( + "encoding/base64" + "errors" + "strings" + "unicode/utf8" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" +) + +const ( + idDelimiter string = ":" +) + +// OwnCloudResourceIDUnwrap returns the wrapped resource id +// by OwnCloudResourceIDWrap and returns nil if not possible +func OwnCloudResourceIDUnwrap(rid string) *provider.ResourceId { + id, err := unwrap(rid) + if err != nil { + return nil + } + return id +} + +func unwrap(rid string) (*provider.ResourceId, error) { + decodedID, err := base64.URLEncoding.DecodeString(rid) + if err != nil { + return nil, err + } + + parts := strings.SplitN(string(decodedID), idDelimiter, 2) + if len(parts) != 2 { + return nil, errors.New("could not find two parts with given delimiter") + } + + if !utf8.ValidString(parts[0]) || !utf8.ValidString(parts[1]) { + return nil, errors.New("invalid utf8 string found") + } + + return &provider.ResourceId{ + StorageId: parts[0], + OpaqueId: parts[1], + }, nil +} + +// OwnCloudResourceIDWrap wraps a resource id into a xml safe string +// which can then be passed to the outside world +func OwnCloudResourceIDWrap(r *provider.ResourceId) string { + return wrap(r.StorageId, r.OpaqueId) +} + +// The fileID must be encoded +// - XML safe, because it is going to be used in the propfind result +// - url safe, because the id might be used in a url, eg. the /dav/meta nodes +// which is why we base64 encode it +func wrap(sid string, oid string) string { + return base64.URLEncoding.EncodeToString([]byte(sid + idDelimiter + oid)) +} diff --git a/pkg/utils/resourceid/owncloud_test.go b/pkg/utils/resourceid/owncloud_test.go new file mode 100644 index 0000000000..5fc45af94a --- /dev/null +++ b/pkg/utils/resourceid/owncloud_test.go @@ -0,0 +1,88 @@ +// Copyright 2021 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. +package resourceid + +import ( + "testing" + + providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/pkg/utils" +) + +func BenchmarkWrap(b *testing.B) { + for i := 0; i < b.N; i++ { + _ = wrap("storageid", "opaqueid") + } +} + +func TestWrap(t *testing.T) { + expected := "c3RvcmFnZWlkOm9wYXF1ZWlk" + wrapped := wrap("storageid", "opaqueid") + + if wrapped != expected { + t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected) + } +} + +func TestWrapResourceID(t *testing.T) { + expected := "c3RvcmFnZWlkOm9wYXF1ZWlk" + wrapped := OwnCloudResourceIDWrap(&providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}) + + if wrapped != expected { + t.Errorf("wrapped id doesn't have the expected format: got %s expected %s", wrapped, expected) + } +} + +func BenchmarkUnwrap(b *testing.B) { + for i := 0; i < b.N; i++ { + _, _ = unwrap("c3RvcmFnZWlkOm9wYXF1ZWlk") + } +} + +func TestUnwrap(t *testing.T) { + tests := []struct { + input string + expected *providerv1beta1.ResourceId + }{ + { + "c3RvcmFnZWlkOm9wYXF1ZWlk", + &providerv1beta1.ResourceId{StorageId: "storageid", OpaqueId: "opaqueid"}, + }, + { + "", + nil, + }, + { + "c", + nil, + }, + } + + for _, tt := range tests { + rid := OwnCloudResourceIDUnwrap(tt.input) + + if tt.expected == nil { + if rid != nil { + t.Errorf("Expected unwrap to return nil, got %v", rid) + } + } else if !utils.ResourceIDEqual(rid, tt.expected) { + t.Error("StorageID or OpaqueID doesn't match") + } + } + +}