From 6252037376917169d02880954c6880ec37ccfd20 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 18:01:24 -0500 Subject: [PATCH 1/2] fix root should be rdonly --- error/error.go | 2 ++ store/store.go | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/error/error.go b/error/error.go index dddad19b4..61eb7c3f7 100644 --- a/error/error.go +++ b/error/error.go @@ -32,6 +32,7 @@ const ( EcodeNotDir = 104 EcodeNodeExist = 105 EcodeKeyIsPreserved = 106 + EcodeRootROnly = 107 EcodeValueRequired = 200 EcodePrevValueRequired = 201 @@ -56,6 +57,7 @@ func init() { errors[EcodeNoMorePeer] = "Reached the max number of peers in the cluster" errors[EcodeNotDir] = "Not A Directory" errors[EcodeNodeExist] = "Already exists" // create + errors[EcodeRootROnly] = "Root is read only" errors[EcodeKeyIsPreserved] = "The prefix of given key is a keyword in etcd" // Post form related errors diff --git a/store/store.go b/store/store.go index 1edb02f29..688e2ccf8 100644 --- a/store/store.go +++ b/store/store.go @@ -156,8 +156,6 @@ func (s *store) Get(nodePath string, recursive, sorted bool) (*Event, error) { // If the node has already existed, create will fail. // If any node on the path is a file, create will fail. func (s *store) Create(nodePath string, value string, unique bool, expireTime time.Time) (*Event, error) { - nodePath = path.Clean(path.Join("/", nodePath)) - s.worldLock.Lock() defer s.worldLock.Unlock() e, err := s.internalCreate(nodePath, value, unique, false, expireTime, Create) @@ -173,8 +171,6 @@ func (s *store) Create(nodePath string, value string, unique bool, expireTime ti // Set function creates or replace the Node at nodePath. func (s *store) Set(nodePath string, value string, expireTime time.Time) (*Event, error) { - nodePath = path.Clean(path.Join("/", nodePath)) - s.worldLock.Lock() defer s.worldLock.Unlock() e, err := s.internalCreate(nodePath, value, false, true, expireTime, Set) @@ -192,6 +188,10 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint value string, expireTime time.Time) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } s.worldLock.Lock() defer s.worldLock.Unlock() @@ -238,6 +238,10 @@ func (s *store) CompareAndSwap(nodePath string, prevValue string, prevIndex uint // If the node is a directory, recursive must be true to delete it. func (s *store) Delete(nodePath string, recursive bool) (*Event, error) { nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } s.worldLock.Lock() defer s.worldLock.Unlock() @@ -334,13 +338,17 @@ func (s *store) walk(nodePath string, walkFunc func(prev *Node, component string // If the node is a file, the value and the ttl can be updated. // If the node is a directory, only the ttl can be updated. func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (*Event, error) { + nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", s.CurrentIndex) + } + s.worldLock.Lock() defer s.worldLock.Unlock() currIndex, nextIndex := s.CurrentIndex, s.CurrentIndex+1 - nodePath = path.Clean(path.Join("/", nodePath)) - n, err := s.internalGet(nodePath) if err != nil { // if the node does not exist, return error @@ -390,6 +398,11 @@ func (s *store) internalCreate(nodePath string, value string, unique bool, repla nodePath = path.Clean(path.Join("/", nodePath)) + // we do not allow the user to change "/" + if nodePath == "/" { + return nil, etcdErr.NewError(etcdErr.EcodeRootROnly, "/", currIndex) + } + // Assume expire times that are way in the past are not valid. // This can occur when the time is serialized to JSON and read back in. if expireTime.Before(minExpireTime) { From b929e71948fbb2fcc5c960390b999fef29916162 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Sun, 1 Dec 2013 18:16:32 -0500 Subject: [PATCH 2/2] tests add root readonly test --- store/store_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/store/store_test.go b/store/store_test.go index 29b2986b9..863874fcb 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -213,6 +213,26 @@ func TestStoreDeleteDiretory(t *testing.T) { assert.Equal(t, e.Action, "delete", "") } +func TestRootRdOnly(t *testing.T) { + s := newStore() + + _, err := s.Set("/", "", Permanent) + assert.NotNil(t, err, "") + + _, err = s.Delete("/", true) + assert.NotNil(t, err, "") + + _, err = s.Create("/", "", false, Permanent) + assert.NotNil(t, err, "") + + _, err = s.Update("/", "", Permanent) + assert.NotNil(t, err, "") + + _, err = s.CompareAndSwap("/", "", 0, "", Permanent) + assert.NotNil(t, err, "") + +} + // Ensure that the store cannot delete a directory if recursive is not specified. func TestStoreDeleteDiretoryFailsIfNonRecursive(t *testing.T) { s := newStore()