Blank prevValue in POST should be interpreted as a blank test-and-set, not a normal set

etcd_handlers.go

@@ -92,15 +92,15 @@ func SetHttpHandler(w http.ResponseWriter, req *http.Request) error {
 
 	debugf("[recv] POST %v/v1/keys/%s [%s]", e.url, key, req.RemoteAddr)
 
-	value := req.FormValue("value")
+	req.ParseForm()
+
+	value := req.Form.Get("value")
 
 	if len(value) == 0 {
 		return etcdErr.NewError(200, "Set")
 	}
 
-	prevValue := req.FormValue("prevValue")
-
-	strDuration := req.FormValue("ttl")
+	strDuration := req.Form.Get("ttl")
 
 	expireTime, err := durationToExpireTime(strDuration)
 
@@ -108,11 +108,11 @@ func SetHttpHandler(w http.ResponseWriter, req *http.Request) error {
 		return etcdErr.NewError(202, "Set")
 	}
 
-	if len(prevValue) != 0 {
+	if prevValueArr, ok := req.Form["prevValue"]; ok && len(prevValueArr) > 0 {
 		command := &TestAndSetCommand{
 			Key:        key,
 			Value:      value,
-			PrevValue:  prevValue,
+			PrevValue:  prevValueArr[0],
 			ExpireTime: expireTime,
 		}
 

etcd_test.go

@@ -54,6 +54,32 @@ func TestSingleNode(t *testing.T) {
 		}
 		t.Fatalf("Set 2 failed with %s %s %v", result.Key, result.Value, result.TTL)
 	}
+
+	// Add a test-and-set test
+
+	// First, we'll test we can change the value if we get it write
+	result, match, err := c.TestAndSet("foo", "bar", "foobar", 100)
+
+	if err != nil || result.Key != "/foo" || result.Value != "foobar" || result.PrevValue != "bar" || result.TTL != 99 || !match {
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Fatalf("Set 3 failed with %s %s %v", result.Key, result.Value, result.TTL)
+	}
+
+	// Next, we'll make sure we can't set it without the correct prior value
+	_, _, err = c.TestAndSet("foo", "bar", "foofoo", 100)
+
+	if err == nil {
+		t.Fatalf("Set 4 expecting error when setting key with incorrect previous value")
+	}
+
+	// Finally, we'll make sure a blank previous value still counts as a test-and-set and still has to match
+	_, _, err = c.TestAndSet("foo", "", "barbar", 100)
+
+	if err == nil {
+		t.Fatalf("Set 5 expecting error when setting key with blank (incorrect) previous value")
+	}
 }
 
 // TestInternalVersionFail will ensure that etcd does not come up if the internal raft