[ARVADOS] created: 979c1342922cab9a3ecd1c43709be0b9420b7ed8

[git at public.curoverse.com]

        at  979c1342922cab9a3ecd1c43709be0b9420b7ed8 (commit)


commit 979c1342922cab9a3ecd1c43709be0b9420b7ed8
Author: Peter Amstutz <peter.amstutz at curoverse.com>
Date:   Wed Feb 25 16:42:10 2015 -0500

    5309: Fix keepclient and keepproxy bugs related to error handling:
    * KeepClient: Handle nil response and nil response Body from Client.Do(GET)
    * KeepProxy: Only defer reader.Close() if reader is not nil
    * KeepProxy tests: Add test for GET failure to read (404)

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 23af470..b81e070 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -159,10 +159,17 @@ func (this KeepClient) AuthorizedGet(hash string,
 
 		var resp *http.Response
 		if resp, err = this.Client.Do(req); err != nil || resp.StatusCode != http.StatusOK {
-			respbody, _ := ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
+			statusCode := -1
+			var respbody []byte
+			if resp != nil {
+				statusCode = resp.StatusCode
+				if resp.Body != nil {
+					respbody, _ = ioutil.ReadAll(&io.LimitedReader{resp.Body, 4096})
+				}
+			}
 			response := strings.TrimSpace(string(respbody))
 			log.Printf("[%v] Download %v status code: %v error: \"%v\" response: \"%v\"",
-				requestId, url, resp.StatusCode, err, response)
+				requestId, url, statusCode, err, response)
 			continue
 		}
 
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index ea14c6c..b8c30d1 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -312,14 +312,14 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 
 	if req.Method == "GET" {
 		reader, blocklen, _, err = kc.AuthorizedGet(hash, locator.Signature, locator.Timestamp)
-		defer reader.Close()
+		if reader != nil {
+			defer reader.Close()
+		}
 	} else if req.Method == "HEAD" {
 		blocklen, _, err = kc.AuthorizedAsk(hash, locator.Signature, locator.Timestamp)
 	}
 
-	if blocklen > -1 {
-		resp.Header().Set("Content-Length", fmt.Sprint(blocklen))
-	} else {
+	if blocklen == -1 {
 		log.Printf("%s: %s %s Keep server did not return Content-Length",
 			GetRemoteAddress(req), req.Method, hash)
 	}
@@ -328,6 +328,7 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 	switch err {
 	case nil:
 		status = http.StatusOK
+		resp.Header().Set("Content-Length", fmt.Sprint(blocklen))
 		if reader != nil {
 			n, err2 := io.Copy(resp, reader)
 			if blocklen > -1 && n != blocklen {
@@ -345,7 +346,7 @@ func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 		}
 	case keepclient.BlockNotFound:
 		status = http.StatusNotFound
-		http.Error(resp, "Not found", http.StatusNotFound)
+		http.Error(resp, "Not Found", http.StatusNotFound)
 	default:
 		status = http.StatusBadGateway
 		http.Error(resp, err.Error(), http.StatusBadGateway)
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index ccbd7d8..57f6268 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -110,10 +110,10 @@ func runProxy(c *C, args []string, port int, bogusClientToken bool) keepclient.K
 	arv, err := arvadosclient.MakeArvadosClient()
 	c.Assert(err, Equals, nil)
 	kc := keepclient.KeepClient{
-		Arvados: &arv,
+		Arvados:       &arv,
 		Want_replicas: 2,
-		Using_proxy: true,
-		Client: &http.Client{},
+		Using_proxy:   true,
+		Client:        &http.Client{},
 	}
 	kc.SetServiceRoots(map[string]string{
 		"proxy": fmt.Sprintf("http://localhost:%v", port),
@@ -173,6 +173,13 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 	}
 
 	{
+		reader, _, _, err := kc.Get(hash)
+		c.Check(reader, Equals, nil)
+		c.Check(err, Equals, keepclient.BlockNotFound)
+		log.Print("Get 1")
+	}
+
+	{
 		var rep int
 		var err error
 		hash2, rep, err = kc.PutB([]byte("foo"))

-----------------------------------------------------------------------


hooks/post-receive
--