graphql-go / graphql

An implementation of GraphQL for Go / Golang
MIT License
9.87k stars 839 forks source link

Dangerous issue about Go anonymous composition #553

Open alimoli opened 4 years ago

alimoli commented 4 years ago

I discovered this bug trying to resolve this problem https://github.com/graphql-go/graphql/issues/183#issuecomment-659317404 .

Essentially, this library does not allow to have anonymous fields for compositions since those fields are not evaluated.

Example

type Car struct {
       Speed int `json:"speed"`
       Product
}

type Product struct {
       Price int `json:"price"`
       Description string `json:"description"`
}

GraphQL schema

carType := graphql.NewObject(graphql.ObjectConfig{
    Name: "Car",
    Fields: graphql.Fields{
        "speed": &graphql.Field{
            Type:        graphql.NewNonNull(graphql.Int),
        },
        "price": &graphql.Field{
            Type:        graphql.NewNonNull(graphql.Int),
        },
        "description": &graphql.Field{
            Type:        graphql.NewNonNull(graphql.String),
        },
    },
})

Result

The price and description fields has a null value for GraphQL library and this is not allowed since we declare a graphql.NewNonNull field. Indeed, we get the following error:

Temporary solution

The only way to fix this problem at the moment is to duplicate code:

type Car struct {
       Speed int `json:"speed"`
       Price int `json:"price"`
       Description string `json:"description"`
}

type Product struct {
       Price int `json:"price"`
       Description string `json:"description"`
}

Or changing a lot of things both internal and external (also the public fields to API consumers).

type Car struct {
       Speed int `json:"speed"`
       Product Product `json:"product"`
}

type Product struct {
       Price int `json:"price"`
       Description string `json:"description"`
}

Instead of:

{
   speed
   price
   description
}

We have to provide this:

{
   speed
   product {
      price
      description
   }
}

Both solutions are really limited and go against clean code.

Nemesisesq commented 4 years ago

I found this as well the issue. In DefaultResolveFn the struct that's reflected isn't "flattened" so it' never accesses the tag on the struct field.