Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix propfind listing for files (#2506)
Browse files Browse the repository at this point in the history
David Christofas authored and butonic committed Feb 14, 2022
1 parent 1aadbc8 commit 064ec82
Showing 7 changed files with 139 additions and 55 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/propfind-listing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Fix propfind listing for files

When doing a propfind for a file the result contained the files twice.

https://github.com/owncloud/ocis/issues/3066
https://github.com/cs3org/reva/pull/2506
34 changes: 15 additions & 19 deletions internal/http/services/owncloud/ocdav/copy.go
Original file line number Diff line number Diff line change
@@ -47,7 +47,7 @@ type copy struct {
source *provider.Reference
sourceInfo *provider.ResourceInfo
destination *provider.Reference
depth string
depth net.Depth
successCode int
}

@@ -105,7 +105,7 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
}

if err := s.executePathCopy(ctx, client, w, r, cp); err != nil {
sublog.Error().Err(err).Str("depth", cp.depth).Msg("error executing path copy")
sublog.Error().Err(err).Str("depth", cp.depth.String()).Msg("error executing path copy")
w.WriteHeader(http.StatusInternalServerError)
}
w.WriteHeader(cp.successCode)
@@ -136,7 +136,7 @@ func (s *svc) executePathCopy(ctx context.Context, client gateway.GatewayAPIClie

// TODO: also copy properties: https://tools.ietf.org/html/rfc4918#section-9.8.2

if cp.depth != "infinity" {
if cp.depth != net.DepthInfinity {
return nil
}

@@ -325,7 +325,7 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s

err = s.executeSpacesCopy(ctx, w, client, cp)
if err != nil {
sublog.Error().Err(err).Str("depth", cp.depth).Msg("error descending directory")
sublog.Error().Err(err).Str("depth", cp.depth.String()).Msg("error descending directory")
w.WriteHeader(http.StatusInternalServerError)
}
w.WriteHeader(cp.successCode)
@@ -358,7 +358,7 @@ func (s *svc) executeSpacesCopy(ctx context.Context, w http.ResponseWriter, clie

// TODO: also copy properties: https://tools.ietf.org/html/rfc4918#section-9.8.2

if cp.depth != "infinity" {
if cp.depth != net.DepthInfinity {
return nil
}

@@ -488,16 +488,23 @@ func (s *svc) prepareCopy(ctx context.Context, w http.ResponseWriter, r *http.Re
errors.HandleWebdavError(log, w, b, err)
return nil
}
depth, err := extractDepth(w, r)
dh := r.Header.Get(net.HeaderDepth)
depth, err := net.ParseDepth(dh)

if err != nil {
w.WriteHeader(http.StatusBadRequest)
m := fmt.Sprintf("Depth header is set to incorrect value %v", depth)
m := fmt.Sprintf("Depth header is set to incorrect value %v", dh)
b, err := errors.Marshal(errors.SabredavBadRequest, m, "")
errors.HandleWebdavError(log, w, b, err)
return nil
}
if dh == "" {
// net.ParseDepth returns "1" for an empty value but copy expects "infinity"
// so we overwrite it here
depth = net.DepthInfinity
}

log.Debug().Str("overwrite", overwrite).Str("depth", depth).Msg("copy")
log.Debug().Str("overwrite", overwrite).Str("depth", depth.String()).Msg("copy")

client, err := s.getClient()
if err != nil {
@@ -605,14 +612,3 @@ func extractOverwrite(w http.ResponseWriter, r *http.Request) (string, error) {

return overwrite, nil
}

func extractDepth(w http.ResponseWriter, r *http.Request) (string, error) {
depth := r.Header.Get(net.HeaderDepth)
if depth == "" {
depth = "infinity"
}
if depth != "infinity" && depth != "0" {
return "", errInvalidValue
}
return depth, nil
}
35 changes: 35 additions & 0 deletions internal/http/services/owncloud/ocdav/net/net.go
Original file line number Diff line number Diff line change
@@ -44,8 +44,23 @@ const (
PropQuotaUnknown = "-2"
// PropOcFavorite is the favorite ns property
PropOcFavorite = "http://owncloud.org/ns/favorite"

// DepthZero represents the webdav zero depth value
DepthZero Depth = "0"
// DepthOne represents the webdav one depth value
DepthOne Depth = "1"
// DepthInfinity represents the webdav infinity depth value
DepthInfinity Depth = "infinity"
)

// Depth is a type representing the webdav depth header value
type Depth string

// String returns the string representation of the webdav depth value
func (d Depth) String() string {
return string(d)
}

// replaceAllStringSubmatchFunc is taken from 'Go: Replace String with Regular Expression Callback'
// see: https://elliotchance.medium.com/go-replace-string-with-regular-expression-callback-f89948bad0bb
func replaceAllStringSubmatchFunc(re *regexp.Regexp, str string, repl func([]string) string) string {
@@ -78,3 +93,23 @@ func EncodePath(path string) string {
return sb.String()
})
}

// ParseDepth parses the depth header value defined in https://tools.ietf.org/html/rfc4918#section-9.1
// Valid values are "0", "1" and "infinity". An empty string will be parsed to "1".
// For all other values this method returns an error.
func ParseDepth(s string) (Depth, error) {
if s == "" {
return DepthOne, nil
}

switch strings.ToLower(s) {
case DepthZero.String():
return DepthZero, nil
case DepthOne.String():
return DepthOne, nil
case DepthInfinity.String():
return DepthInfinity, nil
default:
return "", fmt.Errorf("invalid depth: %s", s)
}
}
55 changes: 55 additions & 0 deletions internal/http/services/owncloud/ocdav/net/net_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2018-2022 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 net

import "testing"

func TestParseDepth(t *testing.T) {
tests := map[string]Depth{
"": DepthOne,
"0": DepthZero,
"1": DepthOne,
"infinity": DepthInfinity,
}

for input, expected := range tests {
parsed, err := ParseDepth(input)
if err != nil {
t.Errorf("failed to parse depth %s", input)
}
if parsed != expected {
t.Errorf("parseDepth returned %s expected %s", parsed.String(), expected.String())
}
}

_, err := ParseDepth("invalid")
if err == nil {
t.Error("parse depth didn't return an error for invalid depth: invalid")
}
}

var result Depth

func BenchmarkParseDepth(b *testing.B) {
inputs := []string{"", "0", "1", "infinity", "INFINITY"}
size := len(inputs)
for i := 0; i < b.N; i++ {
result, _ = ParseDepth(inputs[i%size])
}
}
27 changes: 13 additions & 14 deletions internal/http/services/owncloud/ocdav/propfind/propfind.go
Original file line number Diff line number Diff line change
@@ -237,15 +237,12 @@ func (p *Handler) statSpace(ctx context.Context, client gateway.GatewayAPIClient
}

func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r *http.Request, pf XML, spaces []*provider.StorageSpace, requestPath string, spacesPropfind bool, log zerolog.Logger) ([]*provider.ResourceInfo, bool, bool) {
depth := r.Header.Get(net.HeaderDepth)
if depth == "" {
depth = "1"
}
// see https://tools.ietf.org/html/rfc4918#section-9.1
if depth != "0" && depth != "1" && depth != "infinity" {
log.Debug().Str("depth", depth).Msg("invalid Depth header value")
dh := r.Header.Get(net.HeaderDepth)
depth, err := net.ParseDepth(dh)
if err != nil {
log.Debug().Str("depth", dh).Msg(err.Error())
w.WriteHeader(http.StatusBadRequest)
m := fmt.Sprintf("Invalid Depth header value: %v", depth)
m := fmt.Sprintf("Invalid Depth header value: %v", dh)
b, err := errors.Marshal(errors.SabredavBadRequest, m, "")
errors.HandleWebdavError(&log, w, b, err)
return nil, false, false
@@ -355,7 +352,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r
// then add children
for _, spaceInfo := range spaceInfos {
switch {
case !spacesPropfind && spaceInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != "infinity":
case !spacesPropfind && spaceInfo.Type != provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth != net.DepthInfinity:
// The propfind is requested for a file that exists

childPath := strings.TrimPrefix(spaceInfo.Path, requestPath)
@@ -375,7 +372,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r
childInfos[childName] = spaceInfo
}

case spaceInfo.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1":
case spaceInfo.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == net.DepthOne:
switch {
case strings.HasPrefix(requestPath, spaceInfo.Path):
req := &provider.ListContainerRequest{
@@ -423,7 +420,7 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r
log.Debug().Msg("unhandled")
}

case depth == "infinity":
case depth == net.DepthInfinity:
// use a stack to explore sub-containers breadth-first
if spaceInfo != rootInfo {
resourceInfos = append(resourceInfos, spaceInfo)
@@ -472,9 +469,11 @@ func (p *Handler) getResourceInfos(ctx context.Context, w http.ResponseWriter, r
}
}

// now add all aggregated child infos
for _, childInfo := range childInfos {
resourceInfos = append(resourceInfos, childInfo)
if rootInfo.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER {
// now add all aggregated child infos
for _, childInfo := range childInfos {
resourceInfos = append(resourceInfos, childInfo)
}
}

sendTusHeaders := true
14 changes: 5 additions & 9 deletions internal/http/services/owncloud/ocdav/publicfile.go
Original file line number Diff line number Diff line change
@@ -92,14 +92,10 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s
sublog := appctx.GetLogger(ctx).With().Interface("tokenStatInfo", tokenStatInfo).Logger()
sublog.Debug().Msg("handlePropfindOnToken")

depth := r.Header.Get(net.HeaderDepth)
if depth == "" {
depth = "1"
}

// see https://tools.ietf.org/html/rfc4918#section-10.2
if depth != "0" && depth != "1" && depth != "infinity" {
sublog.Debug().Msgf("invalid Depth header value %s", depth)
dh := r.Header.Get(net.HeaderDepth)
depth, err := net.ParseDepth(dh)
if err != nil {
sublog.Debug().Msg(err.Error())
w.WriteHeader(http.StatusBadRequest)
return
}
@@ -114,7 +110,7 @@ func (s *svc) handlePropfindOnToken(w http.ResponseWriter, r *http.Request, ns s
// prefix tokenStatInfo.Path with token
tokenStatInfo.Path = filepath.Join(r.URL.Path, tokenStatInfo.Path)

infos := s.getPublicFileInfos(onContainer, depth == "0", tokenStatInfo)
infos := s.getPublicFileInfos(onContainer, depth == net.DepthZero, tokenStatInfo)

propRes, err := propfind.MultistatusResponse(ctx, &pf, infos, s.c.PublicURL, ns, nil)
if err != nil {
23 changes: 10 additions & 13 deletions internal/http/services/owncloud/ocdav/trashbin.go
Original file line number Diff line number Diff line change
@@ -170,27 +170,24 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "list_trashbin")
defer span.End()

depth := r.Header.Get(net.HeaderDepth)
if depth == "" {
depth = "1"
sublog := appctx.GetLogger(ctx).With().Logger()

dh := r.Header.Get(net.HeaderDepth)
depth, err := net.ParseDepth(dh)
if err != nil {
sublog.Debug().Str("depth", dh).Msg(err.Error())
w.WriteHeader(http.StatusBadRequest)
return
}

sublog := appctx.GetLogger(ctx).With().Logger()
client, err := s.getClient()
if err != nil {
sublog.Error().Err(err).Msg("error getting grpc client")
w.WriteHeader(http.StatusInternalServerError)
return
}

// see https://tools.ietf.org/html/rfc4918#section-9.1
if depth != "0" && depth != "1" && depth != "infinity" {
sublog.Debug().Str("depth", depth).Msgf("invalid Depth header value")
w.WriteHeader(http.StatusBadRequest)
return
}

if depth == "0" {
if depth == net.DepthZero {
propRes, err := h.formatTrashPropfind(ctx, s, u, nil, nil)
if err != nil {
sublog.Error().Err(err).Msg("error formatting propfind")
@@ -253,7 +250,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s

items := getRecycleRes.RecycleItems

if depth == "infinity" {
if depth == net.DepthInfinity {
var stack []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

0 comments on commit 064ec82

Please sign in to comment.