Merge pull request letsencrypt#1321 from letsencrypt/correct_head_con…

…tent_length

correct Content-Length on HEAD

wfe/web-front-end.go

@@ -107,16 +107,6 @@ func NewWebFrontEndImpl(stats statsd.Statter, clk clock.Clock) (WebFrontEndImpl,
 	}, nil
 }
 
-// BodylessResponseWriter wraps http.ResponseWriter, discarding
-// anything written to the body.
-type BodylessResponseWriter struct {
-	http.ResponseWriter
-}
-
-func (mrw BodylessResponseWriter) Write(buf []byte) (int, error) {
-	return len(buf), nil
-}
-
 // HandleFunc registers a handler at the given path. It's
 // http.HandleFunc(), but with a wrapper around the handler that
 // provides some generic per-request functionality:
@@ -157,10 +147,9 @@ func (wfe *WebFrontEndImpl) HandleFunc(mux *http.ServeMux, pattern string, h wfe
 
 			switch request.Method {
 			case "HEAD":
-				// Whether or not we're sending a 405 error,
-				// we should comply with HTTP spec by not
-				// sending a body.
-				response = BodylessResponseWriter{response}
+				// Go's net/http (and httptest) servers will strip our the body
+				// of responses for us. This keeps the Content-Length for HEAD
+				// requests as the same as GET requests per the spec.
 			case "OPTIONS":
 				wfe.Options(response, request, methodsStr, methodsMap)
 				return

wfe/web-front-end_test.go

@@ -19,6 +19,7 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"sort"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -327,7 +328,7 @@ func TestHandleFunc(t *testing.T) {
 	test.AssertEquals(t, rw.Code, http.StatusMethodNotAllowed)
 	test.AssertEquals(t, rw.Header().Get("Content-Type"), "application/problem+json")
 	test.AssertEquals(t, rw.Header().Get("Allow"), "POST")
-	test.AssertEquals(t, rw.Body.String(), "")
+	test.AssertEquals(t, rw.Body.String(), `{"type":"urn:acme:error:malformed","detail":"Method not allowed","status":405}`)
 
 	wfe.AllowOrigins = []string{"*"}
 	testOrigin := "https://example.com"
@@ -1400,6 +1401,34 @@ func TestBadKeyCSR(t *testing.T) {
 		`{"type":"urn:acme:error:malformed","detail":"Invalid key in certificate request :: Key too small: 512","status":400}`)
 }
 
+// This uses httptest.NewServer because ServeMux.ServeHTTP won't prevent the
+// body from being sent like the net/http Server's actually do.
+func TestGetCertificateHEADHasCorrectBodyLength(t *testing.T) {
+	wfe, _ := setupWFE(t)
+
+	certPemBytes, _ := ioutil.ReadFile("test/178.crt")
+	certBlock, _ := pem.Decode(certPemBytes)
+
+	mockLog := wfe.log.SyslogWriter.(*mocks.SyslogWriter)
+	mockLog.Clear()
+
+	mux, _ := wfe.Handler()
+	s := httptest.NewServer(mux)
+	req, _ := http.NewRequest("HEAD", s.URL+"/acme/cert/0000000000000000000000000000000000b2", nil)
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		test.AssertNotError(t, err, "do error")
+	}
+	body, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		test.AssertNotEquals(t, err, "readall error")
+	}
+	defer resp.Body.Close()
+	test.AssertEquals(t, resp.StatusCode, 200)
+	test.AssertEquals(t, strconv.Itoa(len(certBlock.Bytes)), resp.Header.Get("Content-Length"))
+	test.AssertEquals(t, 0, len(body))
+}
+
 func newRequestEvent() *requestEvent {
 	return &requestEvent{Extra: make(map[string]interface{})}
 }