Skip to content

Commit

Permalink
fix(GraphQl):This PR fix a panic when we pass a single ID as a intege…
Browse files Browse the repository at this point in the history
…r and expected type is [ID].We now coerce that to type array of string. (#7325)

The following query was giving panic because here we passed ID as an int which is expected to be a string.

query allStories {
      queryUser(filter: {
        id: 22
      }) {
        stories {
          id
          text
        }
      }
    }


We now added input coercion so that the ID type value will be coerced to string type. And if we give a slice of integer values
to ID type variable then that will be coerced to slice of integer values.
  • Loading branch information
JatinDev543 authored Jan 21, 2021
1 parent d2ea0e6 commit 5fa6796
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 18 deletions.
2 changes: 1 addition & 1 deletion graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ func RunAll(t *testing.T) {
t.Run("query id directive with int", idDirectiveWithInt)
t.Run("query id directive with int64", idDirectiveWithInt64)
t.Run("query id directive with float", idDirectiveWithFloat)
t.Run("query using single ID in a filter that will be coerced to list", queryFilterSingleIDListCoercion)
t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)
// mutation tests
t.Run("add mutation", addMutation)
t.Run("update mutation by ids", updateMutationByIds)
Expand Down
162 changes: 146 additions & 16 deletions graphql/e2e/common/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package common
import (
"encoding/json"
"fmt"
"github.com/spf13/cast"
"io/ioutil"
"math/rand"
"net/http"
Expand Down Expand Up @@ -3410,7 +3411,7 @@ func passwordTest(t *testing.T) {
deleteUser(t, *newUser)
}

func queryFilterSingleIDListCoercion(t *testing.T) {
func queryFilterWithIDInputCoercion(t *testing.T) {
authors := addMultipleAuthorFromRef(t, []*author{
{
Name: "George",
Expand All @@ -3427,25 +3428,54 @@ func queryFilterSingleIDListCoercion(t *testing.T) {
authorIds := []string{authors[0].ID, authors[1].ID}
postIds := []string{authors[0].Posts[0].PostID, authors[1].Posts[0].PostID}
countryIds := []string{authors[1].Country.ID}
tcase := struct {
authorIdsDecimal := []string{cast.ToString(cast.ToInt(authorIds[0])), cast.ToString(cast.ToInt(authorIds[1]))}
tcases := []struct {
name string
query string
variables map[string]interface{}
respData string
}{
{

name: "Query using single ID in a filter",
query: `query($filter:AuthorFilter){
name: "Query using single ID in a filter",
query: `query($filter:AuthorFilter){
queryAuthor(filter:$filter){
name
reputation
reputation
posts {
text
}
}
}`,
variables: map[string]interface{}{"filter": map[string]interface{}{"id": authors[0].ID}},
respData: `{
variables: map[string]interface{}{"filter": map[string]interface{}{"id": authors[0].ID}},
respData: `{
"queryAuthor": [
{
"name": "George",
"reputation": 4.5,
"posts": [
{
"text": "Got ya!"
}
]
}
]
}`,
},
{

name: "Query using single ID given in variable of type integer coerced to string ",
query: `query($filter:AuthorFilter){
queryAuthor(filter:$filter){
name
reputation
posts {
text
}
}
}`,
variables: map[string]interface{}{"filter": map[string]interface{}{"id": cast.ToInt(authors[0].ID)}},
respData: `{
"queryAuthor": [
{
"name": "George",
Expand All @@ -3458,17 +3488,117 @@ func queryFilterSingleIDListCoercion(t *testing.T) {
}
]
}`,
},
{

name: "Query using multiple ID given in variable of type integer coerced to string",
query: `query($filter:AuthorFilter){
queryAuthor(filter:$filter){
name
reputation
posts {
title
}
}
}`,
variables: map[string]interface{}{"filter": map[string]interface{}{"id": []int{cast.ToInt(authors[0].ID), cast.ToInt(authors[1].ID)}}},
respData: `{
"queryAuthor": [
{
"name": "George",
"reputation": 4.5,
"posts": [
{
"title": "A show about nothing"
}
]
},
{
"name": "Jerry",
"reputation": 4.6,
"posts": [
{
"title": "Outside"
}
]
}
]
}`,
},
{

name: "Query using single ID in a filter of type integer coerced to string",
query: `query{
queryAuthor(filter:{id:` + authorIdsDecimal[0] + `}){
name
reputation
posts {
title
}
}
}`,
respData: `{
"queryAuthor": [
{
"name": "George",
"reputation": 4.5,
"posts": [
{
"title": "A show about nothing"
}
]
}
]
}`,
},
{

name: "Query using multiple ID in a filter of type integer coerced to string",
query: `query{
queryAuthor(filter:{id:[` + authorIdsDecimal[0] + `,` + authorIdsDecimal[1] + `]}){
name
reputation
posts {
title
}
}
}`,
respData: `{
"queryAuthor": [
{
"name": "George",
"reputation": 4.5,
"posts": [
{
"title": "A show about nothing"
}
]
},
{
"name": "Jerry",
"reputation": 4.6,
"posts": [
{
"title": "Outside"
}
]
}
]
}`,
},
}

t.Run(tcase.name, func(t *testing.T) {
params := &GraphQLParams{
Query: tcase.query,
Variables: tcase.variables,
}
resp := params.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, resp)
testutil.CompareJSON(t, tcase.respData, string(resp.Data))
})
for _, tcase := range tcases {
t.Run(tcase.name, func(t *testing.T) {
params := &GraphQLParams{
Query: tcase.query,
Variables: tcase.variables,
}
resp := params.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, resp)
testutil.CompareJSON(t, tcase.respData, string(resp.Data))
})
}

// cleanup
deleteAuthors(t, authorIds, nil)
Expand Down
7 changes: 6 additions & 1 deletion graphql/schema/validation_rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,17 @@ func listInputCoercion(observers *validator.Events, addError validator.AddErrFun
if value.Kind == ast.Variable {
return
}
if value.ExpectedType.NamedType == IDType {
value.Kind = ast.StringValue
}
// If the expected value is a list (ExpectedType.Elem != nil) && the value is not of list type,
// then we need to coerce the value to a list, otherwise, we can return here as we do below.

if !(value.ExpectedType.Elem != nil && value.Kind != ast.ListValue) {
return
}
if value.ExpectedType.Elem.NamedType == IDType {
value.Kind = ast.StringValue
}
val := *value
child := &ast.ChildValue{Value: &val}
valueNew := ast.Value{Children: []*ast.ChildValue{child}, Kind: ast.ListValue, Position: val.Position, Definition: val.Definition}
Expand Down

0 comments on commit 5fa6796

Please sign in to comment.