From 3605643b8d602222036f55b8e2168ebf50e52d68 Mon Sep 17 00:00:00 2001 From: vvakame Date: Tue, 20 Nov 2018 18:48:44 +0900 Subject: [PATCH 1/3] check reserved name at validator --- ast/definition.go | 1 + ast/source.go | 5 ++-- parser/schema.go | 14 +++++++++- validator/prelude.go | 6 ++++- validator/schema.go | 33 +++++++++++++++++++++++ validator/schema_test.yml | 56 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 111 insertions(+), 4 deletions(-) diff --git a/ast/definition.go b/ast/definition.go index 74f4ece5..f5c8ea37 100644 --- a/ast/definition.go +++ b/ast/definition.go @@ -30,6 +30,7 @@ type Definition struct { EnumValues EnumValueList // enum Position *Position `dump:"-"` + BuiltIn bool `dump:"-"` } func (d *Definition) IsLeafType() bool { diff --git a/ast/source.go b/ast/source.go index 9d44dd9c..acb07ba6 100644 --- a/ast/source.go +++ b/ast/source.go @@ -1,8 +1,9 @@ package ast type Source struct { - Name string - Input string + Name string + Input string + BuiltIn bool } type Position struct { diff --git a/parser/schema.go b/parser/schema.go index 6d303022..5689e433 100644 --- a/parser/schema.go +++ b/parser/schema.go @@ -10,7 +10,19 @@ func ParseSchema(source *Source) (*SchemaDocument, *gqlerror.Error) { p := parser{ lexer: lexer.New(source), } - return p.parseSchemaDocument(), p.err + ast, err := p.parseSchemaDocument(), p.err + if err != nil { + return nil, err + } + + for _, def := range ast.Definitions { + def.BuiltIn = source.BuiltIn + } + for _, def := range ast.Extensions { + def.BuiltIn = source.BuiltIn + } + + return ast, nil } func ParseSchemas(inputs ...*Source) (*SchemaDocument, *gqlerror.Error) { diff --git a/validator/prelude.go b/validator/prelude.go index 80ce8a21..c7a4d35b 100644 --- a/validator/prelude.go +++ b/validator/prelude.go @@ -2,4 +2,8 @@ package validator import "github.com/vektah/gqlparser/ast" -var Prelude = &ast.Source{Name: "prelude.graphql", Input: "# This file defines all the implicitly declared types that are required by the graphql spec. It is implicitly included by calls to LoadSchema\n\n# The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1.\nscalar Int\n\n# The `Float` scalar type represents signed double-precision fractional values as specified by [IEEE 754](http://en.wikipedia.org/wiki/IEEE_floating_point).\nscalar Float\n\n# The `String`scalar type represents textual data, represented as UTF-8 character sequences. The String type is most often used by GraphQL to represent free-form human-readable text.\nscalar String\n\n# The `Boolean` scalar type represents ` + \"`\" + `true` + \"`\" + ` or ` + \"`\" + `false` + \"`\" + `.\nscalar Boolean\n\n# The `ID` scalar type represents a unique identifier, often used to refetch an object or as key for a cache. The ID type appears in a JSON response as a String; however, it is not intended to be human-readable. When expected as an input type, any string (such as \"4\") or integer (such as 4) input value will be accepted as an ID.\nscalar ID\n\n# The @include directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional inclusion during execution as described by the if argument.\ndirective @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT\n\n# The @skip directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional exclusion during execution as described by the if argument.\ndirective @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT\n\n# The @deprecated directive is used within the type system definition language to indicate deprecated portions of a GraphQL service’s schema, such as deprecated fields on a type or deprecated enum values.\ndirective @deprecated(reason: String = \"No longer supported\") on FIELD_DEFINITION | ENUM_VALUE\n\ntype __Schema {\n types: [__Type!]!\n queryType: __Type!\n mutationType: __Type\n subscriptionType: __Type\n directives: [__Directive!]!\n}\n\ntype __Type {\n kind: __TypeKind!\n name: String\n description: String\n\n # OBJECT and INTERFACE only\n fields(includeDeprecated: Boolean = false): [__Field!]\n\n # OBJECT only\n interfaces: [__Type!]\n\n # INTERFACE and UNION only\n possibleTypes: [__Type!]\n\n # ENUM only\n enumValues(includeDeprecated: Boolean = false): [__EnumValue!]\n\n # INPUT_OBJECT only\n inputFields: [__InputValue!]\n\n # NON_NULL and LIST only\n ofType: __Type\n}\n\ntype __Field {\n name: String!\n description: String\n args: [__InputValue!]!\n type: __Type!\n isDeprecated: Boolean!\n deprecationReason: String\n}\n\ntype __InputValue {\n name: String!\n description: String\n type: __Type!\n defaultValue: String\n}\n\ntype __EnumValue {\n name: String!\n description: String\n isDeprecated: Boolean!\n deprecationReason: String\n}\n\nenum __TypeKind {\n SCALAR\n OBJECT\n INTERFACE\n UNION\n ENUM\n INPUT_OBJECT\n LIST\n NON_NULL\n}\n\ntype __Directive {\n name: String!\n description: String\n locations: [__DirectiveLocation!]!\n args: [__InputValue!]!\n}\n\nenum __DirectiveLocation {\n QUERY\n MUTATION\n SUBSCRIPTION\n FIELD\n FRAGMENT_DEFINITION\n FRAGMENT_SPREAD\n INLINE_FRAGMENT\n SCHEMA\n SCALAR\n OBJECT\n FIELD_DEFINITION\n ARGUMENT_DEFINITION\n INTERFACE\n UNION\n ENUM\n ENUM_VALUE\n INPUT_OBJECT\n INPUT_FIELD_DEFINITION\n}\n"} +var Prelude = &ast.Source{ + Name: "prelude.graphql", + Input: "# This file defines all the implicitly declared types that are required by the graphql spec. It is implicitly included by calls to LoadSchema\n\n# The `Int` scalar type represents non-fractional signed whole numeric values. Int can represent values between -(2^31) and 2^31 - 1.\nscalar Int\n\n# The `Float` scalar type represents signed double-precision fractional values as specified by [IEEE 754](http://en.wikipedia.org/wiki/IEEE_floating_point).\nscalar Float\n\n# The `String`scalar type represents textual data, represented as UTF-8 character sequences. The String type is most often used by GraphQL to represent free-form human-readable text.\nscalar String\n\n# The `Boolean` scalar type represents ` + \"`\" + `true` + \"`\" + ` or ` + \"`\" + `false` + \"`\" + `.\nscalar Boolean\n\n# The `ID` scalar type represents a unique identifier, often used to refetch an object or as key for a cache. The ID type appears in a JSON response as a String; however, it is not intended to be human-readable. When expected as an input type, any string (such as \"4\") or integer (such as 4) input value will be accepted as an ID.\nscalar ID\n\n# The @include directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional inclusion during execution as described by the if argument.\ndirective @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT\n\n# The @skip directive may be provided for fields, fragment spreads, and inline fragments, and allows for conditional exclusion during execution as described by the if argument.\ndirective @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT\n\n# The @deprecated directive is used within the type system definition language to indicate deprecated portions of a GraphQL service’s schema, such as deprecated fields on a type or deprecated enum values.\ndirective @deprecated(reason: String = \"No longer supported\") on FIELD_DEFINITION | ENUM_VALUE\n\ntype __Schema {\n types: [__Type!]!\n queryType: __Type!\n mutationType: __Type\n subscriptionType: __Type\n directives: [__Directive!]!\n}\n\ntype __Type {\n kind: __TypeKind!\n name: String\n description: String\n\n # OBJECT and INTERFACE only\n fields(includeDeprecated: Boolean = false): [__Field!]\n\n # OBJECT only\n interfaces: [__Type!]\n\n # INTERFACE and UNION only\n possibleTypes: [__Type!]\n\n # ENUM only\n enumValues(includeDeprecated: Boolean = false): [__EnumValue!]\n\n # INPUT_OBJECT only\n inputFields: [__InputValue!]\n\n # NON_NULL and LIST only\n ofType: __Type\n}\n\ntype __Field {\n name: String!\n description: String\n args: [__InputValue!]!\n type: __Type!\n isDeprecated: Boolean!\n deprecationReason: String\n}\n\ntype __InputValue {\n name: String!\n description: String\n type: __Type!\n defaultValue: String\n}\n\ntype __EnumValue {\n name: String!\n description: String\n isDeprecated: Boolean!\n deprecationReason: String\n}\n\nenum __TypeKind {\n SCALAR\n OBJECT\n INTERFACE\n UNION\n ENUM\n INPUT_OBJECT\n LIST\n NON_NULL\n}\n\ntype __Directive {\n name: String!\n description: String\n locations: [__DirectiveLocation!]!\n args: [__InputValue!]!\n}\n\nenum __DirectiveLocation {\n QUERY\n MUTATION\n SUBSCRIPTION\n FIELD\n FRAGMENT_DEFINITION\n FRAGMENT_SPREAD\n INLINE_FRAGMENT\n SCHEMA\n SCALAR\n OBJECT\n FIELD_DEFINITION\n ARGUMENT_DEFINITION\n INTERFACE\n UNION\n ENUM\n ENUM_VALUE\n INPUT_OBJECT\n INPUT_FIELD_DEFINITION\n}\n", + BuiltIn: true, +} diff --git a/validator/schema.go b/validator/schema.go index 1780ae36..f63e6961 100644 --- a/validator/schema.go +++ b/validator/schema.go @@ -4,6 +4,7 @@ package validator import ( "strconv" + "strings" . "github.com/vektah/gqlparser/ast" "github.com/vektah/gqlparser/gqlerror" @@ -158,11 +159,20 @@ func ValidateSchemaDocument(ast *SchemaDocument) (*Schema, *gqlerror.Error) { } func validateDirective(schema *Schema, def *DirectiveDefinition) *gqlerror.Error { + if err := validateName(def.Position, def.Name); err != nil { + // now, GraphQL spec doesn't have reserved directive name + return err + } + return validateArgs(schema, def.Arguments, def) } func validateDefinition(schema *Schema, def *Definition) *gqlerror.Error { for _, field := range def.Fields { + if err := validateName(field.Position, field.Name); err != nil { + // now, GraphQL spec doesn't have reserved field name + return err + } if err := validateTypeRef(schema, field.Type); err != nil { return err } @@ -199,6 +209,14 @@ func validateDefinition(schema *Schema, def *Definition) *gqlerror.Error { } } + if !def.BuiltIn { + // GraphQL spec has reserved type names a lot! + err := validateName(def.Position, def.Name) + if err != nil { + return err + } + } + return validateDirectives(schema, def.Directives, nil) } @@ -211,6 +229,10 @@ func validateTypeRef(schema *Schema, typ *Type) *gqlerror.Error { func validateArgs(schema *Schema, args ArgumentDefinitionList, currentDirective *DirectiveDefinition) *gqlerror.Error { for _, arg := range args { + if err := validateName(arg.Position, arg.Name); err != nil { + // now, GraphQL spec doesn't have reserved argument name + return err + } if err := validateTypeRef(schema, arg.Type); err != nil { return err } @@ -223,6 +245,10 @@ func validateArgs(schema *Schema, args ArgumentDefinitionList, currentDirective func validateDirectives(schema *Schema, dirs DirectiveList, currentDirective *DirectiveDefinition) *gqlerror.Error { for _, dir := range dirs { + if err := validateName(dir.Position, dir.Name); err != nil { + // now, GraphQL spec doesn't have reserved directive name + return err + } if currentDirective != nil && dir.Name == currentDirective.Name { return gqlerror.ErrorPosf(dir.Position, "Directive %s cannot refer to itself.", currentDirective.Name) } @@ -233,3 +259,10 @@ func validateDirectives(schema *Schema, dirs DirectiveList, currentDirective *Di } return nil } + +func validateName(pos *Position, name string) *gqlerror.Error { + if strings.HasPrefix(name, "__") { + return gqlerror.ErrorPosf(pos, `Name "%s" must not begin with "__", which is reserved by GraphQL introspection.`, name) + } + return nil +} diff --git a/validator/schema_test.yml b/validator/schema_test.yml index 375fb640..e3ab10b1 100644 --- a/validator/schema_test.yml +++ b/validator/schema_test.yml @@ -31,6 +31,31 @@ object types: error: message: 'OBJECT must define one or more fields.' locations: [{line: 6, column: 6}] + - name: check reserved names on type name + input: | + type __FooBar { + id: ID + } + error: + message: 'Name "__FooBar" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 1, column: 6}] + - name: check reserved names on type field + input: | + type FooBar { + __id: ID + } + error: + message: 'Name "__id" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 2, column: 3}] + + - name: check reserved names on type field argument + input: | + type FooBar { + foo(__bar: ID): ID + } + error: + message: 'Name "__bar" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 2, column: 7}] interfaces: - name: must exist @@ -82,6 +107,14 @@ interfaces: error: message: 'INTERFACE must define one or more fields.' locations: [{line: 6, column: 11}] + - name: check reserved names on type name + input: | + interface __FooBar { + id: ID + } + error: + message: 'Name "__FooBar" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 1, column: 11}] inputs: - name: must define one or more input fields @@ -103,6 +136,14 @@ inputs: error: message: 'INPUT_OBJECT must define one or more input fields.' locations: [{line: 6, column: 7}] + - name: check reserved names on type name + input: | + input __FooBar { + id: ID + } + error: + message: 'Name "__FooBar" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 1, column: 7}] enums: - name: must define one or more unique enum values @@ -124,6 +165,15 @@ enums: error: message: 'ENUM must define one or more unique enum values.' locations: [{line: 6, column: 6}] + - name: check reserved names on type name + input: | + enum __FooBar { + A + B + } + error: + message: 'Name "__FooBar" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 1, column: 6}] type extensions: - name: cannot extend non existant types @@ -169,6 +219,12 @@ directives: error: message: "Directive A cannot refer to itself." locations: [{line: 1, column: 25}] + - name: check reserved names on type name + input: | + directive @__A on FIELD_DEFINITION + error: + message: 'Name "__A" must not begin with "__", which is reserved by GraphQL introspection.' + locations: [{line: 1, column: 12}] entry points: - name: multiple schema entry points From fc7f38d6806ac8f9e20f5b70985c0da3768caeb3 Mon Sep 17 00:00:00 2001 From: vvakame Date: Thu, 22 Nov 2018 12:53:32 +0900 Subject: [PATCH 2/3] reproduce #73 --- validator/vars_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/validator/vars_test.go b/validator/vars_test.go index f625e66e..84576c0f 100644 --- a/validator/vars_test.go +++ b/validator/vars_test.go @@ -163,6 +163,18 @@ func TestValidateVars(t *testing.T) { }) require.EqualError(t, gerr, "input: variable.var[0].name must be defined") }) + t.Run("invalid variable paths", func(t *testing.T) { + q := gqlparser.MustLoadQuery(schema, `query foo($var1: InputType!, $var2: InputType!) { a:structArg(i: $var1) b:structArg(i: $var2) }`) + _, gerr := validator.VariableValues(schema, q.Operations.ForName(""), map[string]interface{}{ + "var1": map[string]interface{}{ + "name": "foobar", + }, + "var2": map[string]interface{}{ + "nullName": "foobar", + }, + }) + require.EqualError(t, gerr, "input: variable.var2.name must be defined") + }) }) t.Run("Scalars", func(t *testing.T) { From 6d09ed054088611823d73b85d88c83aad8a4fe93 Mon Sep 17 00:00:00 2001 From: vvakame Date: Thu, 22 Nov 2018 12:55:14 +0900 Subject: [PATCH 3/3] restore path variable in all cases --- validator/vars.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/validator/vars.go b/validator/vars.go index f8e0022c..aaf3a0d1 100644 --- a/validator/vars.go +++ b/validator/vars.go @@ -73,12 +73,19 @@ type varValidator struct { } func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerror.Error { + currentPath := v.path + resetPath := func() { + v.path = currentPath + } + defer resetPath() + if typ.Elem != nil { if val.Kind() != reflect.Slice { return gqlerror.ErrorPathf(v.path, "must be an array") } for i := 0; i < val.Len(); i++ { + resetPath() v.path = append(v.path, i) field := val.Index(i) @@ -92,8 +99,6 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr if err := v.validateVarType(typ.Elem, field); err != nil { return err } - - v.path = v.path[0 : len(v.path)-1] } return nil @@ -150,15 +155,16 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr for _, name := range val.MapKeys() { val.MapIndex(name) fieldDef := def.Fields.ForName(name.String()) + resetPath() v.path = append(v.path, name.String()) if fieldDef == nil { return gqlerror.ErrorPathf(v.path, "unknown field") } - v.path = v.path[0 : len(v.path)-1] } for _, fieldDef := range def.Fields { + resetPath() v.path = append(v.path, fieldDef.Name) field := val.MapIndex(reflect.ValueOf(fieldDef.Name)) @@ -184,8 +190,6 @@ func (v *varValidator) validateVarType(typ *ast.Type, val reflect.Value) *gqlerr if err != nil { return err } - - v.path = v.path[0 : len(v.path)-1] } default: panic(fmt.Errorf("unsupported type %s", def.Kind))