diff --git a/router-tests/graphql_over_http_test.go b/router-tests/graphql_over_http_test.go new file mode 100644 index 0000000000..9033041442 --- /dev/null +++ b/router-tests/graphql_over_http_test.go @@ -0,0 +1,109 @@ +package integration + +import ( + "bytes" + "io" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + "github.com/wundergraph/cosmo/router-tests/testenv" +) + +func TestGraphQLOverHTTPCompatibility(t *testing.T) { + t.Parallel() + + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + t.Run("valid request should return 200 OK with valid response", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{"query":"query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}","variables":{"criteria":{"nationality":"GERMAN"}}}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"data":{"findEmployees":[{"id":1,"details":{"forename":"Jens","surname":"Neuse"}},{"id":2,"details":{"forename":"Dustin","surname":"Deus"}},{"id":4,"details":{"forename":"Björn","surname":"Schwenzer"}},{"id":11,"details":{"forename":"Alexandra","surname":"Neuse"}}]}}`, string(data)) + }) + t.Run("malformed JSON should return 400", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{"query":query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}","variables":{"criteria":{"nationality":"GERMAN"}}}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"error parsing request body"}],"data":null}`, string(data)) + }) + t.Run("malformed JSON variant should return 400", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{{"query":query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}","variables":{"criteria":{"nationality":"GERMAN"}}}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"error parsing request body"}],"data":null}`, string(data)) + }) + t.Run("malformed JSON variant #2 should return 400", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"error parsing request body"}],"data":null}`, string(data)) + }) + t.Run("malformed JSON variables variant should return 400", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{"query":"query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}","variables":{"criteria":{"nationality":GERMAN}}}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"error parsing request body"}],"data":null}`, string(data)) + }) + t.Run("missing variables should return 200 OK with validation errors response", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{"query":"query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}"}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"Variable \"$criteria\" of required type \"SearchInput!\" was not provided."}],"data":null}`, string(data)) + }) + t.Run("mismatching variables although valid JSON should not be 400", func(t *testing.T) { + header := http.Header{ + "Content-Type": []string{"application/json"}, + "Accept": []string{"application/json"}, + } + body := []byte(`{"query":"query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}","variables":{"whatever":123}`) + res, err := xEnv.MakeRequest("POST", "/graphql", header, bytes.NewReader(body)) + require.NoError(t, err) + require.Equal(t, http.StatusOK, res.StatusCode) + data, err := io.ReadAll(res.Body) + require.NoError(t, err) + require.Equal(t, `{"errors":[{"message":"Variable \"$criteria\" of required type \"SearchInput!\" was not provided."}],"data":null}`, string(data)) + }) + }) +} diff --git a/router-tests/integration_test.go b/router-tests/integration_test.go index 6d28fcb9b3..25fffdca7f 100644 --- a/router-tests/integration_test.go +++ b/router-tests/integration_test.go @@ -236,7 +236,7 @@ func TestVariables(t *testing.T) { Variables: json.RawMessage(`{}`), }) require.NoError(t, err) - require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, http.StatusOK, res.Response.StatusCode) require.Equal(t, `{"errors":[{"message":"Variable \"$criteria\" of required type \"SearchInput!\" was not provided."}],"data":null}`, res.Body) }) @@ -246,7 +246,7 @@ func TestVariables(t *testing.T) { Variables: json.RawMessage(`{"criteria":1}`), }) require.NoError(t, err) - require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, http.StatusOK, res.Response.StatusCode) require.Equal(t, `{"errors":[{"message":"Variable \"$criteria\" got invalid value 1; Expected type \"SearchInput\" to be an object."}],"data":null}`, res.Body) }) }) diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index a2c68c4951..d62ad4c05d 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -194,8 +194,6 @@ func (h *PreHandler) Handler(next http.Handler) http.Handler { ) operationKit, err := h.operationProcessor.NewKit(body) - defer operationKit.Free() - if err != nil { finalErr = err @@ -208,6 +206,7 @@ func (h *PreHandler) Handler(next http.Handler) http.Handler { writeOperationError(r, w, requestLogger, err) return } + defer operationKit.Free() err = operationKit.Parse(r.Context(), clientInfo, requestLogger) if err != nil { diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index a617294091..0a1195f2de 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -12,6 +12,7 @@ import ( "github.com/buger/jsonparser" "github.com/cespare/xxhash/v2" "github.com/pkg/errors" + "github.com/tidwall/gjson" "github.com/wundergraph/graphql-go-tools/v2/pkg/lexer/literal" "github.com/wundergraph/graphql-go-tools/v2/pkg/variablesvalidation" @@ -189,6 +190,13 @@ func (o *OperationKit) Parse(ctx context.Context, clientInfo *ClientInfo, log *z parseErr = invalidExtensionsTypeError(valueType) return } + if !gjson.ValidBytes(value) { + parseErr = &inputError{ + message: "error parsing request body", + statusCode: http.StatusBadRequest, + } + return + } requestExtensions = value persistedQuery, _, _, err := jsonparser.Get(value, "persistedQuery") if err != nil { @@ -252,7 +260,10 @@ func (o *OperationKit) Parse(ctx context.Context, clientInfo *ClientInfo, log *z } if parseErr != nil { - return errors.WithStack(parseErr) + return &inputError{ + message: "error parsing request body", + statusCode: http.StatusBadRequest, + } } if len(persistedQuerySha256Hash) > 0 { @@ -277,6 +288,20 @@ func (o *OperationKit) Parse(ctx context.Context, clientInfo *ClientInfo, log *z requestOperationNameBytes = nil } + if requestDocumentBytes == nil { + return &inputError{ + message: "error parsing request body", + statusCode: http.StatusBadRequest, + } + } + + if requestVariableBytes != nil && !gjson.ValidBytes(requestVariableBytes) { + return &inputError{ + message: "error parsing request body", + statusCode: http.StatusBadRequest, + } + } + report := &operationreport.Report{} o.kit.doc.Input.ResetInputBytes(requestDocumentBytes) o.kit.parser.Parse(o.kit.doc, report) @@ -432,7 +457,7 @@ func (o *OperationKit) Validate() error { if err != nil { return &inputError{ message: err.Error(), - statusCode: http.StatusBadRequest, + statusCode: http.StatusOK, } } diff --git a/router/core/operation_processor_test.go b/router/core/operation_processor_test.go index 476d671c86..d4ac24e43a 100644 --- a/router/core/operation_processor_test.go +++ b/router/core/operation_processor_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" - "github.com/buger/jsonparser" "github.com/stretchr/testify/assert" "github.com/wundergraph/graphql-go-tools/v2/pkg/engine/plan" "go.uber.org/zap" @@ -177,21 +176,20 @@ func TestOperationParserExtensions(t *testing.T) { } log := zap.NewNop() testCases := []struct { - Input string - ValueType jsonparser.ValueType - Valid bool + Input string + Valid bool }{ { - Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":"this_is_not_valid"}`, - ValueType: jsonparser.String, + Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":"this_is_not_valid"}`, }, { - Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":42}`, - ValueType: jsonparser.Number, + Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":42}`, }, { - Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":true}`, - ValueType: jsonparser.Boolean, + Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":true}`, + }, + { + Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":{"foo":bar}}`, }, { Input: `{"query":"subscription { initialPayload(repeat:3) }","extensions":{}}`, @@ -219,8 +217,6 @@ func TestOperationParserExtensions(t *testing.T) { assert.False(t, isInputError, "expected invalid extensions to not return an input error, got %s", err) } else { assert.True(t, isInputError, "expected invalid extensions to return an input error, got %s", err) - assert.Contains(t, err.Error(), "extensions", "expected error to contain extensions") - assert.Contains(t, err.Error(), tc.ValueType.String(), "expected error to contain value type name") } }) }