graphql-go / graphql-go/graphql
Exported context field in graphql.Params and others is dangerous as of Go 1.15
- Dominant language
- Go
- Stars
- 10.1k
- Forks
- 845
- PR merge metrics
- No merged PRs in 30d
Description
The practice of storing a context as a struct field in `graphql.Params` can result in users doing things like:
```
ctx, cancel := context.WithCancel(p.Context)
```
If `p.Context` is nil this will cause a panic, as of Go 1.15.
It's not clear what an effective resolution might look like. It's not possible to remove the field without breaking compatibility, and it's quite unsafe to use the field directly.
Perhaps the issue could be mitigated by using methods. For example,
```
func (p graphql.Params) RequestContext() context.Context {
if p.Context != nil {
return p.Context
}
return context.Background()
}
```
If the `graphql` codebase replaced instances of `p.Context` with `p.RequestContext()`, and users did the same, this would at least provide a modicum of safety, assuming these methods were written for any types that feature an exported `Context` field. Users could use tools like `semgrep` to make sure they aren't using `p.Context` values directly.
Contributor guide
Assessment
This issue has not been assessed yet.