From 7852f0bac34b7bbc0a16bd7b2ebfabc07a209671 Mon Sep 17 00:00:00 2001
From: "Steven E. Harris" <seh@panix.com>
Date: Tue, 7 Apr 2020 15:36:46 -0400
Subject: [PATCH] Respond to empty RPC requests with HTTP OK status

When receiving an HTTP GET request with no content and no URL query,
we assume the client may be a health check probe, and process the
request without complaint. We had been omitting the HTTP status code
in our otherwise empty response, though, which aggravates some HTTP
clients. Instead, respond with HTTP status code 200, indicating the
request was accepted and processed successfully.
---
 rpc/http.go      |  1 +
 rpc/http_test.go | 66 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/rpc/http.go b/rpc/http.go
index b3ce0a5b5e..78fbd9f8f3 100644
--- a/rpc/http.go
+++ b/rpc/http.go
@@ -208,6 +208,7 @@ func (t *httpServerConn) SetWriteDeadline(time.Time) error { return nil }
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// Permit dumb empty requests for remote health-checks (AWS)
 	if r.Method == http.MethodGet && r.ContentLength == 0 && r.URL.RawQuery == "" {
+		w.WriteHeader(http.StatusOK)
 		return
 	}
 	if code, err := validateRequest(r); err != nil {
diff --git a/rpc/http_test.go b/rpc/http_test.go
index b3f694d8af..fc939ae48f 100644
--- a/rpc/http_test.go
+++ b/rpc/http_test.go
@@ -23,32 +23,78 @@ import (
 	"testing"
 )
 
+func confirmStatusCode(t *testing.T, got, want int) {
+	t.Helper()
+	if got == want {
+		return
+	}
+	if gotName := http.StatusText(got); len(gotName) > 0 {
+		if wantName := http.StatusText(want); len(wantName) > 0 {
+			t.Fatalf("response status code: got %d (%s), want %d (%s)", got, gotName, want, wantName)
+		}
+	}
+	t.Fatalf("response status code: got %d, want %d", got, want)
+}
+
+func confirmRequestValidationCode(t *testing.T, method, contentType, body string, expectedStatusCode int) {
+	t.Helper()
+	request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body))
+	if len(contentType) > 0 {
+		request.Header.Set("Content-Type", contentType)
+	}
+	code, err := validateRequest(request)
+	if code == 0 {
+		if err != nil {
+			t.Errorf("validation: got error %v, expected nil", err)
+		}
+	} else if err == nil {
+		t.Errorf("validation: code %d: got nil, expected error", code)
+	}
+	confirmStatusCode(t, code, expectedStatusCode)
+}
+
 func TestHTTPErrorResponseWithDelete(t *testing.T) {
-	testHTTPErrorResponse(t, http.MethodDelete, contentType, "", http.StatusMethodNotAllowed)
+	confirmRequestValidationCode(t, http.MethodDelete, contentType, "", http.StatusMethodNotAllowed)
 }
 
 func TestHTTPErrorResponseWithPut(t *testing.T) {
-	testHTTPErrorResponse(t, http.MethodPut, contentType, "", http.StatusMethodNotAllowed)
+	confirmRequestValidationCode(t, http.MethodPut, contentType, "", http.StatusMethodNotAllowed)
 }
 
 func TestHTTPErrorResponseWithMaxContentLength(t *testing.T) {
 	body := make([]rune, maxRequestContentLength+1)
-	testHTTPErrorResponse(t,
+	confirmRequestValidationCode(t,
 		http.MethodPost, contentType, string(body), http.StatusRequestEntityTooLarge)
 }
 
 func TestHTTPErrorResponseWithEmptyContentType(t *testing.T) {
-	testHTTPErrorResponse(t, http.MethodPost, "", "", http.StatusUnsupportedMediaType)
+	confirmRequestValidationCode(t, http.MethodPost, "", "", http.StatusUnsupportedMediaType)
 }
 
 func TestHTTPErrorResponseWithValidRequest(t *testing.T) {
-	testHTTPErrorResponse(t, http.MethodPost, contentType, "", 0)
+	confirmRequestValidationCode(t, http.MethodPost, contentType, "", 0)
 }
 
-func testHTTPErrorResponse(t *testing.T, method, contentType, body string, expected int) {
-	request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body))
-	request.Header.Set("content-type", contentType)
-	if code, _ := validateRequest(request); code != expected {
-		t.Fatalf("response code should be %d not %d", expected, code)
+func confirmHTTPRequestYieldsStatusCode(t *testing.T, method, contentType, body string, expectedStatusCode int) {
+	t.Helper()
+	s := Server{}
+	ts := httptest.NewServer(&s)
+	defer ts.Close()
+
+	request, err := http.NewRequest(method, ts.URL, strings.NewReader(body))
+	if err != nil {
+		t.Fatalf("failed to create a valid HTTP request: %v", err)
 	}
+	if len(contentType) > 0 {
+		request.Header.Set("Content-Type", contentType)
+	}
+	resp, err := http.DefaultClient.Do(request)
+	if err != nil {
+		t.Fatalf("request failed: %v", err)
+	}
+	confirmStatusCode(t, resp.StatusCode, expectedStatusCode)
+}
+
+func TestHTTPResponseWithEmptyGet(t *testing.T) {
+	confirmHTTPRequestYieldsStatusCode(t, http.MethodGet, "", "", http.StatusOK)
 }