Skip to content

Commit

Permalink
fix: GraphQL null argument parsing (#3013)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #3012

## Description

This PR fixes an issue where explicitly null GQL inputs would cause a
panic.

## Tasks

- [x] I made sure the code is well commented, particularly
hard-to-understand areas.
- [x] I made sure the repository-held documentation is changed
accordingly.
- [x] I made sure the pull request title adheres to the conventional
commit style (the subset used in the project can be found in
[tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)).
- [x] I made sure to discuss its limitations such as threats to
validity, vulnerability to mistake and misuse, robustness to
invalidation of assumptions, resource requirements, ...

## How has this been tested?

Added tests

Specify the platform(s) on which this was tested:
- MacOS
  • Loading branch information
nasdf authored Sep 18, 2024
1 parent 2675b2e commit 36eacbe
Show file tree
Hide file tree
Showing 11 changed files with 1,330 additions and 164 deletions.
16 changes: 10 additions & 6 deletions internal/planner/mapper/targetable.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,17 @@ func filterObjectToMap(mapping *core.DocumentMapping, obj map[connor.FilterKey]a
for k, v := range obj {
switch keyType := k.(type) {
case *PropertyIndex:
subObj := v.(map[connor.FilterKey]any)
outkey, _ := mapping.TryToFindNameFromIndex(keyType.Index)
childMapping, ok := tryGetChildMapping(mapping, keyType.Index)
if ok {
outmap[outkey] = filterObjectToMap(childMapping, subObj)
} else {
outmap[outkey] = filterObjectToMap(mapping, subObj)
switch subObj := v.(type) {
case map[connor.FilterKey]any:
childMapping, ok := tryGetChildMapping(mapping, keyType.Index)
if ok {
outmap[outkey] = filterObjectToMap(childMapping, subObj)
} else {
outmap[outkey] = filterObjectToMap(mapping, subObj)
}
case nil:
outmap[outkey] = nil
}

case *Operator:
Expand Down
71 changes: 52 additions & 19 deletions internal/request/graphql/parser/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,61 @@ func parseCommitSelect(
arguments := gql.GetArgumentValues(fieldDef.Args, field.Arguments, exe.VariableValues)

for _, argument := range field.Arguments {
prop := argument.Name.Value
if prop == request.DocIDArgName {
commit.DocID = immutable.Some(arguments[prop].(string))
} else if prop == request.Cid {
commit.CID = immutable.Some(arguments[prop].(string))
} else if prop == request.FieldIDName {
commit.FieldID = immutable.Some(arguments[prop].(string))
} else if prop == request.OrderClause {
conditions, err := ParseConditionsInOrder(argument.Value.(*ast.ObjectValue), arguments[prop].(map[string]any))
name := argument.Name.Value
value := arguments[name]

switch name {
case request.DocIDArgName:
if v, ok := value.(string); ok {
commit.DocID = immutable.Some(v)
}

case request.Cid:
if v, ok := value.(string); ok {
commit.CID = immutable.Some(v)
}

case request.FieldIDName:
if v, ok := value.(string); ok {
commit.FieldID = immutable.Some(v)
}

case request.OrderClause:
v, ok := value.(map[string]any)
if !ok {
continue // value is nil
}
conditions, err := ParseConditionsInOrder(argument.Value.(*ast.ObjectValue), v)
if err != nil {
return nil, err
}
commit.OrderBy = immutable.Some(request.OrderBy{
Conditions: conditions,
})
} else if prop == request.LimitClause {
commit.Limit = immutable.Some(uint64(arguments[prop].(int32)))
} else if prop == request.OffsetClause {
commit.Offset = immutable.Some(uint64(arguments[prop].(int32)))
} else if prop == request.DepthClause {
commit.Depth = immutable.Some(uint64(arguments[prop].(int32)))
} else if prop == request.GroupByClause {
fields := []string{}
for _, v := range arguments[prop].([]any) {
fields = append(fields, v.(string))

case request.LimitClause:
if v, ok := value.(int32); ok {
commit.Limit = immutable.Some(uint64(v))
}

case request.OffsetClause:
if v, ok := value.(int32); ok {
commit.Offset = immutable.Some(uint64(v))
}

case request.DepthClause:
if v, ok := value.(int32); ok {
commit.Depth = immutable.Some(uint64(v))
}

case request.GroupByClause:
v, ok := value.([]any)
if !ok {
continue // value is nil
}
fields := make([]string, len(v))
for i, c := range v {
fields[i] = c.(string)
}
commit.GroupBy = immutable.Some(request.GroupBy{
Fields: fields,
Expand Down Expand Up @@ -91,6 +121,9 @@ func parseCommitSelect(
}

commit.Fields, err = parseSelectFields(exe, fieldObject, field.SelectionSet)
if err != nil {
return nil, err
}

return commit, err
}
29 changes: 19 additions & 10 deletions internal/request/graphql/parser/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,9 @@ func ParseConditionsInOrder(stmt *ast.ObjectValue, args map[string]any) ([]reque
for _, field := range stmt.Fields {
switch v := args[field.Name.Value].(type) {
case int: // base direction parsed (hopefully, check NameToOrderDirection)
var dir request.OrderDirection
switch v {
case 0:
dir = request.ASC

case 1:
dir = request.DESC

default:
return nil, ErrInvalidOrderDirection
dir, err := parseOrderDirection(v)
if err != nil {
return nil, err
}
conditions = append(conditions, request.OrderCondition{
Fields: []string{field.Name.Value},
Expand All @@ -109,6 +102,9 @@ func ParseConditionsInOrder(stmt *ast.ObjectValue, args map[string]any) ([]reque
conditions = append(conditions, cond)
}

case nil:
continue // ignore nil filter input

default:
return nil, client.NewErrUnhandledType("parseConditionInOrder", v)
}
Expand Down Expand Up @@ -199,3 +195,16 @@ func parseFilterFieldsForDescriptionSlice(
}
return fields, nil
}

func parseOrderDirection(v int) (request.OrderDirection, error) {
switch v {
case 0:
return request.ASC, nil

case 1:
return request.DESC, nil

default:
return request.ASC, ErrInvalidOrderDirection
}
}
76 changes: 51 additions & 25 deletions internal/request/graphql/parser/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,38 +95,60 @@ func parseMutation(exe *gql.ExecutionContext, parent *gql.Object, field *ast.Fie
mut.Collection = strings.Join(mutNameParts[1:], "_")
}

// parse arguments
for _, argument := range field.Arguments {
prop := argument.Name.Value
// parse each individual arg type seperately
if prop == request.Input { // parse input
mut.Input = arguments[prop].(map[string]any)
} else if prop == request.Inputs {
inputsValue := arguments[prop].([]any)
inputs := make([]map[string]any, len(inputsValue))
for i, v := range inputsValue {
name := argument.Name.Value
value := arguments[name]

switch name {
case request.Input:
if v, ok := value.(map[string]any); ok {
mut.Input = v
}

case request.Inputs:
v, ok := value.([]any)
if !ok {
continue // value is nil
}
inputs := make([]map[string]any, len(v))
for i, v := range v {
inputs[i] = v.(map[string]any)
}
mut.Inputs = inputs
} else if prop == request.FilterClause { // parse filter
mut.Filter = immutable.Some(request.Filter{
Conditions: arguments[prop].(map[string]any),
})
} else if prop == request.DocIDArgName {
mut.DocIDs = immutable.Some([]string{arguments[prop].(string)})
} else if prop == request.DocIDsArgName {
docIDsValue := arguments[prop].([]any)
docIDs := make([]string, len(docIDsValue))
for i, v := range docIDsValue {

case request.FilterClause:
if v, ok := value.(map[string]any); ok {
mut.Filter = immutable.Some(request.Filter{Conditions: v})
}

case request.DocIDArgName:
if v, ok := value.(string); ok {
mut.DocIDs = immutable.Some([]string{v})
}

case request.DocIDsArgName:
v, ok := value.([]any)
if !ok {
continue // value is nil
}
docIDs := make([]string, len(v))
for i, v := range v {
docIDs[i] = v.(string)
}
mut.DocIDs = immutable.Some(docIDs)
} else if prop == request.EncryptDocArgName {
mut.Encrypt = arguments[prop].(bool)
} else if prop == request.EncryptFieldsArgName {
fieldsValue := arguments[prop].([]any)
fields := make([]string, len(fieldsValue))
for i, v := range fieldsValue {

case request.EncryptDocArgName:
if v, ok := value.(bool); ok {
mut.Encrypt = v
}

case request.EncryptFieldsArgName:
v, ok := value.([]any)
if !ok {
continue // value is nil
}
fields := make([]string, len(v))
for i, v := range v {
fields[i] = v.(string)
}
mut.EncryptFields = fields
Expand All @@ -144,5 +166,9 @@ func parseMutation(exe *gql.ExecutionContext, parent *gql.Object, field *ast.Fie
}

mut.Fields, err = parseSelectFields(exe, fieldObject, field.SelectionSet)
if err != nil {
return nil, err
}

return mut, err
}
Loading

0 comments on commit 36eacbe

Please sign in to comment.