Skip to content

Commit

Permalink
internal/jsonutil: Skip unexported fields.
Browse files Browse the repository at this point in the history
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 #36.
  • Loading branch information
shuheiktgw authored and dmitshur committed Dec 31, 2018
1 parent 16b8864 commit d48a9a7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
15 changes: 9 additions & 6 deletions internal/jsonutil/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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())
}
13 changes: 13 additions & 0 deletions internal/jsonutil/graphql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d48a9a7

Please sign in to comment.