Permalink
Browse files

internal/jsonutil: Skip unexported fields.

This change makes jsonutil.UnmarshalGraphQL not consider unexported fields when looking for a matching field.

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

Fixes shurcooL#36.
  • Loading branch information...
dmitshur committed Dec 29, 2018
1 parent 37c5479 commit 60e006b4497879c94c872c8dc547d6616fb6e0b5
Showing with 22 additions and 22 deletions.
  1. +9 −9 internal/jsonutil/graphql.go
  2. +13 −13 internal/jsonutil/graphql_test.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,16 +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)
}
if !v.CanInterface() {
return errors.New("struct contains an unexported field")
}
return json.Unmarshal(b, v.Addr().Interface())
}
@@ -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
@@ -380,16 +393,3 @@ func TestUnmarshalGraphQL_arrayInsideInlineFragment(t *testing.T) {
t.Error("not equal")
}
}

func TestUnmarshalGraphQL_unexportedField(t *testing.T) {
type query struct {
foo graphql.String
}
err := jsonutil.UnmarshalGraphQL([]byte(`{"foo": "bar"}{"foo": "baz"}`), new(query))
if err == nil {
t.Fatal("got error: nil, want: non-nil")
}
if got, want := err.Error(), "struct contains an unexported field"; got != want {
t.Errorf("got error: %v, want: %v", got, want)
}
}

0 comments on commit 60e006b

Please sign in to comment.