Skip to content

encoding/gob: encoder should ignore embedded structs with no exported fields #5819

Open

Description

@gopherbot

by alan.strohm:

What steps will reproduce the problem?
If possible, include a link to a program on play.golang.org.
1. http://play.golang.org/p/ayZMy_HFKE

What is the expected output?

"worked"

I don't see why hidden1 (an unexported field in an embedded struct) should be handled
differently than hidden2 (a top level unexported field). Making the embedded struct type
itself unexported ("inner" vs "Inner") works but seems unecessary.

What do you see instead?

gob: type main.Inner has no exported fields

Activity

  1. robpike commented on Jul 11, 2013

    @robpike
    Contributor

    Comment 1:

    This is a tough one. The reason to leave things alone is that it's a very common user
    error to attempt to marshal a struct with no exported fields, and the error message
    helps diagnose the case with a clear error.
    However, diagnosing it in this more complicated case might be possible.

    Labels changed: added priority-later, removed priority-triage.

    Status changed to Accepted.

  2. orian commented on Sep 12, 2013

    @orian

    Comment 2:

    For consideration, we have:
    var req http.Request
    It has req.URI.User field of type *url.Userinfo. And url.Userinfo doesn't have exported
    fields.
    This blocks possibility of dumping http.Request to file using gob.
    I also wonder why does it happen when req.URI.User==nil.
  3. rsc commented on Nov 27, 2013

    @rsc
    Contributor

    Comment 3:

    Labels changed: added go1.3maybe.

  4. dsymonds commented on Dec 2, 2013

    @dsymonds
    Contributor

    Comment 4:

    Labels changed: added packagechange.

    Owner changed to @robpike.

  5. rsc commented on Dec 4, 2013

    @rsc
    Contributor

    Comment 5:

    Labels changed: added release-none, removed go1.3maybe.

  6. rsc commented on Dec 4, 2013

    @rsc
    Contributor

    Comment 6:

    Labels changed: added repo-main.

  7. glycerine commented on Dec 17, 2013

    @glycerine

    Comment 7:

    This is a real issue when you have structs that contain  an os.File field, for example.
    Go has annotations right? Seems like a good use for them here: annotate the field as
    being ignorable by gob without error.
  8. mever commented on Dec 16, 2014

    @mever

    @robpike is there any indication when this is going to be implemented?

  9. added this to the Unplanned milestone on Apr 10, 2015
  10. 13 remaining items

  11. bryanjeal commented on Oct 20, 2017

    @bryanjeal

    @happierall I know this is quite an old issue, but here is how we solved it:

    If we have the following structure foo:

    type foo struct {
      sync.RWMutex
    
      Bar string
      Baz int
    }

    We create an Encode struct:

    type fooEncode struct {
      Bar *string
      Baz *int
    }

    We then do this to encode:

    fooInstance.RLock()
    defer fooInstance.RUnlock()
    
    encodable := fooEncode {
      Bar: &fooInstance.Bar,
      Baz: &fooInstance.Baz,
    }
    
    err := gobEncoder.Encode(encodable)
    ...

    This is a lot of work for just embedding sync.RWMutex, but we embed more than just sync.RWMutex (it was worth it for us).

  12. sb10 commented on Feb 27, 2018

    @sb10

    Any word on this? My use case is having structs that embed log15.Logger. For the one struct that I encode with gob I've had to work-around this by storing it on unexported field, giving the ugly mystruct.logger.Debug("foo"), instead of mystruct.Debug("foo") used everywhere else in my code.

  13. gopherbot commented on Jun 11, 2018

    @gopherbot
    ContributorAuthor

    Change https://golang.org/cl/117875 mentions this issue: encoding/gob: fix handling of embedded structs without exported fields

  14. iwdgo commented on Jun 11, 2018

    @iwdgo
    Contributor

    Unexported fields were correctly handled except structs without exported field for which an error message is returned.

    The check returning the error in the Encode is removed in the submitted fix as documentation is not excluding the case. The Decode part is also fixed for idem-potency. Tests have been amended.

  15. modified the milestones: Unplanned, Go1.11 on Jun 11, 2018
  16. added
    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
    on Jun 23, 2018
  17. odiferousmint commented on Mar 7, 2019

    @odiferousmint

    What is the rationale for the "limitation"? The fields of my structs are intentionally unexported because I do not want any other parts of the program to have direct access to it, but inside this file I would like to encode and decode this struct because I have visibility and it is OK or should be OK to do that.

    What should I use instead then? Would encoding/binary work?

    Nevermind, I got "invalid type".

  18. bryanjeal commented on Mar 7, 2019

    @bryanjeal
  19. odiferousmint commented on Mar 7, 2019

    @odiferousmint

    The Encoder you are using isn't part of your code. It is another library - one that might be in the standard library, but still separate from your package. Your package could either create a temporary struct to Encode/Decode or you can write your own Encoder/Decoder within your package.
    …
    On Thu, 7 Mar 2019 at 10:30, odiferousmint @.***> wrote: Why is this even a thing, what is the rationale for it? The fields of my structs are intentionally unexported because I do not want any other parts of the program to have direct access to it, but inside this file I would like to encode and decode this struct because I have visibility and it is OK or should be OK to do that. What should I use instead then? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5819 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AJS5jKsaeNQ49LcLAYhPAW1_tn4LzWT5ks5vUUzDgaJpZM4DI-zO .

    I see. Thanks. :)

  20. nikhiljohn10 commented on Jan 18, 2023

    @nikhiljohn10

    There has been a breaking change in "crypto/elliptic" as mentioned in #57422 (comment) when go1.19 was released. But this code works fine in go1.18. This code is a very critical part of cryptocurrency wallets and I prefer to use the latest version of go.

    Code: https://go.dev/play/p/c6HKfG47CG4

    There mentioned a workaround using the Marshal function in crypto/elliptic. But I am not able to achieve it so far. So, I think solving this issue has been a higher priority since go1.19 was released. I am looking forward to it.

    Suggestion

    As it is mentioned by @robpike in the comment, It is helpful for diagnosis. But this error handling condition should be ignored/overridden if the gob.RegisterName function is executed.

  21. anishjain94 commented on Jul 31, 2024

    @anishjain94

    There's a pretty common pattern in the code I'm working on where a sync.RWMutex is embedded into structs that need mutual exclusion, e.g.:

    type foo struct {
      sync.RWMutex
    
      Foo string
      Bar int
    }

    When trying to encode structs like this I get an error with gob: type sync.RWMutex has no exported fields. It's simple enough to make the mutex into a named field, but handling this case automatically would be really nice.

    There's a pretty common pattern in the code I'm working on where a sync.RWMutex is embedded into structs that need mutual exclusion, e.g.:

    type foo struct {
      sync.RWMutex
    
      Foo string
      Bar int
    }

    When trying to encode structs like this I get an error with gob: type sync.RWMutex has no exported fields. It's simple enough to make the mutex into a named field, but handling this case automatically would be really nice.

    Actually this does not work. I had the same usecase. It still threw error.

    type LastReadCheckpointConfig struct {
    	FileLastReadPosition  int64
    	MongoLastReadPosition map[string]primitive.Timestamp
    	sync.RWMutex
    }
    

    But instead what worked for me was to create a new struct without RWMutex and encode/decode that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions

      ⌂ 换网址