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/json: JSON Tags with Profile Support #14756

Closed
akutz opened this issue Mar 10, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@akutz
Copy link

commented Mar 10, 2016

Please answer these questions before submitting your issue. Thanks!

1. What version of Go are you using (go version)?

go version go1.6 darwin/amd64

2. What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/akutz/Projects/go"
GORACE=""
GOROOT="/Users/akutz/.go/1.6"
GOTOOLDIR="/Users/akutz/.go/1.6/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="1"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"

3. What do you want to do?

Currently there's no way to influence how a type's JSON tags are utilized during the marshal process beyond the standard omit, name, and omitempty characteristics. I've run into at least one instance where it would be ideal to omit certain data sometimes and not others depending upon the nature of how the data will be transported or stored.

While there are options available such as a terminating struct inheriting from multiple structs, whereby the parent types are categorized by the nature of the data they include, this is not a scalable or even elegant solution.

I propose the notion of profile support for JSON tags. Much like Spring Profiles, this feature will enable a more dynamic, but still declarative, means of influencing a runtime process. And it will do so in a very succinct and elegant manner.

Take the following type:

type Person struct {
    FirstName      string `json:"firstName"`
    LastName       string `json:"lastName"`
    PassphraseHash string `json:"passphraseHash"`
}

There may be cases for the above data type where the passphrase hash should be included in the marshaled JSON, and there may be cases where it should not. A simple solution is to add support for profiles at the JSON tag scope. For example:

type Person struct {
    FirstName string `json:"firstName"`
    LastName  string `json:"lastName"`

    // PassphraseHash should only be marshaled to JSON if the
    // "secure" profile is used.
    PassphraseHash string `json:"-" json.secure:"passphraseHash"`
}

As you can see, now the default behavior is to omit the PassphraseHash field when marshaling this type to JSON unless the secure profile is explicitly activated.

I've provided a complete example on the Go playground.`

I hope you'll agree that the implementation of the described feature has a value-to-cost ratio which makes it an ideal candidate for inclusion in the "encoding/json" package.

Please let me know if you have any questions or concerns. I'm more than happy to go ahead and submit a patch to go along with this proposed feature in order to progress the discussion to the next phase. Thank you!

@ianlancetaylor ianlancetaylor changed the title JSON Tags with Profile Support encoding/json: JSON Tags with Profile Support Mar 10, 2016

@akutz

This comment has been minimized.

Copy link
Author

commented Mar 10, 2016

Hi @ianlancetaylor,

Thank you for taking the time to triage my first Golang proposal. If it would help I could go ahead and create a patch for the proposed changes as there are only stubs in the Golang playground. The contribution guide stated to first submit an issue for discussion, and so I did. However, it should also not be too difficult to start patching encoding/json/encode.go#L1033 in order to facilitate discussion either.

Thank you again.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 10, 2016

I don't think the proposal pays for the additional complexity. I'm not even sure what a profile is or how you opt in to a profile.

What's wrong with just implementing https://golang.org/pkg/encoding/json/#Marshaler when you need a custom encoding on a type?

@akutz

This comment has been minimized.

Copy link
Author

commented Mar 10, 2016

Hi @bradfitz,

So a profile would be appended to to the json bit of a json tag for a struct field. For example:

type Person struct {
    Password `json:"-" json.secure:"password"`
}

I think the above enhancement to JSON tags is fairly straight-forward and simple. It doesn't offer breaking changes; only additive ones.

A profile simply notes a secondary, tertiary, etc. means of describing the JSON tag behavior depending upon the active profile(s) when marshaling types.

Regarding the use of the Marshaler interface, that's a very good point. I actually do use that when it's appropriate, but the use case I'm suggesting with this proposal is different.

Obviously one big difference between Go and a language like Java or C# is the absence of threads (in a meaningful manner for 99% of basic development). This of course means the absence of things like Thread Locals and DI Thread Scopes. Additionally this means it's difficult to know from a runtime perspective the provenance/context of a call by the time the type's MarshalJSON function is invoked.

Side note, if MarshalJSON would provide the context.Context parameter, I wouldn't be filing this proposal.

However, because there's no context for the MarshalJSON function, there's no standard manner to imply a context externally or consume a context from within that function. At best you could define some meaningful switch/flag/context field on each type related to the issue. That would, though, require either:

  1. Manually marshaling the type to JSON since you cannot call MarshalJSON on a type from within its implementation of the MarshalJSON function without getting a recursive loop, or
  2. Using reflection to first iterate over the type's exported fields and put them into a map[string]interface{} and then use json.Marshal to marshal the map instead of the type (after applying the logic associated with profiles with respect to the tags of course).

I do something similar to No. 2 above in the Golf package where I defined a tag to compliment json called golf for purposes similar to this proposal for profiles. However, the issue is that all packages that wish to afford themselves of Golf must first import Golf at the point at which a type is marshaled.

This proposal would enable an across-the-board, consistent manner to influence when individual fields are marshaled using encoding/json based on an extension to the json tag specification as well as enabling profiles for the encoding/json package and/or invoking a Marshal variant with an explicit profile.

@minux

This comment has been minimized.

Copy link
Member

commented Mar 10, 2016

@akutz

This comment has been minimized.

Copy link
Author

commented Mar 10, 2016

Hi @minux,

I definitely see your point. It is a specific type of marshaling that is certainly not global and thus not the correct place for a security boundary. For what it's worth, what you outlined as a solution is exactly what I attempted to describe with this:

While there are options available such as a terminating struct inheriting from multiple structs, whereby the parent types are categorized by the nature of the data they include, this is not a scalable or even elegant solution.

It's also exactly what I ended up doing before I created Gofig and the notion of a SecureKey.

I think your remark about relying on the JSON encoder as a security boundary is what really resonates with me though. I might be convinced to close the issue on my own as I have to agree, the JSON encoder simply isn't the place at which to implement something related to security.

While I still think there's value in altering how a type's fields are marshaled based on this proposed notion of profiles, with respect to @bradfitz's comments earlier, I'm having a hard time justifying additional complexity for this feature in a vacuum without an immediate and/or broad use case.

I'll close the issue. Thank you @bradfitz and @minux for providing quick and thoughtful feedback

@akutz akutz closed this Mar 10, 2016

@golang golang locked and limited conversation to collaborators Mar 13, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.