From ab0a719d66ea62026d5b5e30fdbb8c243d47a658 Mon Sep 17 00:00:00 2001 From: JatinDevDG Date: Wed, 27 Jan 2021 18:48:53 +0530 Subject: [PATCH 1/3] added fix and test --- graphql/e2e/common/error_test.yaml | 16 +++++++++++++++- graphql/schema/rules.go | 1 + graphql/schema/validation_rules.go | 26 +++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/graphql/e2e/common/error_test.yaml b/graphql/e2e/common/error_test.yaml index a2f538d63ce..f70cecb119f 100644 --- a/graphql/e2e/common/error_test.yaml +++ b/graphql/e2e/common/error_test.yaml @@ -348,4 +348,18 @@ { } errors: [ { "message": "Out of range value '2147483648', for type `Int`", - "locations": [ { "line": 2, "column": 53 } ] } ] \ No newline at end of file + "locations": [ { "line": 2, "column": 53 } ] } ] + +- name: "Error when multiple filter functions are used" + gqlrequest: | + query { + queryBook(filter:{bookId: {eq:2 le:2}}) + { + bookId + } + } + gqlvariables: | + { } + errors: + [ { "message": "Multiple filter functions for Int64Filter not allowed", + "locations": [ { "line": 2, "column": 29 } ] } ] \ No newline at end of file diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go index d20ce1c32f7..fad7edb74f8 100644 --- a/graphql/schema/rules.go +++ b/graphql/schema/rules.go @@ -47,6 +47,7 @@ func init() { validator.AddRule("Check arguments of cascade directive", directiveArgumentsCheck) validator.AddRule("Check range for Int type", intRangeCheck) validator.AddRule("Input Coercion to List", listInputCoercion) + validator.AddRule("Check filter functions", filterCheck) } diff --git a/graphql/schema/validation_rules.go b/graphql/schema/validation_rules.go index 129ca9e0e6c..e0cd2969297 100644 --- a/graphql/schema/validation_rules.go +++ b/graphql/schema/validation_rules.go @@ -24,12 +24,14 @@ import ( "github.com/dgraph-io/gqlparser/v2/validator" ) +var allowedFilters = []string{"StringHashFilter", "StringExactFilter", "StringFullTextFilter", + "StringRegExpFilter", "StringTermFilter", "DateTimeFilter", "FloatFilter", "Int64Filter", "IntFilter"} + func listInputCoercion(observers *validator.Events, addError validator.AddErrFunc) { observers.OnValue(func(walker *validator.Walker, value *ast.Value) { if value.Definition == nil || value.ExpectedType == nil { return } - if value.Kind == ast.Variable { return } @@ -51,6 +53,18 @@ func listInputCoercion(observers *validator.Events, addError validator.AddErrFun }) } +func filterCheck(observers *validator.Events, addError validator.AddErrFunc) { + observers.OnValue(func(walker *validator.Walker, value *ast.Value) { + if value.Definition == nil || value.ExpectedType == nil { + return + } + + if contains(allowedFilters, value.Definition.Name) && len(value.Children) > 1 { + addError(validator.Message("Multiple filter functions for %s not allowed", value.Definition.Name), validator.At(value.Position)) + } + }) +} + func variableTypeCheck(observers *validator.Events, addError validator.AddErrFunc) { observers.OnValue(func(walker *validator.Walker, value *ast.Value) { if value.Definition == nil || value.ExpectedType == nil || @@ -162,3 +176,13 @@ func valueKindToString(valKind ast.ValueKind) string { } return "" } + +func contains(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + + return false +} From 15d97acb195a6771e8456f3e3392ac0dacc16fcd Mon Sep 17 00:00:00 2001 From: JatinDevDG Date: Wed, 27 Jan 2021 19:01:53 +0530 Subject: [PATCH 2/3] clear code --- graphql/schema/validation_rules.go | 1 + 1 file changed, 1 insertion(+) diff --git a/graphql/schema/validation_rules.go b/graphql/schema/validation_rules.go index e0cd2969297..3eb0b2db636 100644 --- a/graphql/schema/validation_rules.go +++ b/graphql/schema/validation_rules.go @@ -32,6 +32,7 @@ func listInputCoercion(observers *validator.Events, addError validator.AddErrFun if value.Definition == nil || value.ExpectedType == nil { return } + if value.Kind == ast.Variable { return } From 17a63095106d8c148f962f188207490d160ddcad Mon Sep 17 00:00:00 2001 From: JatinDevDG Date: Wed, 27 Jan 2021 19:51:51 +0530 Subject: [PATCH 3/3] addressed abhimanyu's comments --- graphql/e2e/common/error_test.yaml | 2 +- graphql/schema/validation_rules.go | 20 ++++++-------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/graphql/e2e/common/error_test.yaml b/graphql/e2e/common/error_test.yaml index f70cecb119f..6e75208ec8c 100644 --- a/graphql/e2e/common/error_test.yaml +++ b/graphql/e2e/common/error_test.yaml @@ -361,5 +361,5 @@ gqlvariables: | { } errors: - [ { "message": "Multiple filter functions for Int64Filter not allowed", + [ { "message": "Int64Filter filter expects only one filter function, got: 2", "locations": [ { "line": 2, "column": 29 } ] } ] \ No newline at end of file diff --git a/graphql/schema/validation_rules.go b/graphql/schema/validation_rules.go index 3eb0b2db636..15a1328cfc0 100644 --- a/graphql/schema/validation_rules.go +++ b/graphql/schema/validation_rules.go @@ -18,6 +18,7 @@ package schema import ( "errors" + "github.com/dgraph-io/dgraph/x" "strconv" "github.com/dgraph-io/gqlparser/v2/ast" @@ -25,7 +26,8 @@ import ( ) var allowedFilters = []string{"StringHashFilter", "StringExactFilter", "StringFullTextFilter", - "StringRegExpFilter", "StringTermFilter", "DateTimeFilter", "FloatFilter", "Int64Filter", "IntFilter"} + "StringRegExpFilter", "StringTermFilter", "DateTimeFilter", "FloatFilter", "Int64Filter", "IntFilter", "PointGeoFilter", + "ContainsFilter", "IntersectsFilter", "PolygonGeoFilter"} func listInputCoercion(observers *validator.Events, addError validator.AddErrFunc) { observers.OnValue(func(walker *validator.Walker, value *ast.Value) { @@ -56,12 +58,12 @@ func listInputCoercion(observers *validator.Events, addError validator.AddErrFun func filterCheck(observers *validator.Events, addError validator.AddErrFunc) { observers.OnValue(func(walker *validator.Walker, value *ast.Value) { - if value.Definition == nil || value.ExpectedType == nil { + if value.Definition == nil { return } - if contains(allowedFilters, value.Definition.Name) && len(value.Children) > 1 { - addError(validator.Message("Multiple filter functions for %s not allowed", value.Definition.Name), validator.At(value.Position)) + if x.HasString(allowedFilters, value.Definition.Name) && len(value.Children) > 1 { + addError(validator.Message("%s filter expects only one filter function, got: %d", value.Definition.Name, len(value.Children)), validator.At(value.Position)) } }) } @@ -177,13 +179,3 @@ func valueKindToString(valKind ast.ValueKind) string { } return "" } - -func contains(s []string, str string) bool { - for _, v := range s { - if v == str { - return true - } - } - - return false -}