From 2da1010cf7af020f19b490ce29868acf66a67fce Mon Sep 17 00:00:00 2001
From: Jonathan Boulle <jonathanboulle@gmail.com>
Date: Wed, 24 Sep 2014 17:35:53 -0700
Subject: [PATCH] etcdhttp: make TTL= equivalent to unset, and TTL=0 expire

---
 etcdserver/etcdhttp/http.go      | 30 ++++++++++++++++++------------
 etcdserver/etcdhttp/http_test.go | 20 ++++++++++----------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/etcdserver/etcdhttp/http.go b/etcdserver/etcdhttp/http.go
index ca4750bef..a1b2919dd 100644
--- a/etcdserver/etcdhttp/http.go
+++ b/etcdserver/etcdhttp/http.go
@@ -165,7 +165,7 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 	}
 	p := r.URL.Path[len(keysPrefix):]
 
-	var pIdx, wIdx, ttl uint64
+	var pIdx, wIdx uint64
 	if pIdx, err = getUint64(r.Form, "prevIndex"); err != nil {
 		return emptyReq, etcdErr.NewRequestError(
 			etcdErr.EcodeIndexNaN,
@@ -178,15 +178,6 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 			`invalid value for "waitIndex"`,
 		)
 	}
-	// An empty TTL value is equivalent to it being unset entirely
-	if len(r.FormValue("ttl")) > 0 {
-		if ttl, err = getUint64(r.Form, "ttl"); err != nil {
-			return emptyReq, etcdErr.NewRequestError(
-				etcdErr.EcodeTTLNaN,
-				`invalid value for "ttl"`,
-			)
-		}
-	}
 
 	var rec, sort, wait, dir, stream bool
 	if rec, err = getBool(r.Form, "recursive"); err != nil {
@@ -236,6 +227,20 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 		)
 	}
 
+	// TTL is nullable, so leave it null if not specified
+	// or an empty string
+	var ttl *uint64
+	if len(r.FormValue("ttl")) > 0 {
+		i, err := getUint64(r.Form, "ttl")
+		if err != nil {
+			return emptyReq, etcdErr.NewRequestError(
+				etcdErr.EcodeTTLNaN,
+				`invalid value for "ttl"`,
+			)
+		}
+		ttl = &i
+	}
+
 	// prevExist is nullable, so leave it null if not specified
 	var pe *bool
 	if _, ok := r.Form["prevExist"]; ok {
@@ -269,10 +274,11 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 		rr.PrevExist = pe
 	}
 
+	// Null TTL is equivalent to unset Expiration
 	// TODO(jonboulle): use fake clock instead of time module
 	// https://github.com/coreos/etcd/issues/1021
-	if ttl > 0 {
-		expr := time.Duration(ttl) * time.Second
+	if ttl != nil {
+		expr := time.Duration(*ttl) * time.Second
 		rr.Expiration = time.Now().Add(expr).UnixNano()
 	}
 
diff --git a/etcdserver/etcdhttp/http_test.go b/etcdserver/etcdhttp/http_test.go
index 66f215d68..818750b98 100644
--- a/etcdserver/etcdhttp/http_test.go
+++ b/etcdserver/etcdhttp/http_test.go
@@ -293,16 +293,6 @@ func TestGoodParseRequest(t *testing.T) {
 				Path:   "/foo",
 			},
 		},
-		{
-			// zero TTL specified
-			mustNewRequest(t, "foo?ttl=0"),
-			etcdserverpb.Request{
-				Id:         1234,
-				Method:     "GET",
-				Path:       "/foo",
-				Expiration: 0,
-			},
-		},
 		{
 			// empty TTL specified
 			mustNewRequest(t, "foo?ttl="),
@@ -433,6 +423,16 @@ func TestGoodParseRequest(t *testing.T) {
 	if got.Expiration <= now {
 		t.Fatalf("expiration = %v, wanted > %v", got.Expiration, now)
 	}
+
+	// ensure TTL=0 results in an expiration time
+	req = mustNewForm(t, "foo", url.Values{"ttl": []string{"0"}})
+	got, err = parseRequest(req, 1234)
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	if got.Expiration <= now {
+		t.Fatalf("expiration = %v, wanted > %v", got.Expiration, now)
+	}
 }
 
 // eventingWatcher immediately returns a simple event of the given action on its channel