Bugfix: add missing return statements in GRAPHQL and UrlEncodedForm t…

…ransports. (#2625)

Two transports (GRAPHQL and UrlEncodedForm) did not have return
statement at the end of `if err` block. Instead of returning
a 'could not cleanup body' error, we continued processing.

User still got an error. But instead of early 'could not cleanup'
error, user gor 'Internal system error' which happened a few
lines after the if block.

Tests are added.

graphql/handler/transport/http_form_urlencode_test.go

@@ -48,6 +48,13 @@ func TestUrlEncodedForm(t *testing.T) {
 		assert.Equal(t, `{"errors":[{"message":"Expected Name, found \u003cInvalid\u003e","locations":[{"line":1,"column":6}],"extensions":{"code":"GRAPHQL_PARSE_FAILED"}}],"data":null}`, resp.Body.String())
 	})
 
+	t.Run("parse query failure", func(t *testing.T) {
+		resp := doRequest(h, "POST", "/graphql", `{"query":{"wrong": "format"}}`, "application/x-www-form-urlencoded")
+		assert.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
+		assert.Equal(t, resp.Header().Get("Content-Type"), "application/json")
+		assert.Equal(t, resp.Body.String(), `{"errors":[{"message":"could not cleanup body: json: cannot unmarshal object into Go struct field RawParams.query of type string"}],"data":null}`)
+	})
+
 	t.Run("validate content type", func(t *testing.T) {
 		doReq := func(handler http.Handler, method string, target string, body string, contentType string) *httptest.ResponseRecorder {
 			r := httptest.NewRequest(method, target, strings.NewReader(body))

graphql/handler/transport/http_form_urlencoded.go

@@ -2,7 +2,6 @@ package transport
 
 import (
 	"io"
-	"log"
 	"mime"
 	"net/http"
 	"net/url"
@@ -48,18 +47,20 @@ func (h UrlEncodedForm) Do(w http.ResponseWriter, r *http.Request, exec graphql.
 
 	bodyString, err := getRequestBody(r)
 	if err != nil {
+		w.WriteHeader(http.StatusBadRequest)
 		gqlErr := gqlerror.Errorf("could not get form body: %+v", err)
 		resp := exec.DispatchError(ctx, gqlerror.List{gqlErr})
-		log.Printf("could not get json request body: %+v", err.Error())
 		writeJson(w, resp)
+		return
 	}
 
 	params, err = h.parseBody(bodyString)
 	if err != nil {
+		w.WriteHeader(http.StatusUnprocessableEntity)
 		gqlErr := gqlerror.Errorf("could not cleanup body: %+v", err)
 		resp := exec.DispatchError(ctx, gqlerror.List{gqlErr})
-		log.Printf("could not cleanup body: %+v", err.Error())
 		writeJson(w, resp)
+		return
 	}
 
 	rc, OpErr := exec.CreateOperationContext(ctx, params)

graphql/handler/transport/http_graphql.go

@@ -1,7 +1,6 @@
 package transport
 
 import (
-	"log"
 	"mime"
 	"net/http"
 	"net/url"
@@ -52,16 +51,17 @@ func (h GRAPHQL) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphEx
 	if err != nil {
 		gqlErr := gqlerror.Errorf("could not get request body: %+v", err)
 		resp := exec.DispatchError(ctx, gqlerror.List{gqlErr})
-		log.Printf("could not get request body: %+v", err.Error())
 		writeJson(w, resp)
+		return
 	}
 
 	params.Query, err = cleanupBody(bodyString)
 	if err != nil {
+		w.WriteHeader(http.StatusUnprocessableEntity)
 		gqlErr := gqlerror.Errorf("could not cleanup body: %+v", err)
 		resp := exec.DispatchError(ctx, gqlerror.List{gqlErr})
-		log.Printf("could not cleanup body: %+v", err.Error())
 		writeJson(w, resp)
+		return
 	}
 
 	rc, OpErr := exec.CreateOperationContext(ctx, params)

graphql/handler/transport/http_graphql_test.go

@@ -35,6 +35,13 @@ func TestGRAPHQL(t *testing.T) {
 		assert.Equal(t, `{"errors":[{"message":"Expected Name, found String","locations":[{"line":1,"column":3}],"extensions":{"code":"GRAPHQL_PARSE_FAILED"}}],"data":null}`, resp.Body.String())
 	})
 
+	t.Run("parse query failure", func(t *testing.T) {
+		resp := doGraphqlRequest(h, "POST", "/graphql", `%7B%H7U6Z`)
+		assert.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())
+		assert.Equal(t, resp.Header().Get("Content-Type"), "application/json")
+		assert.Equal(t, resp.Body.String(), `{"errors":[{"message":"could not cleanup body: invalid URL escape \"%H7\""}],"data":null}`)
+	})
+
 	t.Run("validation failure", func(t *testing.T) {
 		resp := doGraphqlRequest(h, "POST", "/graphql", `{ title }`)
 		assert.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String())