Skip to content

Commit

Permalink
validate paths before performing operations (samuel#169)
Browse files Browse the repository at this point in the history
Java and other clients will validate on the client side before sending
it on.  Although this should be handled server side, there are
apparently some implemenations that don't do this server side, so this
follows suit with the other client libraries.

Fixes samuel#63
  • Loading branch information
nemith authored and samuel committed Aug 4, 2017
1 parent 6f3354f commit 8ac67fa
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 2 deletions.
53 changes: 51 additions & 2 deletions zk/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -906,12 +906,20 @@ func (c *Conn) AddAuth(scheme string, auth []byte) error {
}

func (c *Conn) Children(path string) ([]string, *Stat, error) {
if err := validatePath(path); err != nil {
return nil, nil, err
}

res := &getChildren2Response{}
_, err := c.request(opGetChildren2, &getChildren2Request{Path: path, Watch: false}, res, nil)
return res.Children, &res.Stat, err
}

func (c *Conn) ChildrenW(path string) ([]string, *Stat, <-chan Event, error) {
if err := validatePath(path); err != nil {
return nil, nil, nil, err
}

var ech <-chan Event
res := &getChildren2Response{}
_, err := c.request(opGetChildren2, &getChildren2Request{Path: path, Watch: true}, res, func(req *request, res *responseHeader, err error) {
Expand All @@ -926,13 +934,21 @@ func (c *Conn) ChildrenW(path string) ([]string, *Stat, <-chan Event, error) {
}

func (c *Conn) Get(path string) ([]byte, *Stat, error) {
if err := validatePath(path); err != nil {
return nil, nil, err
}

res := &getDataResponse{}
_, err := c.request(opGetData, &getDataRequest{Path: path, Watch: false}, res, nil)
return res.Data, &res.Stat, err
}

// GetW returns the contents of a znode and sets a watch
func (c *Conn) GetW(path string) ([]byte, *Stat, <-chan Event, error) {
if err := validatePath(path); err != nil {
return nil, nil, nil, err
}

var ech <-chan Event
res := &getDataResponse{}
_, err := c.request(opGetData, &getDataRequest{Path: path, Watch: true}, res, func(req *request, res *responseHeader, err error) {
Expand All @@ -947,15 +963,20 @@ func (c *Conn) GetW(path string) ([]byte, *Stat, <-chan Event, error) {
}

func (c *Conn) Set(path string, data []byte, version int32) (*Stat, error) {
if path == "" {
return nil, ErrInvalidPath
if err := validatePath(path); err != nil {
return nil, err
}

res := &setDataResponse{}
_, err := c.request(opSetData, &SetDataRequest{path, data, version}, res, nil)
return &res.Stat, err
}

func (c *Conn) Create(path string, data []byte, flags int32, acl []ACL) (string, error) {
if err := validatePath(path); err != nil {
return "", err
}

res := &createResponse{}
_, err := c.request(opCreate, &CreateRequest{path, data, acl, flags}, res, nil)
return res.Path, err
Expand All @@ -966,6 +987,10 @@ func (c *Conn) Create(path string, data []byte, flags int32, acl []ACL) (string,
// ephemeral node still exists. Therefore, on reconnect we need to check if a node
// with a GUID generated on create exists.
func (c *Conn) CreateProtectedEphemeralSequential(path string, data []byte, acl []ACL) (string, error) {
if err := validatePath(path); err != nil {
return "", err
}

var guid [16]byte
_, err := io.ReadFull(rand.Reader, guid[:16])
if err != nil {
Expand Down Expand Up @@ -1007,11 +1032,19 @@ func (c *Conn) CreateProtectedEphemeralSequential(path string, data []byte, acl
}

func (c *Conn) Delete(path string, version int32) error {
if err := validatePath(path); err != nil {
return err
}

_, err := c.request(opDelete, &DeleteRequest{path, version}, &deleteResponse{}, nil)
return err
}

func (c *Conn) Exists(path string) (bool, *Stat, error) {
if err := validatePath(path); err != nil {
return false, nil, err
}

res := &existsResponse{}
_, err := c.request(opExists, &existsRequest{Path: path, Watch: false}, res, nil)
exists := true
Expand All @@ -1023,6 +1056,10 @@ func (c *Conn) Exists(path string) (bool, *Stat, error) {
}

func (c *Conn) ExistsW(path string) (bool, *Stat, <-chan Event, error) {
if err := validatePath(path); err != nil {
return false, nil, nil, err
}

var ech <-chan Event
res := &existsResponse{}
_, err := c.request(opExists, &existsRequest{Path: path, Watch: true}, res, func(req *request, res *responseHeader, err error) {
Expand All @@ -1044,17 +1081,29 @@ func (c *Conn) ExistsW(path string) (bool, *Stat, <-chan Event, error) {
}

func (c *Conn) GetACL(path string) ([]ACL, *Stat, error) {
if err := validatePath(path); err != nil {
return nil, nil, err
}

res := &getAclResponse{}
_, err := c.request(opGetAcl, &getAclRequest{Path: path}, res, nil)
return res.Acl, &res.Stat, err
}
func (c *Conn) SetACL(path string, acl []ACL, version int32) (*Stat, error) {
if err := validatePath(path); err != nil {
return nil, err
}

res := &setAclResponse{}
_, err := c.request(opSetAcl, &setAclRequest{Path: path, Acl: acl, Version: version}, res, nil)
return &res.Stat, err
}

func (c *Conn) Sync(path string) (string, error) {
if err := validatePath(path); err != nil {
return "", err
}

res := &syncResponse{}
_, err := c.request(opSync, &syncRequest{Path: path}, res, nil)
return res.Path, err
Expand Down
62 changes: 62 additions & 0 deletions zk/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"math/rand"
"strconv"
"strings"
"unicode/utf8"
)

// AuthACL produces an ACL list containing a single ACL which uses the
Expand Down Expand Up @@ -52,3 +53,64 @@ func stringShuffle(s []string) {
s[i], s[j] = s[j], s[i]
}
}

// validatePath will make sure a path is valid before sending the request
func validatePath(path string) error {
if path == "" {
return ErrInvalidPath
}

if path[0] != '/' {
return ErrInvalidPath
}

n := len(path)
if n == 1 {
// path is just the root
return nil
}

if path[n-1] == '/' {
return ErrInvalidPath
}

// Start at rune 1 since we already know that the first character is
// a '/'.
for i, w := 1, 0; i < n; i += w {
r, width := utf8.DecodeRuneInString(path[i:])
switch {
case r == '\u0000':
return ErrInvalidPath
case r == '/':
last, _ := utf8.DecodeLastRuneInString(path[:i])
if last == '/' {
return ErrInvalidPath
}
case r == '.':
last, lastWidth := utf8.DecodeLastRuneInString(path[:i])

// Check for double dot
if last == '.' {
last, _ = utf8.DecodeLastRuneInString(path[:i-lastWidth])
}

if last == '/' {
if i+1 == n {
return ErrInvalidPath
}

next, _ := utf8.DecodeRuneInString(path[i+w:])
if next == '/' {
return ErrInvalidPath
}
}
case r >= '\u0000' && r <= '\u001f',
r >= '\u007f' && r <= '\u009f',
r >= '\uf000' && r <= '\uf8ff',
r >= '\ufff0' && r < '\uffff':
return ErrInvalidPath
}
w = width
}
return nil
}
37 changes: 37 additions & 0 deletions zk/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,40 @@ func TestFormatServers(t *testing.T) {
}
}
}

func TestValidatePath(t *testing.T) {
tt := []struct {
path string
valid bool
}{
{"/this is / a valid/path", true},
{"/", true},
{"", false},
{"not/valid", false},
{"/ends/with/slash/", false},
{"/test\u0000", false},
{"/double//slash", false},
{"/single/./period", false},
{"/double/../period", false},
{"/double/..ok/period", true},
{"/double/alsook../period", true},
{"/double/period/at/end/..", false},
{"/name/with.period", true},
{"/test\u0001", false},
{"/test\u001f", false},
{"/test\u0020", true}, // first allowable
{"/test\u007e", true}, // last valid ascii
{"/test\u007f", false},
{"/test\u009f", false},
{"/test\uf8ff", false},
{"/test\uffef", true},
{"/test\ufff0", false},
}

for _, tc := range tt {
err := validatePath(tc.path)
if (err != nil) == tc.valid {
t.Errorf("failed to validate path %q", tc.path)
}
}
}

0 comments on commit 8ac67fa

Please sign in to comment.