reflect: disallow invoking methods on unexported embedded fields

Given:

    type u struct{}
    func (u) M() {}

    type t struct { u; u2 u }

    var v = reflect.ValueOf(t{})

Package reflect allows:

    v.Method(0)          // v.M
    v.Field(0).Method(0) // v.u.M

but panics from:

    v.Field(1).Method(0) // v.u2.M

because u2 is not an exported field. However, u is not an exported
field either, so this is inconsistent.

It seems like this behavior originates from #12367, where it was
decided to allow traversing unexported embedded fields to be able to
access their exported fields, since package reflect doesn't provide an
alternative way to access promoted fields directly.

But extending that logic to promoted *methods* was inappropriate,
because package reflect's normal method handling logic already handles
promoted methods correctly. This CL corrects that mistake.

Fixes #38521.

Change-Id: If65008965f35927b4e7927cddf8614695288eb19
Reviewed-on: https://go-review.googlesource.com/c/go/+/228902
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/doc/go1.15.html b/doc/go1.15.html
index bb5628cb19a10dd1d2fbed4d6459f18a87c54e95..e2c90f5ad21ba081fcecc58e4ba6dfe45b8481ad 100644 (file)
--- a/doc/go1.15.html
+++ b/doc/go1.15.html
@@ -177,6 +177,18 @@ TODO
   </dd>
 </dl>
 
+<dl id="reflect"><dt><a href="/pkg/reflect/">reflect</a></dt>
+  <dd>
+    <p><!-- CL 228902 -->
+      Package reflect now disallows accessing methods of all
+      non-exported fields, whereas previously it allowed accessing
+      those of non-exported, embedded fields. Code that relies on the
+      previous behavior should be updated to instead access the
+      corresponding promoted method of the enclosing variable.
+    </p>
+  </dd>
+</dl>
+
 <dl id="pkg-runtime"><dt><a href="/pkg/runtime/">runtime</a></dt>
   <dd>
     <p><!-- CL 221779 -->

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index cb0c8344f3e3b8b28980eebccefd917c340bc07b..3129ff8e5d11cc63445397a0b7dedeb8f8df6a65 100644 (file)
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -3567,7 +3567,7 @@ func TestCallPanic(t *testing.T) {
 
        i := timp(0)
        v := ValueOf(T{i, i, i, i, T2{i, i}, i, i, T2{i, i}})
-       ok(func() { call(v.Field(0).Method(0)) })               // .t0.W
+       badCall(func() { call(v.Field(0).Method(0)) })          // .t0.W
        badCall(func() { call(v.Field(0).Elem().Method(0)) })   // .t0.W
        badCall(func() { call(v.Field(0).Method(1)) })          // .t0.w
        badMethod(func() { call(v.Field(0).Elem().Method(2)) }) // .t0.w
@@ -3588,7 +3588,7 @@ func TestCallPanic(t *testing.T) {
 
        ok(func() { call(v.Field(4).Field(0).Method(0)) })             // .NamedT2.T1.Y
        ok(func() { call(v.Field(4).Field(0).Elem().Method(0)) })      // .NamedT2.T1.W
-       ok(func() { call(v.Field(4).Field(1).Method(0)) })             // .NamedT2.t0.W
+       badCall(func() { call(v.Field(4).Field(1).Method(0)) })        // .NamedT2.t0.W
        badCall(func() { call(v.Field(4).Field(1).Elem().Method(0)) }) // .NamedT2.t0.W
 
        badCall(func() { call(v.Field(5).Method(0)) })          // .namedT0.W

diff --git a/src/reflect/value.go b/src/reflect/value.go
index 57ac65e0842c7362dea0d825cb4510d69a4fec2d..de6f22b5b3e24283a0f879c07d0ff853fe60f253 100644 (file)
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -1315,7 +1315,7 @@ func (v Value) Method(i int) Value {
        if v.typ.Kind() == Interface && v.IsNil() {
                panic("reflect: Method on nil interface value")
        }
-       fl := v.flag & (flagStickyRO | flagIndir) // Clear flagEmbedRO
+       fl := v.flag.ro() | (v.flag & flagIndir)
        fl |= flag(Func)
        fl |= flag(i)<<flagMethodShift | flagMethod
        return Value{v.typ, v.ptr, fl}