code review 2246049: json: do not write to unexported fields

json: do not write to unexported fields

Fixes issue 977.
Fixes issue 451.

[rsc, 2010-09-27 17:15:44]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r, 2010-09-28 18:27:13]

http://codereview.appspot.com/2246049/diff/2001/src/pkg/json/decode.go
File src/pkg/json/decode.go (right):

http://codereview.appspot.com/2246049/diff/2001/src/pkg/json/decode.go#newcode94
src/pkg/json/decode.go:94: return "json: cannot unmarshal object key " +
strconv.Quote(e.Key) + " into unexported field " + e.Field.Name + " of type " +
e.Type.String()
do you want to print the field name of an unexported field?  for that matter,
i'm a little surprised you can even see it.

[rsc, 2010-09-28 18:35:47]

>
http://codereview.appspot.com/2246049/diff/2001/src/pkg/json/decode.go#newcode94
> src/pkg/json/decode.go:94: return "json: cannot unmarshal object key " +
> strconv.Quote(e.Key) + " into unexported field " + e.Field.Name + " of
> type " + e.Type.String()
> do you want to print the field name of an unexported field?  for that
> matter, i'm a little surprised you can even see it.

you can see it so that fmt can print it.
i think as long as it's there, it's worth
worth showing the field name.

an alternative would be to silently ignore
such fields, but 9 times out of 10 i bet the
fields belong to the caller's package
and the caller just hasn't realized that
such fields aren't writable via reflection,
so good to give a detailed error.

russ

[r2, 2010-09-28 18:35:54]

LGTM

[rsc, 2010-09-28 18:40:28]

*** Submitted as 382f5e538916 ***

json: do not write to unexported fields

Fixes issue 977.
Fixes issue 451.

R=r, r2
CC=golang-dev
http://codereview.appspot.com/2246049

[benolive, 2010-11-06 22:41:27]

On 2010/09/28 18:40:28, rsc wrote:
> *** Submitted as 382f5e538916 ***
> 
> json: do not write to unexported fields
> 
> Fixes issue 977.
> Fixes issue 451.
> 
> R=r, r2
> CC=golang-dev
> http://codereview.appspot.com/2246049

Should JSON encode be allowed to read unexported fields if JSON decode cannot
write them?

[rsc, 2010-11-15 21:28:40]

> Should JSON encode be allowed to read unexported fields if JSON decode
> cannot write them?

I don't see why.  The restriction is in package
reflect, not JSON.  JSON is just giving a nicer
error than trying the operations with reflect would.

Russ