Ocdav improvements (#2658)

* use different props method for static values

When putting static values in propstat properties we can skip the xml encoding when we know that there are no reserved characters. Same for 'not found' properties.

* reduce unnecessary use of pointers

* simplify the EncodePath method

The results of the old code were a bit different but this version is
still compliant to the webdav rfc:
https://datatracker.ietf.org/doc/html/rfc4918#section-8.3.1.
It is also a whole lot faster.

* add changelog

* improve naming of prop package

* update core commit id

.drone.env

@@ -1,3 +1,3 @@
 # The test runner source for API tests
-CORE_COMMITID=e285879a8a79e692497937ebf340bc6b9c925b4f
+CORE_COMMITID=cb558615d4a1850629be1c463454dabac71a4905
 CORE_BRANCH=master

changelog/unreleased/ocdav-cleanup.md

@@ -0,0 +1,5 @@
+Change: small clean up of the ocdav code  
+
+Cleaned up the ocdav code to make it more readable and in one case a bit faster.
+
+https://github.com/cs3org/reva/pull/2658

internal/http/services/owncloud/ocdav/locks.go

@@ -36,7 +36,7 @@ import (
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
-	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/props"
+	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/prop"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	ctxpkg "github.com/cs3org/reva/v2/pkg/ctx"
@@ -557,10 +557,10 @@ func writeLockInfo(w io.Writer, token string, ld LockDetails) (int, error) {
 		lockdiscovery.WriteString("  <d:timeout>Infinite</d:timeout>\n")
 	}
 	if token != "" {
-		lockdiscovery.WriteString(fmt.Sprintf("  <d:locktoken><d:href>%s</d:href></d:locktoken>\n", props.Escape(token)))
+		lockdiscovery.WriteString(fmt.Sprintf("  <d:locktoken><d:href>%s</d:href></d:locktoken>\n", prop.Escape(token)))
 	}
 	if href != "" {
-		lockdiscovery.WriteString(fmt.Sprintf("  <d:lockroot><d:href>%s</d:href></d:lockroot>\n", props.Escape(href)))
+		lockdiscovery.WriteString(fmt.Sprintf("  <d:lockroot><d:href>%s</d:href></d:lockroot>\n", prop.Escape(href)))
 	}
 	lockdiscovery.WriteString("</d:activelock></d:lockdiscovery></d:prop>")
 

internal/http/services/owncloud/ocdav/net/net.go

@@ -19,9 +19,7 @@
 package net
 
 import (
-	"fmt"
 	"net/url"
-	"regexp"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -69,37 +67,11 @@ 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 {
-	result := ""
-	lastIndex := 0
-	for _, v := range re.FindAllSubmatchIndex([]byte(str), -1) {
-		groups := []string{}
-		for i := 0; i < len(v); i += 2 {
-			groups = append(groups, str[v[i]:v[i+1]])
-		}
-		result += str[lastIndex:v[0]] + repl(groups)
-		lastIndex = v[1]
-	}
-	return result + str[lastIndex:]
-}
-
-var hrefre = regexp.MustCompile(`([^A-Za-z0-9_\-.~()/:@!$])`)
-
 // EncodePath encodes the path of a url.
 //
 // slashes (/) are treated as path-separators.
-// ported from https://github.com/sabre-io/http/blob/bb27d1a8c92217b34e778ee09dcf79d9a2936e84/lib/functions.php#L369-L379
 func EncodePath(path string) string {
-	return replaceAllStringSubmatchFunc(hrefre, path, func(groups []string) string {
-		b := groups[1]
-		var sb strings.Builder
-		for i := 0; i < len(b); i++ {
-			sb.WriteString(fmt.Sprintf("%%%x", b[i]))
-		}
-		return sb.String()
-	})
+	return (&url.URL{Path: path}).EscapedPath()
 }
 
 // ParseDepth parses the depth header value defined in https://tools.ietf.org/html/rfc4918#section-9.1

internal/http/services/owncloud/ocdav/net/net_test.go

@@ -64,7 +64,7 @@ var _ = Describe("Net", func() {
 	Describe("EncodePath", func() {
 		It("encodes paths", func() {
 			Expect(net.EncodePath("foo")).To(Equal("foo"))
-			Expect(net.EncodePath("/some/path/Folder %^*(#1)")).To(Equal("/some/path/Folder%20%25%5e%2a(%231)"))
+			Expect(net.EncodePath("/some/path/Folder %^*(#1)")).To(Equal("/some/path/Folder%20%25%5E%2A%28%231%29"))
 		})
 
 		/*

internal/http/services/owncloud/ocdav/props/props.go → internal/http/services/owncloud/ocdav/prop/prop.go

@@ -16,7 +16,7 @@
 // granted to it by virtue of its status as an Intergovernmental Organization
 // or submit itself to any jurisdiction.
 
-package props
+package prop
 
 import (
 	"bytes"
@@ -49,29 +49,45 @@ func xmlEscaped(val string) []byte {
 	return buf.Bytes()
 }
 
-// NewPropNS returns a new PropertyXML instance
-func NewPropNS(namespace string, local string, val string) *PropertyXML {
-	return &PropertyXML{
+// EscapedNS returns a new PropertyXML instance while xml-escaping the value
+func EscapedNS(namespace string, local string, val string) PropertyXML {
+	return PropertyXML{
 		XMLName:  xml.Name{Space: namespace, Local: local},
 		Lang:     "",
 		InnerXML: xmlEscaped(val),
 	}
 }
 
-// NewProp returns a new PropertyXML instance while xml-escaping the value
+// Escaped returns a new PropertyXML instance while xml-escaping the value
 // TODO properly use the space
-func NewProp(key, val string) *PropertyXML {
-	return &PropertyXML{
+func Escaped(key, val string) PropertyXML {
+	return PropertyXML{
 		XMLName:  xml.Name{Space: "", Local: key},
 		Lang:     "",
 		InnerXML: xmlEscaped(val),
 	}
 }
 
-// NewPropRaw returns a new PropertyXML instance for the given key/value pair
+// NotFound returns a new PropertyXML instance with an empty value
+func NotFound(key string) PropertyXML {
+	return PropertyXML{
+		XMLName: xml.Name{Space: "", Local: key},
+		Lang:    "",
+	}
+}
+
+// NotFoundNS returns a new PropertyXML instance with the given namespace and an empty value
+func NotFoundNS(namespace, key string) PropertyXML {
+	return PropertyXML{
+		XMLName: xml.Name{Space: namespace, Local: key},
+		Lang:    "",
+	}
+}
+
+// Raw returns a new PropertyXML instance for the given key/value pair
 // TODO properly use the space
-func NewPropRaw(key, val string) *PropertyXML {
-	return &PropertyXML{
+func Raw(key, val string) PropertyXML {
+	return PropertyXML{
 		XMLName:  xml.Name{Space: "", Local: key},
 		Lang:     "",
 		InnerXML: []byte(val),