-
Notifications
You must be signed in to change notification settings - Fork 18k
encoding/gob: panic on encoding nil pointer #3704
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
Labels
Comments
I don't know what it should do, but not panicking would be a good start. I actually came across this while encoding a struct with a []interface{} field that had a nil pointer in amongst stuff like non-empty strings. Isn't that a reasonable data structure for gob to be able to encode/decode? |
I think a panic is the right thing to do. It could return an error but there's nothing to do if the error occurs because nothing is sent, and that will violate the wire protocol. Consider this code: enc.Encode(x) enc.Encode(y) You should always check errors, of course, and something else could go wrong, but it's reasonable to consider that in a working, debugged program this transmission can always be decoded by doing, on the remote side, dec.Decode(&x) dec.Decode(&y) The thing is, if x and y are both of type *T, no message will be sent for that pointer, and the value of y will be decoded into x. In short, attempting to send a top-level nil is a pretty serious violation of the streaming protocol. Of course, it's trivial to return an error and expect the user to check for error, and I could be talked into that, but I want to be clear that you cannot send a top-level nil ever. The gob package is not at fault here; the client is misbehaving. |
Correct. An interface value can be transmitted only if the concrete value itself is transmittable. At least for now, that's equivalent to saying that interfaces holding typed nil pointers cannot be sent. I can try to make the panic more descriptive, but actually "fixing" this is a significant design change. |
This issue was closed by revision ea3c3bb. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: