From 17606d3fbadf41bd3656a82472e19ddc51e5221c Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Fri, 12 Nov 2021 10:31:20 +0100 Subject: [PATCH] Fix unset xattr on darwin (#2260) * Fix unset xattr on darwin * Improve Webdav error handling on propfind * Change method name --- changelog/unreleased/fix-unset-quota.md | 5 +++ .../http/services/owncloud/ocdav/propfind.go | 6 +++ pkg/storage/utils/decomposedfs/node/node.go | 16 ++++---- .../utils/decomposedfs/node/permissions.go | 10 +---- .../decomposedfs/node/permissions_darwin.go | 37 +++++++++++++++++++ .../decomposedfs/node/permissions_unix.go | 37 +++++++++++++++++++ 6 files changed, 94 insertions(+), 17 deletions(-) create mode 100644 changelog/unreleased/fix-unset-quota.md create mode 100644 pkg/storage/utils/decomposedfs/node/permissions_darwin.go create mode 100644 pkg/storage/utils/decomposedfs/node/permissions_unix.go diff --git a/changelog/unreleased/fix-unset-quota.md b/changelog/unreleased/fix-unset-quota.md new file mode 100644 index 0000000000..bd632ff0a9 --- /dev/null +++ b/changelog/unreleased/fix-unset-quota.md @@ -0,0 +1,5 @@ +Bugfix: Fix unset quota xattr on darwin + +Unset quota attributes were creating errors in the logfile on darwin. + +https://github.com/cs3org/reva/pull/2260 diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 7458f378bf..6febf5db63 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -200,6 +200,12 @@ func (s *svc) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *ht if depth != "0" && depth != "1" && depth != "infinity" { log.Debug().Str("depth", depth).Msgf("invalid Depth header value") w.WriteHeader(http.StatusBadRequest) + m := fmt.Sprintf("Invalid Depth header value: %v", depth) + b, err := Marshal(exception{ + code: SabredavBadRequest, + message: m, + }) + HandleWebdavError(&log, w, b, err) return nil, nil, false } diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 896ed0701b..d8f867497f 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -185,7 +185,7 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error switch { case err == nil: n.ParentID = string(attrBytes) - case isNoData(err): + case isAttrUnset(err): return nil, errtypes.InternalError(err.Error()) case isNotFound(err): return n, nil // swallow not found, the node defaults to exists = false @@ -326,7 +326,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.OpaqueId = string(attrBytes) - case isNoData(err), isNotFound(err): + case isAttrUnset(err), isNotFound(err): fallthrough default: return nil, err @@ -337,7 +337,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.Idp = string(attrBytes) - case isNoData(err), isNotFound(err): + case isAttrUnset(err), isNotFound(err): fallthrough default: return nil, err @@ -348,7 +348,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.Type = utils.UserTypeMap(string(attrBytes)) - case isNoData(err), isNotFound(err): + case isAttrUnset(err), isNotFound(err): fallthrough default: // TODO the user type defaults to invalid, which is the case @@ -675,7 +675,7 @@ func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string Type: storageprovider.PKG2GRPCXS(algo), Sum: hex.EncodeToString(v), } - case isNoData(err): + case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") case isNotFound(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") @@ -697,7 +697,7 @@ func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *prov Decoder: "plain", Value: []byte(hex.EncodeToString(v)), } - case isNoData(err): + case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") case isNotFound(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") @@ -729,7 +729,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso } else { appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("quota", string(v)).Msg("malformed quota") } - case isNoData(err): + case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Msg("quota not set") case isNotFound(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Msg("file not found when reading quota") @@ -856,7 +856,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap prov switch { case err == nil: AddPermissions(&ap, g.GetPermissions()) - case isNoData(err): + case isAttrUnset(err): err = nil appctx.GetLogger(ctx).Error().Interface("node", n).Str("grant", grantees[i]).Interface("grantees", grantees).Msg("grant vanished from node after listing") // continue with next segment diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index 05a1282e86..eca4daea44 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -239,7 +239,7 @@ func (p *Permissions) HasPermission(ctx context.Context, n *Node, check func(*pr if check(g.GetPermissions()) { return true, nil } - case isNoData(err): + case isAttrUnset(err): err = nil appctx.GetLogger(ctx).Error().Interface("node", cn).Str("grant", grantees[i]).Interface("grantees", grantees).Msg("grant vanished from node after listing") default: @@ -284,14 +284,6 @@ func (p *Permissions) getUserAndPermissions(ctx context.Context, n *Node) (*user } return u, nil } -func isNoData(err error) bool { - if xerr, ok := err.(*xattr.Error); ok { - if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { - return serr == syscall.ENODATA - } - } - return false -} // The os not exists error is buried inside the xattr error, // so we cannot just use os.IsNotExists(). diff --git a/pkg/storage/utils/decomposedfs/node/permissions_darwin.go b/pkg/storage/utils/decomposedfs/node/permissions_darwin.go new file mode 100644 index 0000000000..a44b30df19 --- /dev/null +++ b/pkg/storage/utils/decomposedfs/node/permissions_darwin.go @@ -0,0 +1,37 @@ +// 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. + +//go:build darwin +// +build darwin + +package node + +import ( + "syscall" + + "github.com/pkg/xattr" +) + +func isAttrUnset(err error) bool { + if xerr, ok := err.(*xattr.Error); ok { + if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { + return serr == syscall.ENOATTR + } + } + return false +} diff --git a/pkg/storage/utils/decomposedfs/node/permissions_unix.go b/pkg/storage/utils/decomposedfs/node/permissions_unix.go new file mode 100644 index 0000000000..d10efca03a --- /dev/null +++ b/pkg/storage/utils/decomposedfs/node/permissions_unix.go @@ -0,0 +1,37 @@ +// 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. + +//go:build !darwin +// +build !darwin + +package node + +import ( + "syscall" + + "github.com/pkg/xattr" +) + +func isAttrUnset(err error) bool { + if xerr, ok := err.(*xattr.Error); ok { + if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { + return serr == syscall.ENODATA + } + } + return false +}