Skip to content

Commit

Permalink
Fix unset xattr on darwin (#2260)
Browse files Browse the repository at this point in the history
* Fix unset xattr on darwin

* Improve Webdav error handling on propfind

* Change method name
  • Loading branch information
micbar authored Nov 12, 2021
1 parent 67d8106 commit 17606d3
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 17 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-unset-quota.md
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions internal/http/services/owncloud/ocdav/propfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/storage/utils/decomposedfs/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
10 changes: 1 addition & 9 deletions pkg/storage/utils/decomposedfs/node/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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().
Expand Down
37 changes: 37 additions & 0 deletions pkg/storage/utils/decomposedfs/node/permissions_darwin.go
Original file line number Diff line number Diff line change
@@ -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
}
37 changes: 37 additions & 0 deletions pkg/storage/utils/decomposedfs/node/permissions_unix.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 17606d3

Please sign in to comment.