internal/jsonutil: Skip unexported fields.

This change makes jsonutil.UnmarshalGraphQL not consider unexported
fields when looking for a matching field. This is done because such
fields cannot be unmarshaled into, and it's more consistent with
behavior of package encoding/json.

Document unmarshalValue precondition that v must be addressing and
not obtained by the use of unexported fields. That would make it
settable, which is a requirement for the needs of unmarshalValue.
We arrange the internal jsonutil code so that unmarshalValue is
never called on an unsettable reflect.Value.

Fixes hasura#36.

Author: shuheiktgw

internal/jsonutil/graphql.go

@@ -102,7 +102,7 @@ func (d *decoder) decode() error {
 				d.vs[i] = append(d.vs[i], f)
 			}
 			if !someFieldExist {
-				return fmt.Errorf("struct field for %s doesn't exist in any of %v places to unmarshal", key, len(d.vs))
+				return fmt.Errorf("struct field for %q doesn't exist in any of %v places to unmarshal", key, len(d.vs))
 			}
 
 			// We've just consumed the current token, which was the key.
@@ -252,10 +252,14 @@ func (d *decoder) popAllVs() {
 	d.vs = nonEmpty
 }
 
-// fieldByGraphQLName returns a struct field of struct v that matches GraphQL name,
-// or invalid reflect.Value if none found.
+// fieldByGraphQLName returns an exported struct field of struct v
+// that matches GraphQL name, or invalid reflect.Value if none found.
 func fieldByGraphQLName(v reflect.Value, name string) reflect.Value {
 	for i := 0; i < v.NumField(); i++ {
+		if v.Type().Field(i).PkgPath != "" {
+			// Skip unexported field.
+			continue
+		}
 		if hasGraphQLName(v.Type().Field(i), name) {
 			return v.Field(i)
 		}
@@ -296,13 +300,12 @@ func isGraphQLFragment(f reflect.StructField) bool {
 }
 
 // unmarshalValue unmarshals JSON value into v.
+// v must be addressable and not obtained by the use of unexported
+// struct fields, otherwise unmarshalValue will panic.
 func unmarshalValue(value json.Token, v reflect.Value) error {
 	b, err := json.Marshal(value) // TODO: Short-circuit (if profiling says it's worth it).
 	if err != nil {
 		return err
 	}
-	if !v.CanAddr() {
-		return fmt.Errorf("value %v is not addressable", v)
-	}
 	return json.Unmarshal(b, v.Addr().Interface())
 }

internal/jsonutil/graphql_test.go

@@ -241,6 +241,19 @@ func TestUnmarshalGraphQL_pointerWithInlineFragment(t *testing.T) {
 	}
 }
 
+func TestUnmarshalGraphQL_unexportedField(t *testing.T) {
+	type query struct {
+		foo graphql.String
+	}
+	err := jsonutil.UnmarshalGraphQL([]byte(`{"foo": "bar"}`), new(query))
+	if err == nil {
+		t.Fatal("got error: nil, want: non-nil")
+	}
+	if got, want := err.Error(), "struct field for \"foo\" doesn't exist in any of 1 places to unmarshal"; got != want {
+		t.Errorf("got error: %v, want: %v", got, want)
+	}
+}
+
 func TestUnmarshalGraphQL_multipleValues(t *testing.T) {
 	type query struct {
 		Foo graphql.String