reflect: disallow Interface method on Value obtained via unexported name

Had been allowing it for use by fmt, but it is too hard to lock down.
Fix other packages not to depend on it.

R=r, r
CC=golang-dev
https://golang.org/cl/5266054

src/pkg/fmt/fmt_test.go

@@ -62,7 +62,7 @@ type I int
 func (i I) String() string { return Sprintf("<%d>", int(i)) }
 
 type B struct {
-	i I
+	I I
 	j int
 }
 
@@ -84,8 +84,8 @@ func (g G) GoString() string {
 }
 
 type S struct {
-	f F // a struct field that Formats
-	g G // a struct field that GoStrings
+	F F // a struct field that Formats
+	G G // a struct field that GoStrings
 }
 
 // A type with a String method with pointer receiver for testing %p
@@ -333,8 +333,8 @@ var fmttests = []struct {
 	{"%+v", A{1, 2, "a", []int{1, 2}}, `{i:1 j:2 s:a x:[1 2]}`},
 
 	// +v on structs with Stringable items
-	{"%+v", B{1, 2}, `{i:<1> j:2}`},
-	{"%+v", C{1, B{2, 3}}, `{i:1 B:{i:<2> j:3}}`},
+	{"%+v", B{1, 2}, `{I:<1> j:2}`},
+	{"%+v", C{1, B{2, 3}}, `{i:1 B:{I:<2> j:3}}`},
 
 	// q on Stringable items
 	{"%s", I(23), `<23>`},
@@ -350,7 +350,7 @@ var fmttests = []struct {
 	{"%#v", uint64(1<<64 - 1), "0xffffffffffffffff"},
 	{"%#v", 1000000000, "1000000000"},
 	{"%#v", map[string]int{"a": 1, "b": 2}, `map[string] int{"a":1, "b":2}`},
-	{"%#v", map[string]B{"a": {1, 2}, "b": {3, 4}}, `map[string] fmt_test.B{"a":fmt_test.B{i:1, j:2}, "b":fmt_test.B{i:3, j:4}}`},
+	{"%#v", map[string]B{"a": {1, 2}, "b": {3, 4}}, `map[string] fmt_test.B{"a":fmt_test.B{I:1, j:2}, "b":fmt_test.B{I:3, j:4}}`},
 	{"%#v", []string{"a", "b"}, `[]string{"a", "b"}`},
 
 	// slices with other formats
@@ -385,11 +385,11 @@ var fmttests = []struct {
 	// Formatter
 	{"%x", F(1), "<x=F(1)>"},
 	{"%x", G(2), "2"},
-	{"%+v", S{F(4), G(5)}, "{f:<v=F(4)> g:5}"},
+	{"%+v", S{F(4), G(5)}, "{F:<v=F(4)> G:5}"},
 
 	// GoStringer
 	{"%#v", G(6), "GoString(6)"},
-	{"%#v", S{F(7), G(8)}, "fmt_test.S{f:<v=F(7)>, g:GoString(8)}"},
+	{"%#v", S{F(7), G(8)}, "fmt_test.S{F:<v=F(7)>, G:GoString(8)}"},
 
 	// %T
 	{"%T", (4 - 3i), "complex128"},

src/pkg/reflect/all_test.go

@@ -853,13 +853,13 @@ func TestIsNil(t *testing.T) {
 
 func TestInterfaceExtraction(t *testing.T) {
 	var s struct {
-		w io.Writer
+		W io.Writer
 	}
 
-	s.w = os.Stdout
+	s.W = os.Stdout
 	v := Indirect(ValueOf(&s)).Field(0).Interface()
-	if v != s.w.(interface{}) {
-		t.Error("Interface() on interface: ", v, s.w)
+	if v != s.W.(interface{}) {
+		t.Error("Interface() on interface: ", v, s.W)
 	}
 }
 
@@ -1190,18 +1190,18 @@ type D2 struct {
 }
 
 type S0 struct {
-	a, b, c int
+	A, B, C int
 	D1
 	D2
 }
 
 type S1 struct {
-	b int
+	B int
 	S0
 }
 
 type S2 struct {
-	a int
+	A int
 	*S1
 }
 
@@ -1216,36 +1216,36 @@ type S1y struct {
 type S3 struct {
 	S1x
 	S2
-	d, e int
+	D, E int
 	*S1y
 }
 
 type S4 struct {
 	*S4
-	a int
+	A int
 }
 
 var fieldTests = []FTest{
 	{struct{}{}, "", nil, 0},
-	{struct{}{}, "foo", nil, 0},
-	{S0{a: 'a'}, "a", []int{0}, 'a'},
-	{S0{}, "d", nil, 0},
-	{S1{S0: S0{a: 'a'}}, "a", []int{1, 0}, 'a'},
-	{S1{b: 'b'}, "b", []int{0}, 'b'},
+	{struct{}{}, "Foo", nil, 0},
+	{S0{A: 'a'}, "A", []int{0}, 'a'},
+	{S0{}, "D", nil, 0},
+	{S1{S0: S0{A: 'a'}}, "A", []int{1, 0}, 'a'},
+	{S1{B: 'b'}, "B", []int{0}, 'b'},
 	{S1{}, "S0", []int{1}, 0},
-	{S1{S0: S0{c: 'c'}}, "c", []int{1, 2}, 'c'},
-	{S2{a: 'a'}, "a", []int{0}, 'a'},
+	{S1{S0: S0{C: 'c'}}, "C", []int{1, 2}, 'c'},
+	{S2{A: 'a'}, "A", []int{0}, 'a'},
 	{S2{}, "S1", []int{1}, 0},
-	{S2{S1: &S1{b: 'b'}}, "b", []int{1, 0}, 'b'},
-	{S2{S1: &S1{S0: S0{c: 'c'}}}, "c", []int{1, 1, 2}, 'c'},
-	{S2{}, "d", nil, 0},
+	{S2{S1: &S1{B: 'b'}}, "B", []int{1, 0}, 'b'},
+	{S2{S1: &S1{S0: S0{C: 'c'}}}, "C", []int{1, 1, 2}, 'c'},
+	{S2{}, "D", nil, 0},
 	{S3{}, "S1", nil, 0},
-	{S3{S2: S2{a: 'a'}}, "a", []int{1, 0}, 'a'},
-	{S3{}, "b", nil, 0},
-	{S3{d: 'd'}, "d", []int{2}, 0},
-	{S3{e: 'e'}, "e", []int{3}, 'e'},
-	{S4{a: 'a'}, "a", []int{1}, 'a'},
-	{S4{}, "b", nil, 0},
+	{S3{S2: S2{A: 'a'}}, "A", []int{1, 0}, 'a'},
+	{S3{}, "B", nil, 0},
+	{S3{D: 'd'}, "D", []int{2}, 0},
+	{S3{E: 'e'}, "E", []int{3}, 'e'},
+	{S4{A: 'a'}, "A", []int{1}, 'a'},
+	{S4{}, "B", nil, 0},
 }
 
 func TestFieldByIndex(t *testing.T) {
@@ -1587,3 +1587,68 @@ func TestSetBytes(t *testing.T) {
 		t.Errorf("ValueOf(%p).Bytes() = %p", &x[0], &y[0])
 	}
 }
+
+type Private struct {
+	x int
+	y **int
+}
+
+func (p *Private) m() {
+}
+
+type Public struct {
+	X int
+	Y **int
+}
+
+func (p *Public) M() {
+}
+
+func TestUnexported(t *testing.T) {
+	var pub Public
+	v := ValueOf(&pub)
+	isValid(v.Elem().Field(0))
+	isValid(v.Elem().Field(1))
+	isValid(v.Elem().FieldByName("X"))
+	isValid(v.Elem().FieldByName("Y"))
+	isValid(v.Type().Method(0).Func)
+	isNonNil(v.Elem().Field(0).Interface())
+	isNonNil(v.Elem().Field(1).Interface())
+	isNonNil(v.Elem().FieldByName("X").Interface())
+	isNonNil(v.Elem().FieldByName("Y").Interface())
+	isNonNil(v.Type().Method(0).Func.Interface())
+
+	var priv Private
+	v = ValueOf(&priv)
+	isValid(v.Elem().Field(0))
+	isValid(v.Elem().Field(1))
+	isValid(v.Elem().FieldByName("x"))
+	isValid(v.Elem().FieldByName("y"))
+	isValid(v.Type().Method(0).Func)
+	shouldPanic(func() { v.Elem().Field(0).Interface() })
+	shouldPanic(func() { v.Elem().Field(1).Interface() })
+	shouldPanic(func() { v.Elem().FieldByName("x").Interface() })
+	shouldPanic(func() { v.Elem().FieldByName("y").Interface() })
+	shouldPanic(func() { v.Type().Method(0).Func.Interface() })
+}
+
+func shouldPanic(f func()) {
+	defer func() {
+		if recover() == nil {
+			panic("did not panic")
+		}
+	}()
+	f()
+}
+
+func isNonNil(x interface{}) {
+	if x == nil {
+		panic("nil interface")
+	}
+}
+
+func isValid(v Value) {
+	if !v.IsValid() {
+		panic("zero Value")
+	}
+}

src/pkg/reflect/deepequal.go

@@ -104,7 +104,7 @@ func deepValueEqual(v1, v2 Value, visited map[uintptr]*visit, depth int) (b bool
 		return true
 	default:
 		// Normal equality suffices
-		return v1.Interface() == v2.Interface()
+		return valueInterface(v1, false) == valueInterface(v2, false)
 	}
 
 	panic("Not reached")

src/pkg/reflect/value.go

@@ -876,14 +876,7 @@ func (v Value) CanInterface() bool {
 	if iv.kind == Invalid {
 		panic(&ValueError{"reflect.Value.CanInterface", iv.kind})
 	}
-	// TODO(rsc): Check flagRO too.  Decide what to do about asking for
-	// interface for a value obtained via an unexported field.
-	// If the field were of a known type, say chan int or *sync.Mutex,
-	// the caller could interfere with the data after getting the
-	// interface.  But fmt.Print depends on being able to look.
-	// Now that reflect is more efficient the special cases in fmt
-	// might be less important.
-	return v.InternalMethod == 0
+	return v.InternalMethod == 0 && iv.flag&flagRO == 0
 }
 
 // Interface returns v's value as an interface{}.
@@ -891,22 +884,28 @@ func (v Value) CanInterface() bool {
 // (as opposed to Type.Method), Interface cannot return an
 // interface value, so it panics.
 func (v Value) Interface() interface{} {
-	return v.internal().Interface()
+	return valueInterface(v, true)
 }
 
-func (iv internalValue) Interface() interface{} {
+func valueInterface(v Value, safe bool) interface{} {
+	iv := v.internal()
+	return iv.valueInterface(safe)
+}
+
+func (iv internalValue) valueInterface(safe bool) interface{} {
 	if iv.kind == 0 {
 		panic(&ValueError{"reflect.Value.Interface", iv.kind})
 	}
 	if iv.method {
 		panic("reflect.Value.Interface: cannot create interface value for method with bound receiver")
 	}
-	/*
-		if v.flag()&noExport != 0 {
-			panic("reflect.Value.Interface: cannot return value obtained from unexported struct field")
-		}
-	*/
 
+	if safe && iv.flag&flagRO != 0 {
+		// Do not allow access to unexported values via Interface,
+		// because they might be pointers that should not be 
+		// writable or methods or function that should not be callable.
+		panic("reflect.Value.Interface: cannot return value obtained from unexported field or method")
+	}
 	if iv.kind == Interface {
 		// Special case: return the element inside the interface.
 		// Won't recurse further because an interface cannot contain an interface.
@@ -1758,7 +1757,7 @@ func convertForAssignment(what string, addr unsafe.Pointer, dst Type, iv interna
 		if addr == nil {
 			addr = unsafe.Pointer(new(interface{}))
 		}
-		x := iv.Interface()
+		x := iv.valueInterface(false)
 		if dst.NumMethod() == 0 {
 			*(*interface{})(addr) = x
 		} else {

test/interface/fake.go

@@ -12,20 +12,20 @@ package main
 import "reflect"
 
 type T struct {
-	f float32
-	g float32
+	F float32
+	G float32
 
-	s string
-	t string
+	S string
+	T string
 
-	u uint32
-	v uint32
+	U uint32
+	V uint32
 
-	w uint32
-	x uint32
+	W uint32
+	X uint32
 
-	y uint32
-	z uint32
+	Y uint32
+	Z uint32
 }
 
 func add(s, t string) string {
@@ -40,16 +40,16 @@ func assert(b bool) {
 
 func main() {
 	var x T
-	x.f = 1.0
-	x.g = x.f
-	x.s = add("abc", "def")
-	x.t = add("abc", "def")
-	x.u = 1
-	x.v = 2
-	x.w = 1 << 28
-	x.x = 2 << 28
-	x.y = 0x12345678
-	x.z = x.y
+	x.F = 1.0
+	x.G = x.F
+	x.S = add("abc", "def")
+	x.T = add("abc", "def")
+	x.U = 1
+	x.V = 2
+	x.W = 1 << 28
+	x.X = 2 << 28
+	x.Y = 0x12345678
+	x.Z = x.Y
 
 	// check mem and string
 	v := reflect.ValueOf(x)