From 1071863808be96f270968b09af82d9d58441dfea Mon Sep 17 00:00:00 2001 From: srfrog Date: Fri, 8 Feb 2019 12:06:49 -0700 Subject: [PATCH 1/7] query/query.go: default value should not be nil types.Marshal expects vars to have non-nil values, this change set empty string as the default value for a referenced aggregate value. This should prevent nil-reference panic in types.Marshal. --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index 39c63cd4f46..2673c4d5f41 100644 --- a/query/query.go +++ b/query/query.go @@ -1681,7 +1681,7 @@ func (sg *SubGraph) fillVars(mp map[string]varValue) error { // Provide a default value for valueVarAggregation() to eval val(). // This is a noop for aggregation funcs that would fail. // The zero aggs won't show because there are no uids matched. - mp[v.Name].Vals[0] = types.Val{} + mp[v.Name].Vals[0] = types.Val{Value: ""} sg.Params.uidToVal = mp[v.Name].Vals } } From 06b1de8da14c992152c9cb2d39b52526e623d5a6 Mon Sep 17 00:00:00 2001 From: srfrog Date: Fri, 8 Feb 2019 14:54:40 -0700 Subject: [PATCH 2/7] query/query.go: retract default value change it breaks aggs. Value var aggregations use nil values for an optimization, so don't assign a value to defaults. --- query/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/query/query.go b/query/query.go index 2673c4d5f41..39c63cd4f46 100644 --- a/query/query.go +++ b/query/query.go @@ -1681,7 +1681,7 @@ func (sg *SubGraph) fillVars(mp map[string]varValue) error { // Provide a default value for valueVarAggregation() to eval val(). // This is a noop for aggregation funcs that would fail. // The zero aggs won't show because there are no uids matched. - mp[v.Name].Vals[0] = types.Val{Value: ""} + mp[v.Name].Vals[0] = types.Val{} sg.Params.uidToVal = mp[v.Name].Vals } } From 68d65e90fa79dd66f9f94fdf3d1fd4fcaf640788 Mon Sep 17 00:00:00 2001 From: srfrog Date: Fri, 8 Feb 2019 14:55:51 -0700 Subject: [PATCH 3/7] types/conversion.go: add exception in Marshal for default->string The marshaling of default value vars to string causes panic because the incoming value is nil. This affects a narrow case so we check that in fact it's a default value var and marshal to empty string. In theory nothing should be nil by the time we Marshal, but it's an edge case. --- types/conversion.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/types/conversion.go b/types/conversion.go index 31038b2f2ea..456d320ee33 100644 --- a/types/conversion.go +++ b/types/conversion.go @@ -353,6 +353,12 @@ func Marshal(from Val, to *Val) error { return cantConvert(fromID, toID) } case StringID, DefaultID: + // This is a default value from sg.fillVars, don't convert it's empty. + // Fixes issue #2295. + if fromID == DefaultID && val == nil { + *res = "" + break + } vc := val.(string) switch toID { case StringID, DefaultID: From a1128e543a232c1f9262c7f2c21f416908de1419 Mon Sep 17 00:00:00 2001 From: srfrog Date: Fri, 8 Feb 2019 15:14:08 -0700 Subject: [PATCH 4/7] wrong issue referenced in comments --- types/conversion.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/conversion.go b/types/conversion.go index 456d320ee33..8110c474189 100644 --- a/types/conversion.go +++ b/types/conversion.go @@ -354,7 +354,7 @@ func Marshal(from Val, to *Val) error { } case StringID, DefaultID: // This is a default value from sg.fillVars, don't convert it's empty. - // Fixes issue #2295. + // Fixes issue #2980. if fromID == DefaultID && val == nil { *res = "" break From 4501a71fafa29cf8fc723465a2bc39c48af2fc63 Mon Sep 17 00:00:00 2001 From: srfrog Date: Mon, 11 Feb 2019 19:02:35 -0700 Subject: [PATCH 5/7] types/conversion.go: general case in nil value mashaling If we are trying to mashal a nil value, return the zero value instead of crashing on nil reference. --- types/conversion.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/types/conversion.go b/types/conversion.go index 8110c474189..75a9533a5e9 100644 --- a/types/conversion.go +++ b/types/conversion.go @@ -341,6 +341,13 @@ func Marshal(from Val, to *Val) error { val := from.Value res := &to.Value + // This is a default value from sg.fillVars, don't convert it's empty. + // Fixes issue #2980. + if val == nil { + *res = ValueForType(toID).Value + return nil + } + switch fromID { case BinaryID: vc := val.([]byte) @@ -353,12 +360,6 @@ func Marshal(from Val, to *Val) error { return cantConvert(fromID, toID) } case StringID, DefaultID: - // This is a default value from sg.fillVars, don't convert it's empty. - // Fixes issue #2980. - if fromID == DefaultID && val == nil { - *res = "" - break - } vc := val.(string) switch toID { case StringID, DefaultID: From 61b9f90b9f2b643a2bf8ace0591e46e5d70cd5b6 Mon Sep 17 00:00:00 2001 From: srfrog Date: Mon, 11 Feb 2019 19:06:07 -0700 Subject: [PATCH 6/7] types/conversion.go: replace the original to-value --- types/conversion.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/conversion.go b/types/conversion.go index 75a9533a5e9..e45e2e44792 100644 --- a/types/conversion.go +++ b/types/conversion.go @@ -344,7 +344,7 @@ func Marshal(from Val, to *Val) error { // This is a default value from sg.fillVars, don't convert it's empty. // Fixes issue #2980. if val == nil { - *res = ValueForType(toID).Value + *to = ValueForType(toID) return nil } From aeb058dbf89445baebe70da2bed70646ec60f0df Mon Sep 17 00:00:00 2001 From: srfrog Date: Wed, 13 Feb 2019 17:51:35 -0700 Subject: [PATCH 7/7] types/conversion.go: added check for Marshal to nil object --- types/conversion.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/types/conversion.go b/types/conversion.go index e45e2e44792..7232252ac9d 100644 --- a/types/conversion.go +++ b/types/conversion.go @@ -336,6 +336,10 @@ func Convert(from Val, toID TypeID) (Val, error) { } func Marshal(from Val, to *Val) error { + if to == nil { + return x.Errorf("Invalid conversion %s to nil", from.Tid.Name()) + } + fromID := from.Tid toID := to.Tid val := from.Value