From 9887edd580d161d22d284e8778b090016bd24f80 Mon Sep 17 00:00:00 2001 From: "Steven E. Harris" Date: Tue, 28 Apr 2020 04:43:21 -0400 Subject: [PATCH] rpc: add explicit 200 response for empty HTTP GET (#20952) --- 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 b3ce0a5b5..78fbd9f8f 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 b3f694d8a..fc939ae48 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) }