search

shurcooL / graphql

Package graphql provides a GraphQL client implementation.
MIT License
706 stars 283 forks source link

Stack overflow when types contain a cycle #17

Open eric-am opened 6 years ago

eric-am commented 6 years ago

(Originally mentioned at https://github.com/shurcooL/graphql/issues/9, but filing as a separate issue for clarity of discussion.)

Structs with fields that reference themselves (or reference other types that eventually reference themselves) result in a stack overflow as the query generator code traverses them.

For example, this is a problematic struct containing fields with its own type:

type Foobar struct {
    Children []Foobar
}

This is the relevant part of the stack trace resulting:

runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow

goroutine 7 [running]:
... some frames elided ....
theproject/vendor/github.com/shurcooL/graphql.writeQuery(0x15b3be0, 0xc42017f180, 0x15be320, 0x139e100, 0x0)
        /theproject/vendor/github.com/shurcooL/graphql/query.go:103 +0x93 fp=0xc4402005c0 sp=0xc4402004c0 pc=0x1303dd3
theproject/vendor/github.com/shurcooL/graphql.writeQuery(0x15b3be0, 0xc42017f180, 0x15be320, 0x13ba360, 0x133be00)
        /theproject/vendor/github.com/shurcooL/graphql/query.go:123 +0x11f fp=0xc4402006c0 sp=0xc4402005c0 pc=0x1303e5f
theproject/vendor/github.com/shurcooL/graphql.writeQuery(0x15b3be0, 0xc42017f180, 0x15be320, 0x1358c40, 0x0)
        /theproject/vendor/github.com/shurcooL/graphql/query.go:100 +0x3fa fp=0xc4402007c0 sp=0xc4402006c0 pc=0x130413a
theproject/vendor/github.com/shurcooL/graphql.writeQuery(0x15b3be0, 0xc42017f180, 0x15be320, 0x13ac720, 0x133be00)
        /theproject/vendor/github.com/shurcooL/graphql/query.go:123 +0x11f fp=0xc4402008c0 sp=0xc4402007c0 pc=0x1303e5f
... loop continues between 100 and 123 ...

Basically, the writeQuery function as currently written is perfectly happy to keep looking up the type of the Foobar struct, looking at the Children field and generating a little more query, then recursing to look up the type of the field, which is of course still Foobar...

It would be nicer if the library detected this cycle and raised an error with a relevant message as soon as possible. A panic still seems reasonable (IMO), because you're pretty much statically doomed from the time you compiled the program and have no hope of handling this; but a stack overflow as that panic is a bit of a bummer.

droslean commented 2 years ago

@dmitshur Same here. What is the status of this issue? Currently, this problem makes the library useless.