diff --git a/rpc/http.go b/rpc/http.go index 2ac9f6c371..68634e3fdf 100644 --- a/rpc/http.go +++ b/rpc/http.go @@ -33,6 +33,7 @@ import ( ) const ( + contentType = "application/json" maxHTTPRequestContentLength = 1024 * 128 ) @@ -69,8 +70,8 @@ func DialHTTP(endpoint string) (*Client, error) { if err != nil { return nil, err } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", contentType) + req.Header.Set("Accept", contentType) initctx := context.Background() return newClient(initctx, func(context.Context) (net.Conn, error) { @@ -150,21 +151,11 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.Method == "GET" && r.ContentLength == 0 && r.URL.RawQuery == "" { return } - // For meaningful requests, validate it's size and content type - if r.ContentLength > maxHTTPRequestContentLength { - http.Error(w, - fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength), - http.StatusRequestEntityTooLarge) - return - } - ct := r.Header.Get("content-type") - mt, _, err := mime.ParseMediaType(ct) - if err != nil || mt != "application/json" { - http.Error(w, - "invalid content type, only application/json is supported", - http.StatusUnsupportedMediaType) + if responseCode, errorMessage := httpErrorResponse(r); responseCode != 0 { + http.Error(w, errorMessage, responseCode) return } + // All checks passed, create a codec that reads direct from the request body // untilEOF and writes the response to w and order the server to process a // single request. @@ -175,6 +166,28 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { srv.ServeSingleRequest(codec, OptionMethodInvocation) } +// Returns a non-zero response code and error message if the request is invalid. +func httpErrorResponse(r *http.Request) (int, string) { + if r.Method == "PUT" || r.Method == "DELETE" { + errorMessage := "method not allowed" + return http.StatusMethodNotAllowed, errorMessage + } + + if r.ContentLength > maxHTTPRequestContentLength { + errorMessage := fmt.Sprintf("content length too large (%d>%d)", r.ContentLength, maxHTTPRequestContentLength) + return http.StatusRequestEntityTooLarge, errorMessage + } + + ct := r.Header.Get("content-type") + mt, _, err := mime.ParseMediaType(ct) + if err != nil || mt != contentType { + errorMessage := fmt.Sprintf("invalid content type, only %s is supported", contentType) + return http.StatusUnsupportedMediaType, errorMessage + } + + return 0, "" +} + func newCorsHandler(srv *Server, allowedOrigins []string) http.Handler { // disable CORS support if user has not specified a custom CORS configuration if len(allowedOrigins) == 0 { diff --git a/rpc/http_test.go b/rpc/http_test.go new file mode 100644 index 0000000000..f4afd52167 --- /dev/null +++ b/rpc/http_test.go @@ -0,0 +1,40 @@ +package rpc + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestHTTPErrorResponseWithDelete(t *testing.T) { + httpErrorResponseTest(t, "DELETE", contentType, "", http.StatusMethodNotAllowed) +} + +func TestHTTPErrorResponseWithPut(t *testing.T) { + httpErrorResponseTest(t, "PUT", contentType, "", http.StatusMethodNotAllowed) +} + +func TestHTTPErrorResponseWithMaxContentLength(t *testing.T) { + body := make([]rune, maxHTTPRequestContentLength+1, maxHTTPRequestContentLength+1) + httpErrorResponseTest(t, + "POST", contentType, string(body), http.StatusRequestEntityTooLarge) +} + +func TestHTTPErrorResponseWithEmptyContentType(t *testing.T) { + httpErrorResponseTest(t, "POST", "", "", http.StatusUnsupportedMediaType) +} + +func TestHTTPErrorResponseWithValidRequest(t *testing.T) { + httpErrorResponseTest(t, "POST", contentType, "", 0) +} + +func httpErrorResponseTest(t *testing.T, + method, contentType, body string, expectedResponse int) { + + request := httptest.NewRequest(method, "http://url.com", strings.NewReader(body)) + request.Header.Set("content-type", contentType) + if response, _ := httpErrorResponse(request); response != expectedResponse { + t.Fatalf("response code should be %d not %d", expectedResponse, response) + } +}