From 5559c86b1a6bba1d3a1dbdb633ac377f54ed14b0 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curoverse.com>
Date: Wed, 29 Jun 2016 13:41:57 -0400
Subject: [PATCH 1/1] 9513: Do not set response Content-Length to -1 when
 returning 411 Length Required.

Add tests for requests with bad/missing Content-Length headers.
---
 services/keepproxy/keepproxy.go      | 13 ++-------
 services/keepproxy/keepproxy_test.go | 41 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index df7528ed42..226c388ae2 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -367,7 +367,7 @@ func (this PutBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 
 	kc := *this.KeepClient
 	var err error
-	var expectLength int64 = -1
+	var expectLength int64
 	var status = http.StatusInternalServerError
 	var wroteReplicas int
 	var locatorOut string = "-"
@@ -381,15 +381,8 @@ func (this PutBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 
 	locatorIn := mux.Vars(req)["locator"]
 
-	if req.Header.Get("Content-Length") != "" {
-		_, err := fmt.Sscanf(req.Header.Get("Content-Length"), "%d", &expectLength)
-		if err != nil {
-			resp.Header().Set("Content-Length", fmt.Sprintf("%d", expectLength))
-		}
-
-	}
-
-	if expectLength < 0 {
+	_, err = fmt.Sscanf(req.Header.Get("Content-Length"), "%d", &expectLength)
+	if err != nil || expectLength < 0 {
 		err = LengthRequiredError
 		status = http.StatusLengthRequired
 		return
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index a3ca5f1156..e6c129806f 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bytes"
 	"crypto/md5"
 	"fmt"
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
@@ -9,6 +10,7 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
+	"net/http/httptest"
 	"os"
 	"strings"
 	"testing"
@@ -126,6 +128,45 @@ func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
 	}
 }
 
+func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
+	kc := runProxy(c, nil, false)
+	defer closeListener()
+
+	content := []byte("TestPutWrongContentLength")
+	hash := fmt.Sprintf("%x", md5.Sum(content))
+
+	// If we use http.Client to send these requests to the network
+	// server we just started, the Go http library automatically
+	// fixes the invalid Content-Length header. In order to test
+	// our server behavior, we have to call the handler directly
+	// using an httptest.ResponseRecorder.
+	rtr := MakeRESTRouter(true, true, kc)
+
+	type testcase struct {
+		sendLength   string
+		expectStatus int
+	}
+
+	for _, t := range []testcase{
+		{"1", http.StatusBadRequest},
+		{"", http.StatusLengthRequired},
+		{"-1", http.StatusLengthRequired},
+		{"abcdef", http.StatusLengthRequired},
+	} {
+		req, err := http.NewRequest("PUT",
+			fmt.Sprintf("http://%s/%s+%d", listener.Addr().String(), hash, len(content)),
+			bytes.NewReader(content))
+		c.Assert(err, IsNil)
+		req.Header.Set("Content-Length", t.sendLength)
+		req.Header.Set("Authorization", "OAuth2 4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h")
+		req.Header.Set("Content-Type", "application/octet-stream")
+
+		resp := httptest.NewRecorder()
+		rtr.ServeHTTP(resp, req)
+		c.Check(resp.Code, Equals, t.expectStatus)
+	}
+}
+
 func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
 	kc := runProxy(c, nil, false)
 	defer closeListener()
-- 
2.30.2