From 0eb694e9c217c051cd8cc18258bf593d0be7fb8d Mon Sep 17 00:00:00 2001
From: Matthew Dempsky <mdempsky@google.com>
Date: Sun, 19 Apr 2020 13:59:16 -0700
Subject: [PATCH] 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>
---
 doc/go1.15.html         | 12 ++++++++++++
 src/reflect/all_test.go |  4 ++--
 src/reflect/value.go    |  2 +-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/doc/go1.15.html b/doc/go1.15.html
index bb5628cb19a..e2c90f5ad21 100644
--- 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 cb0c8344f3e..3129ff8e5d1 100644
--- 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 57ac65e0842..de6f22b5b3e 100644
--- 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}