2960: Rewrite getLocatorInfo and add some more error checks.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index d85961fd2a8d7c6d07d8c2a3ecbd1e1bc75a7876..b29c8817690b548c4188b003c76a56a2ca876d35 100644 (file)
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -719,31 +719,44 @@ type locatorInfo struct {
 
 func getLocatorInfo(loc string) (locatorInfo, error) {
        var li locatorInfo
-       for i, part := range strings.Split(loc, "+") {
-               if i == 0 {
-                       if len(part) != 32 {
+       plus := 0    // number of '+' chars seen so far
+       partlen := 0 // chars since last '+'
+       for i, c := range loc + "+" {
+               if c == '+' {
+                       if partlen == 0 {
+                               // double/leading/trailing '+'
                                return li, errInvalidLocator
                        }
-                       li.hash = part
+                       if plus == 0 {
+                               if i != 32 {
+                                       return li, errInvalidLocator
+                               }
+                               li.hash = loc[:i]
+                       }
+                       if plus == 1 {
+                               if size, err := strconv.Atoi(loc[i-partlen : i]); err == nil {
+                                       li.size = size
+                               }
+                       }
+                       plus++
+                       partlen = 0
                        continue
                }
-               if i == 1 {
-                       if size, err := strconv.Atoi(part); err == nil {
-                               li.size = size
-                               continue
+               partlen++
+               if partlen == 1 {
+                       if c == 'A' {
+                               li.signed = true
+                       }
+                       if c == 'R' {
+                               li.remote = true
+                       }
+                       if plus > 1 && c >= '0' && c <= '9' {
+                               // size, if present at all, must come first
+                               return li, errInvalidLocator
                        }
                }
-               if len(part) == 0 {
-                       return li, errInvalidLocator
-               }
-               if part[0] == 'A' {
-                       li.signed = true
-               }
-               if part[0] == 'R' {
-                       li.remote = true
-               }
-               if part[0] >= '0' && part[0] <= '9' {
-                       // size, if present at all, must come first
+               if plus == 0 && !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) {
+                       // non-hexadecimal char in hash part
                        return li, errInvalidLocator
                }
        }

diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 79d51829fee0ee7521aacdfc776fefc9da2bcc9a..f9d9888f984872aded1ee653054dde8eb0d19fc4 100644 (file)
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -606,7 +606,7 @@ func (s *keepstoreSuite) TestBlockWrite_SkipReadOnly(c *C) {
        c.Check(ks.mounts["zzzzz-nyw5e-222222222222222"].volume.(*stubVolume).stubLog.String(), HasLen, 0)
 }
 
-func (s *keepstoreSuite) TestParseLocator(c *C) {
+func (s *keepstoreSuite) TestGetLocatorInfo(c *C) {
        for _, trial := range []struct {
                locator string
                ok      bool
@@ -622,6 +622,15 @@ func (s *keepstoreSuite) TestParseLocator(c *C) {
                        ok: true, expect: locatorInfo{size: 1234, remote: true}},
                {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+12345+Zexample+Rzzzzz-abcdef",
                        ok: true, expect: locatorInfo{size: 12345, remote: true}},
+               {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+123456+👶🦈+Rzzzzz-abcdef",
+                       ok: true, expect: locatorInfo{size: 123456, remote: true}},
+               // invalid: bad hash char
+               {locator: "aaaaaaaaaaaaaazaaaaaaaaaaaaaaaaa+1234",
+                       ok: false},
+               {locator: "aaaaaaaaaaaaaaFaaaaaaaaaaaaaaaaa+1234",
+                       ok: false},
+               {locator: "aaaaaaaaaaaaaa⛵aaaaaaaaaaaaaaaaa+1234",
+                       ok: false},
                // invalid: hash length != 32
                {locator: "",
                        ok: false},
@@ -636,6 +645,15 @@ func (s *keepstoreSuite) TestParseLocator(c *C) {
                // invalid: first hint is not size
                {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+Abcdef+1234",
                        ok: false},
+               // invalid: leading/trailing/double +
+               {locator: "+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234",
+                       ok: false},
+               {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234+",
+                       ok: false},
+               {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa++1234",
+                       ok: false},
+               {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234++Abcdef@abcdef",
+                       ok: false},
        } {
                c.Logf("=== %s", trial.locator)
                li, err := getLocatorInfo(trial.locator)