syncbase: Relax restrictions on wire.Id Name.
Relaxed restrictions on wire.Id Name component to allow up to 1024 byte
(previously 64 byte), arbitrary Unicode (previously C identifier) names,
excluding only Unicode control characters (notably including \0 and \n).
Added restriction on blessings (in general, not just Id blessings) and
row keys to also exclude Unicode control characters.
Also cleaned up key splitting helper usage.
MultiPart: 1/3
Change-Id: I28115b84171a07aabbde7876a642303e7002353e
diff --git a/security/certificate.go b/security/certificate.go
index 2738712..d67ee1e 100644
--- a/security/certificate.go
+++ b/security/certificate.go
@@ -9,6 +9,7 @@
"crypto/sha256"
"strings"
"sync"
+ "unicode"
"v.io/v23/verror"
)
@@ -22,6 +23,7 @@
errBadBlessingBadStart = verror.Register(pkgPath+".errBadBlessingBadStart", verror.NoRetry, "{1:}{2:}invalid blessing extension(starts with {3}){:_}")
errBadBlessingBadEnd = verror.Register(pkgPath+".errBadBlessingBadEnd", verror.NoRetry, "{1:}{2:}invalid blessing extension(ends with {3}){:_}")
errBadBlessingExtension = verror.Register(pkgPath+".errBadBlessingExtension", verror.NoRetry, "{1:}{2:}invalid blessing extension({3}){:_}")
+ errBadBlessingControlChar = verror.Register(pkgPath+".errBadBlessingControlChar", verror.NoRetry, "{1:}{2:}invalid blessing extension({3} contains control character as a substring){:_}")
errBadBlessingBadSubstring = verror.Register(pkgPath+".errBadBlessingBadSubstring", verror.NoRetry, "{1:}{2:}invalid blessing extension({3} has {4} as a substring){:_}")
// invalidBlessingSubStrings are strings that a blessing extension cannot have as a substring.
@@ -105,6 +107,9 @@
return verror.New(errBadBlessingExtension, nil, extension)
}
}
+ if strings.IndexFunc(extension, unicode.IsControl) != -1 {
+ return verror.New(errBadBlessingControlChar, nil, extension)
+ }
for _, n := range invalidBlessingSubStrings {
if strings.Contains(extension, n) {
return verror.New(errBadBlessingBadSubstring, nil, extension, n)
diff --git a/services/syncbase/types.go b/services/syncbase/types.go
index a4f0133..a7978a7 100644
--- a/services/syncbase/types.go
+++ b/services/syncbase/types.go
@@ -11,7 +11,8 @@
// Note that "," is not allowed to appear in blessing patterns. We also
// could've used "/" as a separator, but then we would've had to be more
-// careful with decoding and splitting name components elsewhere.
+// careful with decoding and splitting name components elsewhere. However,
+// "," is allowed to appear in id names.
// TODO(sadovsky): Maybe define "," constant in v23/services/syncbase.
// String implements Stringer.
@@ -21,6 +22,7 @@
// ParseId parses an Id from a string formatted according to the String method.
func ParseId(from string) (Id, error) {
+ // SplitN must be used because "," is allowed to appear in id names.
parts := strings.SplitN(from, ",", 2)
if len(parts) != 2 {
return Id{}, fmt.Errorf("failed to parse id: %s", from)
diff --git a/syncbase/featuretests/test_util_test.go b/syncbase/featuretests/test_util_test.go
index 5c56e06..42b4c93 100644
--- a/syncbase/featuretests/test_util_test.go
+++ b/syncbase/featuretests/test_util_test.go
@@ -311,6 +311,8 @@
// parseSgCollections converts, for example, "a,c" to
// [Collection: {clientBlessing, "a"}, Collection: {clientBlessing, "c"}].
func parseSgCollections(csv string) []wire.Id {
+ // Note that "," is allowed to appear in syncgroup names in general.
+ // Restricting it in tests is acceptable.
strs := strings.Split(csv, ",")
res := make([]wire.Id, len(strs))
for i, v := range strs {
diff --git a/syncbase/util/util.go b/syncbase/util/util.go
index 585bbb2..e976f06 100644
--- a/syncbase/util/util.go
+++ b/syncbase/util/util.go
@@ -6,8 +6,8 @@
import (
"fmt"
- "regexp"
"strings"
+ "unicode"
"v.io/v23/context"
"v.io/v23/conventions"
@@ -54,8 +54,8 @@
// separator in storage engine keys. \xfc and \xfd are not used. In the future
// we might use \xfc followed by "c", "d", "e", or "f" as an order-preserving
// 2-byte encoding of our reserved bytes. Note that all valid UTF-8 byte
-// sequences are allowed.
-var reservedBytes = []string{"\xfc", "\xfd", "\xfe", "\xff"}
+// sequences not containing the NUL character are allowed.
+var reservedBytes = []string{"\x00", "\xfc", "\xfd", "\xfe", "\xff"}
func containsAnyOf(s string, needles []string) bool {
for _, v := range needles {
@@ -66,15 +66,12 @@
return false
}
-// TODO(ivanpi): Consider relaxing this.
-var idNameRegexp *regexp.Regexp = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_]*$")
-
const (
// maxBlessingLen is the max allowed number of bytes in an Id blessing.
// TODO(ivanpi): Blessings are theoretically unbounded in length.
maxBlessingLen = 1024
// maxNameLen is the max allowed number of bytes in an Id name.
- maxNameLen = 64
+ maxNameLen = 1024
)
// ValidateId returns nil iff the given Id is a valid database, collection, or
@@ -93,14 +90,17 @@
}
if bp := security.BlessingPattern(id.Blessing); bp == security.NoExtension {
return fmt.Errorf("Id blessing %q cannot match any blessings, check blessing conventions", id.Blessing)
- } else if !bp.IsValid() {
+ } else if !bp.IsValid() { // also checks for control characters
return fmt.Errorf("Id blessing %q is not a valid blessing pattern", id.Blessing)
}
if containsAnyOf(id.Blessing, reservedBytes) {
return fmt.Errorf("Id blessing %q contains a reserved byte (one of %q)", id.Blessing, reservedBytes)
}
- if !idNameRegexp.MatchString(id.Name) {
- return fmt.Errorf("Id name %q does not satisfy regex %q", id.Name, idNameRegexp.String())
+ if strings.IndexFunc(id.Name, unicode.IsControl) != -1 {
+ return fmt.Errorf("Id name %q contains a control character", id.Name)
+ }
+ if containsAnyOf(id.Name, reservedBytes) {
+ return fmt.Errorf("Id name %q contains a reserved byte (one of %q)", id.Name, reservedBytes)
}
return nil
}
@@ -110,6 +110,9 @@
if s == "" {
return fmt.Errorf("row key cannot be empty")
}
+ if strings.IndexFunc(s, unicode.IsControl) != -1 {
+ return fmt.Errorf("row key %q contains a control character", s)
+ }
if containsAnyOf(s, reservedBytes) {
return fmt.Errorf("row key %q contains a reserved byte (one of %q)", s, reservedBytes)
}
