Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/binary: Read and Write have asymmetric value for unexported struct fields #7482

bgilmore opened this issue Mar 6, 2014 · 6 comments


Copy link

bgilmore commented Mar 6, 2014

What does 'go version' print?

  go version go1.2 linux/amd64

What steps reproduce the problem?

  Use binary.Write to marshal a struct with an unexported field, then attempt to unmarshal the resulting bytes using binary.Read

What happened?

  The reflect module panics when attempting to set the unexported field value, e.g:

  "panic: reflect: reflect.Value.SetInt using value obtained using unexported field"

What should have happened instead?

  The documentation clearly states unnamed fields are written out zeroed, but no verbiage exists for unexported fields.

  Some potential ways forward:

    * unexported fields could be written zeroed and skipped during reading (treat as unnamed fields).
    * unexported fields could be skipped during reading and writing.
    * continue to panic, but add explicit documentation about unexported field handling.

See also:

  Blank field handling added in
Copy link

minux commented Mar 6, 2014

Comment 1:

i think ideally Write should just skip unexported fields, however, that is an API change.
so at this stage, we actually have only two options:
1. skip unexported fields during read (and document that),
2. panic as usual, document it.
I think option 1 is the way to go.
What do you think?

Copy link

bgilmore commented Mar 6, 2014

Comment 2:

rsc objected to not-panicking in during Read in message #3 on the linked change, and we
don't want to change the output of Write, so I think this may be best addressed as a doc
That being said, I agree that Write should be skipping (or zeroing) unexported fields.
Requiring developers to implement BinaryMarshaler and BinaryUnmarshaler to get symmetric
behavior is unfortunate.

Copy link

Comment 3:

Owner changed to @ianlancetaylor.

Status changed to Started.

Copy link

Comment 4:

CL mentions this issue.

Copy link

Comment 5:

This issue was closed by revision c00804c.

Status changed to Fixed.

Copy link

Comment 6:

Issue #8315 has been merged into this issue.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet

No branches or pull requests

4 participants