diff --git a/router-tests/automatic_persisted_queries_test.go b/router-tests/automatic_persisted_queries_test.go index 4f846aa5e0..9ba4ebc5b3 100644 --- a/router-tests/automatic_persisted_queries_test.go +++ b/router-tests/automatic_persisted_queries_test.go @@ -35,7 +35,7 @@ func TestAutomaticPersistedQueries(t *testing.T) { }, }, func(t *testing.T, xEnv *testenv.Environment) { res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), }) require.Equal(t, `{"errors":[{"message":"PersistedQueryNotFound","extensions":{"code":"PERSISTED_QUERY_NOT_FOUND"}}]}`, res.Body) }) @@ -198,7 +198,7 @@ func TestAutomaticPersistedQueries(t *testing.T) { }, }, func(t *testing.T, xEnv *testenv.Environment) { res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), }) require.Equal(t, `{"errors":[{"message":"PersistedQueryNotFound","extensions":{"code":"PERSISTED_QUERY_NOT_FOUND"}}]}`, res.Body) }) diff --git a/router-tests/batch_test.go b/router-tests/batch_test.go index 0a09487e4d..aa3a7527a8 100644 --- a/router-tests/batch_test.go +++ b/router-tests/batch_test.go @@ -78,7 +78,7 @@ func TestBatch(t *testing.T) { }, nil) require.NoError(t, err) require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) - require.JSONEq(t, `{"errors":[{"message":"error parsing request body"}]}`, res.Body) + require.JSONEq(t, `{"errors":[{"message":"invalid request body: json: cannot unmarshal array into Go value of type core.GraphQLRequest"}]}`, res.Body) }, ) }) diff --git a/router-tests/graphql_over_get_test.go b/router-tests/graphql_over_get_test.go index 871ee6bc94..38b84eab65 100644 --- a/router-tests/graphql_over_get_test.go +++ b/router-tests/graphql_over_get_test.go @@ -46,6 +46,50 @@ func TestOperationsOverGET(t *testing.T) { }) }) + t.Run("Invalid GET variables", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + OperationName: []byte(`Find`), + Query: `query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}`, + Variables: []byte(`{"criteria":{ "nationality":GERMAN} } `), + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, `{"errors":[{"message":"invalid GET request: error parsing variables: invalid character 'G' looking for beginning of value"}]}`, res.Body) + }) + }) + + t.Run("Invalid GET extensions from bool", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + OperationName: []byte(`Find`), + Query: `query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}`, + Variables: []byte(`{"criteria":{ "nationality":"GERMAN"} } `), + Extensions: []byte(`true`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, `{"errors":[{"message":"invalid GET request: error parsing extensions: json: cannot unmarshal bool"}]}`, res.Body) + }) + }) + + t.Run("Invalid GET extensions version from bool", func(t *testing.T) { + t.Parallel() + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + OperationName: []byte(`Find`), + Query: `query Find($criteria: SearchInput!) {findEmployees(criteria: $criteria){id details {forename surname}}}`, + Variables: []byte(`{"criteria":{ "nationality":"GERMAN"} } `), + Extensions: []byte(`{"persistedQuery": {"version": true, "sha256Hash": "` + cacheHashNotStored + `"}}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, `{"errors":[{"message":"invalid GET request: error parsing extensions: json: cannot unmarshal bool"}]}`, res.Body) + }) + }) + t.Run("Only queries are supported over GET", func(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { diff --git a/router-tests/graphql_over_http_test.go b/router-tests/graphql_over_http_test.go index 899a0651d4..120e8a4281 100644 --- a/router-tests/graphql_over_http_test.go +++ b/router-tests/graphql_over_http_test.go @@ -60,7 +60,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"invalid request body: variables must be a JSON object"}]}`, string(data)) }) t.Run("return 400 bad request when extensions is not a map", func(t *testing.T) { header := http.Header{ @@ -74,7 +74,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"invalid request body: error parsing extensions: json: cannot unmarshal bool"}]}`, string(data)) }) t.Run("valid request with Operation Name should return 200 OK with valid response", func(t *testing.T) { header := http.Header{ @@ -101,7 +101,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"invalid request body: invalid character 'q' looking for beginning of value"}]}`, string(data)) }) t.Run("malformed JSON variant should return 400", func(t *testing.T) { header := http.Header{ @@ -115,7 +115,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"invalid request body: invalid character '{' looking for beginning of object key string"}]}`, string(data)) }) t.Run("malformed JSON variant #2 should return 400", func(t *testing.T) { header := http.Header{ @@ -129,7 +129,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"empty request body"}]}`, string(data)) }) t.Run("malformed JSON variables variant should return 400", func(t *testing.T) { header := http.Header{ @@ -143,7 +143,7 @@ func TestGraphQLOverHTTPCompatibility(t *testing.T) { require.Equal(t, res.Header.Get("Content-Type"), "application/json; charset=utf-8") data, err := io.ReadAll(res.Body) require.NoError(t, err) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, string(data)) + require.Equal(t, `{"errors":[{"message":"invalid request body: invalid character 'G' looking for beginning of value"}]}`, string(data)) }) t.Run("missing variables should return 200 OK with validation errors response", func(t *testing.T) { header := http.Header{ diff --git a/router-tests/header_propagation_test.go b/router-tests/header_propagation_test.go index 0e16772328..fdf9ddcc02 100644 --- a/router-tests/header_propagation_test.go +++ b/router-tests/header_propagation_test.go @@ -48,7 +48,7 @@ func TestCacheControl(t *testing.T) { res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ Query: "", // Empty query Variables: json.RawMessage(`{}`), - Extensions: json.RawMessage(`{"persistedQuery": {"version": 1, "sha256Hash": "invalid-hash"}}`), + Extensions: json.RawMessage(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), }) require.Contains(t, res.Body, "PERSISTED_QUERY_NOT_FOUND") diff --git a/router-tests/integration_test.go b/router-tests/integration_test.go index 53272736e2..66201d696f 100644 --- a/router-tests/integration_test.go +++ b/router-tests/integration_test.go @@ -532,7 +532,7 @@ func TestVariables(t *testing.T) { }) require.NoError(t, err) require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, res.Body) + require.Equal(t, `{"errors":[{"message":"invalid request body: variables must be a JSON object"}]}`, res.Body) }) t.Run("invalid string", func(t *testing.T) { @@ -542,7 +542,7 @@ func TestVariables(t *testing.T) { }) require.NoError(t, err) require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, res.Body) + require.Equal(t, `{"errors":[{"message":"invalid request body: variables must be a JSON object"}]}`, res.Body) }) t.Run("invalid boolean", func(t *testing.T) { @@ -552,7 +552,7 @@ func TestVariables(t *testing.T) { }) require.NoError(t, err) require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, res.Body) + require.Equal(t, `{"errors":[{"message":"invalid request body: variables must be a JSON object"}]}`, res.Body) }) t.Run("invalid array", func(t *testing.T) { @@ -562,7 +562,7 @@ func TestVariables(t *testing.T) { }) require.NoError(t, err) require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) - require.Equal(t, `{"errors":[{"message":"error parsing request body"}]}`, res.Body) + require.Equal(t, `{"errors":[{"message":"invalid request body: variables must be a JSON object"}]}`, res.Body) }) t.Run("missing", func(t *testing.T) { diff --git a/router-tests/persisted_operations_over_get_test.go b/router-tests/persisted_operations_over_get_test.go index 836ff00beb..9451ee5748 100644 --- a/router-tests/persisted_operations_over_get_test.go +++ b/router-tests/persisted_operations_over_get_test.go @@ -23,7 +23,7 @@ func TestPersistedOperationOverGET(t *testing.T) { header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), Header: header, }) require.NoError(t, err) @@ -99,7 +99,7 @@ func TestAutomatedPersistedQueriesOverGET(t *testing.T) { header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), Header: header, }) require.NoError(t, err) diff --git a/router-tests/persisted_operations_test.go b/router-tests/persisted_operations_test.go index 85220ef676..3f236cd5b1 100644 --- a/router-tests/persisted_operations_test.go +++ b/router-tests/persisted_operations_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "io" "net/http" + "strings" "testing" "github.com/buger/jsonparser" @@ -16,18 +17,57 @@ import ( "github.com/wundergraph/cosmo/router/pkg/config" ) +// cacheHashNotStored is a constant representing a non-existent entry in Persisted Queries Cache +// of the testenv. All the pre-stored persistent entries used in testing are located here: +// ./testenv/testdata/cdn/organization/graph/operations/my-client +const cacheHashNotStored = "0000000000000000000000000000000000000000000000000000000000000000" + func TestPersistedOperationNotFound(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}`), }) require.Equal(t, res.Response.Header.Get("Content-Type"), "application/json; charset=utf-8") require.Equal(t, `{"errors":[{"message":"PersistedQueryNotFound","extensions":{"code":"PERSISTED_QUERY_NOT_FOUND"}}]}`, res.Body) }) } +func TestPersistedOperationInvalidHash(t *testing.T) { + t.Parallel() + + malformed := []string{ + "malformed", + "some-words-free-style-not-allowed", + "../../../path/not/ok", + // too short + strings.Repeat("0", 63), + strings.Repeat("0", 62), + "0000", + "0", + "", + // too long + strings.Repeat("0", 74), + strings.Repeat("0", 65), + // 1 bad character spoils everything + "0000000000000z00000000000000000000000000000000000000000000000000", + "0000000000000000000000000000000000000z00000000000000000000000000", + "000000000000000000000000000000000000000000000000000000000000000z", + } + for _, hash := range malformed { + testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + hash + `"}}`), + }) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, res.Response.StatusCode) + require.Equal(t, res.Response.Header.Get("Content-Type"), "application/json; charset=utf-8") + require.Equal(t, `{"errors":[{"message":"invalid request body: persistedQuery does not have a valid sha256 hash"}]}`, res.Body) + }) + } +} + func TestPersistedOperation(t *testing.T) { t.Parallel() @@ -440,11 +480,12 @@ func TestPersistedOperationsWithNestedVariablesExtraction(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + nestedVariableExtraction := "5000000000000000000000000000000000000000000000000000000000000000" header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"NormalizationQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "nestedVariableExtraction"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + nestedVariableExtraction + `"}}`), Header: header, Variables: []byte(`{"arg":"a"}`), }) @@ -453,7 +494,7 @@ func TestPersistedOperationsWithNestedVariablesExtraction(t *testing.T) { require.Equal(t, `{"data":{"rootFieldWithListOfInputArg":[{"arg":"a"}]}}`, res.Body) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"NormalizationQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "nestedVariableExtraction"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + nestedVariableExtraction + `"}}`), Header: header, Variables: []byte(`{"arg":"a"}`), }) @@ -462,7 +503,7 @@ func TestPersistedOperationsWithNestedVariablesExtraction(t *testing.T) { require.Equal(t, `{"data":{"rootFieldWithListOfInputArg":[{"arg":"a"}]}}`, res.Body) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"NormalizationQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "nestedVariableExtraction"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + nestedVariableExtraction + `"}}`), Header: header, Variables: []byte(`{"arg":"b"}`), }) @@ -476,11 +517,12 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + skipVariableWithDefault := "4000000000000000000000000000000000000000000000000000000000000000" header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{}`), }) @@ -489,7 +531,7 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { require.Equal(t, `{"data":{"employee":{"details":{"forename":"Jens","surname":"Neuse"}}}}`, res.Body) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{}`), }) @@ -498,7 +540,7 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { require.Equal(t, `{"data":{"employee":{"details":{"forename":"Jens","surname":"Neuse"}}}}`, res.Body) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{"yes":false}`), }) @@ -507,7 +549,7 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{"yes":false}`), }) @@ -516,7 +558,7 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{"yes":true}`), }) @@ -525,7 +567,7 @@ func TestPersistedOperationCacheWithVariablesAndDefaultValues(t *testing.T) { require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "skipVariableWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + skipVariableWithDefault + `"}}`), Header: header, Variables: []byte(`{"yes":true}`), }) @@ -539,11 +581,12 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { t.Parallel() testenv.Run(t, &testenv.Config{}, func(t *testing.T, xEnv *testenv.Environment) { + listArgQuery := "1000000000000000000000000000000000000000000000000000000000000000" header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQuery"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQuery + `"}}`), Header: header, Variables: []byte(`{"arg": "a"}`), }) @@ -552,7 +595,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQuery"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQuery + `"}}`), Header: header, Variables: []byte(`{"arg": "a"}`), }) @@ -561,7 +604,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQuery"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQuery + `"}}`), Header: header, Variables: []byte(`{"arg": "b"}`), }) @@ -569,9 +612,10 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, `{"data":{"rootFieldWithListArg":["b"]}}`, res.Body) require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) + listArgQueryWithDefault := "2000000000000000000000000000000000000000000000000000000000000000" res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQueryWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQueryWithDefault + `"}}`), Header: header, Variables: []byte(`{}`), }) @@ -580,7 +624,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQueryWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQueryWithDefault + `"}}`), Header: header, Variables: []byte(`{}`), }) @@ -589,7 +633,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQueryWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQueryWithDefault + `"}}`), Header: header, Variables: []byte(`{"arg": "b"}`), }) @@ -598,7 +642,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQueryWithDefault"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQueryWithDefault + `"}}`), Header: header, Variables: []byte(`{"arg": ["c"]}`), }) @@ -606,10 +650,11 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, `{"data":{"rootFieldWithListArg":["c"]}}`, res.Body) require.Equal(t, "HIT", res.Response.Header.Get(core.PersistedOperationCacheHeader)) + nestedEnum := "3000000000000000000000000000000000000000000000000000000000000000" // nested list of enums res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "nestedEnum"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + nestedEnum + `"}}`), Header: header, Variables: []byte(`{"arg":{"enums":"A"}}`), }) @@ -618,7 +663,7 @@ func TestPersistedOperationCacheWithVariablesCoercion(t *testing.T) { require.Equal(t, "MISS", res.Response.Header.Get(core.PersistedOperationCacheHeader)) res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "nestedEnum"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + nestedEnum + `"}}`), Header: header, Variables: []byte(`{"arg":{"enums":"B"}}`), }) diff --git a/router-tests/telemetry/telemetry_test.go b/router-tests/telemetry/telemetry_test.go index b6f3ef0298..f8058d4aef 100644 --- a/router-tests/telemetry/telemetry_test.go +++ b/router-tests/telemetry/telemetry_test.go @@ -4039,11 +4039,12 @@ func TestFlakyTelemetry(t *testing.T) { TraceExporter: exporter, MetricReader: metricReader, }, func(t *testing.T, xEnv *testenv.Environment) { + listArgQuery := "1000000000000000000000000000000000000000000000000000000000000000" header := make(http.Header) header.Add("graphql-client-name", "my-client") res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQuery"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQuery + `"}}`), Header: header, Variables: []byte(`{"arg": "a"}`), }) @@ -4071,7 +4072,7 @@ func TestFlakyTelemetry(t *testing.T) { res, err = xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ OperationName: []byte(`"MyQuery"`), - Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "listArgQuery"}}`), + Extensions: []byte(`{"persistedQuery": {"version": 1, "sha256Hash": "` + listArgQuery + `"}}`), Header: header, Variables: []byte(`{"arg": "a"}`), }) diff --git a/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/listArgQuery.json b/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/1000000000000000000000000000000000000000000000000000000000000000.json similarity index 100% rename from router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/listArgQuery.json rename to router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/1000000000000000000000000000000000000000000000000000000000000000.json diff --git a/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/listArgQueryWithDefault.json b/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/2000000000000000000000000000000000000000000000000000000000000000.json similarity index 100% rename from router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/listArgQueryWithDefault.json rename to router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/2000000000000000000000000000000000000000000000000000000000000000.json diff --git a/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/nestedEnum.json b/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/3000000000000000000000000000000000000000000000000000000000000000.json similarity index 100% rename from router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/nestedEnum.json rename to router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/3000000000000000000000000000000000000000000000000000000000000000.json diff --git a/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/skipVariableWithDefault.json b/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/4000000000000000000000000000000000000000000000000000000000000000.json similarity index 100% rename from router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/skipVariableWithDefault.json rename to router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/4000000000000000000000000000000000000000000000000000000000000000.json diff --git a/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/nestedVariableExtraction.json b/router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/5000000000000000000000000000000000000000000000000000000000000000.json similarity index 100% rename from router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/nestedVariableExtraction.json rename to router-tests/testenv/testdata/cdn/organization/graph/operations/my-client/5000000000000000000000000000000000000000000000000000000000000000.json diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index 944b325d7b..1592df8cf0 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -476,8 +476,8 @@ func (h *PreHandler) shouldComputeOperationSha256(operationKit *OperationKit) bo return true } - hasPersistedHash := operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery != nil && operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash != "" - // If it already has a persisted hash attached to the request, then there is no need for us to compute it anew + hasPersistedHash := operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.HasHash() + // If it already has a persisted hash attached to the request, then there is no need for us to compute it anew. // Otherwise, we only want to compute the hash (an expensive operation) if we're safelisting or logging unknown persisted operations return !hasPersistedHash && (h.operationBlocker.safelistEnabled || h.operationBlocker.logUnknownOperationsEnabled) } @@ -512,14 +512,14 @@ func (h *PreHandler) handleOperation(req *http.Request, variablesParser *astjson if req.Method == http.MethodGet { if err := operationKit.UnmarshalOperationFromURL(req.URL); err != nil { return &httpGraphqlError{ - message: fmt.Sprintf("error parsing request query params: %s", err), + message: fmt.Sprintf("invalid GET request: %s", err), statusCode: http.StatusBadRequest, } } } else if req.Method == http.MethodPost { if err := operationKit.UnmarshalOperationFromBody(httpOperation.body); err != nil { return &httpGraphqlError{ - message: "error parsing request body", + message: fmt.Sprintf("invalid request body: %s", err), statusCode: http.StatusBadRequest, } } @@ -685,11 +685,10 @@ func (h *PreHandler) handleOperation(req *http.Request, variablesParser *astjson } } - if operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery != nil && - operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash != "" { - - requestContext.operation.persistedID = operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash - persistedIDAttribute := otel.WgOperationPersistedID.String(operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash) + if operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.HasHash() { + hash := operationKit.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash + requestContext.operation.persistedID = hash + persistedIDAttribute := otel.WgOperationPersistedID.String(hash) requestContext.telemetry.addCommonAttribute(persistedIDAttribute) diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index 9cb939fa33..06c0a8264f 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -192,6 +192,19 @@ type GraphQLRequestExtensionsPersistedQuery struct { Sha256Hash string `json:"sha256Hash"` } +// isValidHash verifies if the Sha256Hash string is valid and well-formed. +func (pq *GraphQLRequestExtensionsPersistedQuery) isValidHash() bool { + if len(pq.Sha256Hash) != 64 { + return false + } + _, err := hex.DecodeString(pq.Sha256Hash) + return err == nil +} + +func (pq *GraphQLRequestExtensionsPersistedQuery) HasHash() bool { + return pq != nil && len(pq.Sha256Hash) > 0 +} + type complexityComparison struct { field int cachedField int @@ -248,29 +261,26 @@ func (o *OperationKit) UnmarshalOperationFromURL(url *url.URL) error { variables := values.Get("variables") if variables != "" { o.parsedOperation.Request.Variables = []byte(variables) + // Do sanity check early with json because later we parse variables with fastjson, + // and fastjson produces verbose error messages. buf := bytes.NewBuffer(make([]byte, len(o.parsedOperation.Request.Variables))[:0]) err := json.Compact(buf, o.parsedOperation.Request.Variables) if err != nil { - return err + return fmt.Errorf("error parsing variables: %w", err) } } extensions := values.Get("extensions") if extensions != "" { o.parsedOperation.Request.Extensions = []byte(extensions) - buf := bytes.NewBuffer(make([]byte, len(o.parsedOperation.Request.Extensions))[:0]) - err := json.Compact(buf, o.parsedOperation.Request.Extensions) - if err != nil { - return err - } } return o.unmarshalOperation() } -// UnmarshalOperationFromBody loads the operation from the request body and unmarshal it into the ParsedOperation -// This will load operationName, query, variables and extensions from the request body but extension and variables -// will be unmarshalled as JSON.RawMessage. +// UnmarshalOperationFromBody loads the operation from the request body and unmarshal it into the ParsedOperation. +// This will load operationName, query, variables and extensions from the request body, +// but extension and variables will be unmarshalled as JSON.RawMessage. // We always compact the variables and extensions to ensure that we produce easy to parse JSON for the engine func (o *OperationKit) UnmarshalOperationFromBody(data []byte) error { buf := bytes.NewBuffer(make([]byte, len(data))[:0]) @@ -291,23 +301,31 @@ func (o *OperationKit) UnmarshalOperationFromBody(data []byte) error { func (o *OperationKit) unmarshalOperation() error { var err error - if o.parsedOperation.Request.Extensions != nil { - var mapExtensions map[string]any - err = json.Unmarshal(o.parsedOperation.Request.Extensions, &mapExtensions) - if err != nil { - return &httpGraphqlError{ - message: fmt.Sprintf("error parsing extensions: %s", err), - statusCode: http.StatusBadRequest, - } + // trimmedError removes details not relevant to a user + trimmedError := func(err error) string { + var ue *json.UnmarshalTypeError + if errors.As(err, &ue) { + return "json: cannot unmarshal " + ue.Value } + return err.Error() + } + + if o.parsedOperation.Request.Extensions != nil { err = json.Unmarshal(o.parsedOperation.Request.Extensions, &o.parsedOperation.GraphQLRequestExtensions) if err != nil { return &httpGraphqlError{ - message: fmt.Sprintf("error parsing extensions: %s", err), + message: fmt.Sprintf("error parsing extensions: %s", trimmedError(err)), statusCode: http.StatusBadRequest, } } if o.parsedOperation.GraphQLRequestExtensions.PersistedQuery != nil { + if !o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.isValidHash() { + return &httpGraphqlError{ + message: "persistedQuery does not have a valid sha256 hash", + statusCode: http.StatusBadRequest, + } + } + // Delete persistedQuery from extensions to avoid it being passed to the subgraphs o.parsedOperation.Request.Extensions, err = sjson.DeleteBytes(o.parsedOperation.Request.Extensions, "persistedQuery") if err != nil { @@ -338,13 +356,13 @@ func (o *OperationKit) unmarshalOperation() error { o.parsedOperation.Variables = variables.GetObject() default: return &httpGraphqlError{ - message: "variables must be an object", + message: "variables must be a JSON object", statusCode: http.StatusBadRequest, } } } else { - // set variables to empty object if they are null, so we can later add exported defaults - // also, other parts of the engine depend on variables being a valid JSON object + // Set variables to an empty object if they are null, so we can add exported defaults later. + // Also, other parts of the engine depend on variables being a valid JSON object. o.parsedOperation.Request.Variables = []byte("{}") o.parsedOperation.Variables = fastjson.MustParseBytes(o.parsedOperation.Request.Variables).GetObject() } @@ -354,7 +372,7 @@ func (o *OperationKit) unmarshalOperation() error { o.parsedOperation.Request.OperationName = "" } - if o.parsedOperation.GraphQLRequestExtensions.PersistedQuery != nil && len(o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.Sha256Hash) > 0 { + if o.parsedOperation.GraphQLRequestExtensions.PersistedQuery.HasHash() { o.parsedOperation.IsPersistedOperation = true } @@ -509,7 +527,7 @@ func (o *OperationKit) isIntrospectionQuery() (result bool, err error) { return false, nil } -// Parse parses the operation, populate the document and set the operation type. +// Parse parses the operation, populates the document and set the operation type. // UnmarshalOperationFromBody must be called before calling this method. func (o *OperationKit) Parse() error { var ( @@ -519,7 +537,7 @@ func (o *OperationKit) Parse() error { if len(o.parsedOperation.Request.Query) == 0 { return &httpGraphqlError{ - message: "error parsing request body", + message: "empty request body", statusCode: http.StatusBadRequest, } } diff --git a/router/core/operation_processor_test.go b/router/core/operation_processor_test.go index e498ff0a85..ba63cfe899 100644 --- a/router/core/operation_processor_test.go +++ b/router/core/operation_processor_test.go @@ -29,6 +29,7 @@ func TestOperationProcessorPersistentOperations(t *testing.T) { Name: "test", Version: "1.0.0", } + const cacheHashNotStored = "0000000000000000000000000000000000000000000000000000000000000000" testCases := []struct { ExpectedType string ExpectedError error @@ -39,7 +40,7 @@ func TestOperationProcessorPersistentOperations(t *testing.T) { * Test cases persist operation */ { - Input: `{"operationName": "test", "variables": {"foo": "bar"}, "extensions": {"persistedQuery": {"version": 1, "sha256Hash": "does-not-exist"}}}`, + Input: `{"operationName": "test", "variables": {"foo": "bar"}, "extensions": {"persistedQuery": {"version": 1, "sha256Hash": "` + cacheHashNotStored + `"}}}`, Variables: `{"foo": "bar"}`, ExpectedError: errors.New("could not resolve persisted query, feature is not configured"), },