Playgroud link: http://play.golang.org/p/9j0ome9HqK

@rsc, @adg, thoughts? This surprised me. I thought we only did the case insensitive thing when there was no struct tag.

But from the ietf link above:

is looks quite diabolical for security as it is trivial to create valid JSON values that will be interpreted differently by different implementations.

Related to "differently by different implementations", we permit JSON keys twice: http://play.golang.org/p/lPgEj1T6Zk (two "alg"). That probably differs between implementations in either its output or whether it's even successful.

This has been the behavior at least as far back as Go 1.2 (I can't run
earlier on my Mac).
The docs also seem to state quite clearly that this is what happens:

To unmarshal JSON into a struct, Unmarshal matches incoming object keys to
the keys used by Marshal (either the struct field name or its tag),
preferring an exact match but also accepting a case-insensitive match.
Unmarshal will only set exported fields of the struct.

I understand there are security implications if JSON is used in security
contexts, and I was a little surprised too, but the docs are very clear. I
doubt that encoding/json's behavior should be dictated by security
concerns. FWIW, I don't believe we should go out of our way to reject
repeated fields either, especially not now. I might go so far as to argue
that using JSON in security standards is a mistake anyway, but I won't do
that here.

In any event it's too late to change the defaults. If we want to support
the security use case, maybe we could have a UseStrictNames method on
Decoder (like UseNumber).

UseStrictNames SGTM.

I would consider addressing a bunch of related issue as an option:
#14749
#14135

UseStrictNames does look like a decent way to add case-sensitive decoding support in a backward compatible way.

However, while that would make case-sensitive decodes possible, it wouldn't help make this safer mode common or the default.

How about also defining a "strict" field tag option (c.f. "omitempty")? That should allow types to be safely used via newDecoder, or Unmarshall, or when the decoding is within a library. You can define the safety in the type, without finding every place it might be decoded.

Shall I send a UseStrictNames CL? Should that apply strict names across the board, whether or not the user supplied a struct tag?

type Foo struct {
        Bar string
        Baz string `json:"baz"`
}

So that would only match exactly "Bar" and "baz".

@cespare If it's simple, then yes sure. But after a thought on #15314 I wonder if maybe the UseStrictNames method is the wrong approach and instead it should be a tag attribute on the field. Then it can be enforced by the author of the struct instead of the author of the unmarshal call. Specifically, something like json:"Foo,exactname".

Would it get super repetitive if the person had to set it on every tag name?

I've come to realise that pretty much all of my previous comments in this thread were wrong :) The current documentation is technically correct, as @rsc points out in #14750 (comment), but a bit misleading, as @ysmolsky points out in #14750 (comment).

I do think that many parts of the json package could be designed better, and I think the edge cases concerning missing, repeated, or case-insensitive-matching keys are some of them.

Having said that, this issue is largely not about making a breaking change to the JSON decoder, or adding an option to change its behavior. This is a documentation issue; far too many users (myself included!) were misreading the docs. I hope that my CL above fixes that problem.

I think that a proposal to modify the decoder's API or behavior should be filed separately, once this issue is fixed with docs. This thread has become too long to discuss any one particular solution, and proposals have had their own process for a few years now.

@mvdan i don't understand how the docs are correct today. they say an exact match is preferred, but it is obviously not at all. that's what throws people off. there is not preferring happening at all, it's a race case of what is encountered first.

i would be happy with a change to the docs to reflect that it's simply a case-insensitive match, with a brief mention of the proper approach if a case-sensitive match is required (writing a custom unmarshaller using a map with string keys). my concerns about it being a bug were a result of the fact that i didn't think there was a workaround to get a case-sensitive match. knowing there is a workaround, i can accept this behavior with the documentation change.

@justinrixx please read the comments on the CL above.

To summarize my exchange with @dsnet on Gerrit: Yes, the current behavior is technically correct from the point of view of the decoder, as "incoming object" can easily be understood to be the key-value pairs being parsed in the order in which they appear.

However, what Joe argues is that this is an implementation detail - the JSON spec doesn't specify object keys to be ordered in any way. We could change the underlying behavior to be closer to what most users would expect, not to stick to what's most efficient or most logical from the decoder's point of view. For example, by skipping case-insensitive matches of a field if a case-sensitive match for it already happened.

The devil is in the details, of course. Would this change slow down the decoder too much? Would it break too much existing code? That is yet to be determined, and I'd like to find some time to experiment with it in the near future.

Change https://golang.org/cl/224079 mentions this issue: encoding/json: skip inexact matches after exact matches

The CL above is the first iteration of the experiment I mentioned two weeks ago. Decoding structs is ~1% slower, but we get the benefit we want. Please test the CL and let me know if your code breaks, or if it properly fixes this issue for you.

1% performance loss is unfortunate, but I can't figure out a way around it. Alternative patches without the slow-down are welcome.

I think the CL is interesting, but it would, from my perspective, be even more useful to explicitly define an option. It's useful to be able to set that a case-insensitive match be treated as an unknown field.

As a comment on #14750 (comment):

I would vote for allowing case-sensitive matching as a decoder option enabled through a method call. (UseStrictNames, DisableEqualFold, or some better name) over struct-tags.

I want to advocate for this solution not because it's a better solution, but because it's in line with the current design for DisallowUnknownFields and UseNumbers. Having one annoying -- but consistent -- API has a value; it means the same work-around can be used for all cases where a custom json.Unmarshaler implementation is needed.

The DisallowUnknownFields and UseNumbers are in my opinion within the same class of problems, and that puts some restrains on how a strictly case-sensitve parser should be implemented.

Some notes:

• Projects can easily define their own jsonDecoder(data []byte) json.Decoder helper-functions to reduce the boiler plate in json.Unmarshaler implementations.
• Making a new json.Unmarshaler interface that accepts passing along options (or better still, accept a pre-initialized (sub) json.Docoder with inherited options) is certainly possible, but it's a separate issue. It is not given that it's always preferred.

PS! The Encoder/Decoder interfaces defined in #12001 (old issue) could serve as a way of passing along decoder option. Perhaps that problem can be discussed there, although the motivation here is different.

@rsc this issue seems to have died down and gone off-topic. Would it be appropriate to reboot this as a proposal for UseStrictNames?

preferring an exact match but also accepting a case-insensitive match

The wording of the first half line is some misleading IMHO.
It contains negative information.
I recommend to remove this half line to match the current behavior.

Just learned about this bahavior and this screams unintended bugs (no matter how much documentation). I always assumed that explicit struct tags take precedence over field names but they seem to just be ignore if there i field that matched.

This breaks binance API parsing (see https://binance-docs.github.io/apidocs/spot/en/#individual-symbol-ticker-streams)
The API returns objects that have many single-character properties, many of them are the same letter but in different cases (so you pretty much have half the object's properties confused by the parser), and as far as I understand, after 6 years of this issue being known, there is still no workaround aside from parsing to a map[string]interface{} and then parsing to the desired object
I don't mind implementing a decoder option or whatever, but it needs to be decided so that I or someone else can implement it

@krigga a different workaround wouldn't be to declare all possible attributes cases, even those not required by the application?

For example, considering Binance's ticker payload, even if you only wanted to parse the "a" atribute, it seems to me that you could avoid the mismatching with the "A" field by declaring both on your target structure and simply ignoring the unwanted value https://go.dev/play/p/1_71Bw5FBJz.

Of course, this is just a workaround for the issue. One shouldn't be required to parse unwanted atributes for the sole purpose of avoiding this behavior. On this same snippet, removing any of the struct fields (by commenting line 14 or 15) leaves the script susceptible to the mentioned awkward behavior, by parsing "a" into an atribute with an "A" field tag.

This fork deals with the issue and implements the original proposed change I believe; https://github.com/james-d-elliott/go-json

It's a 1:1 fork of go1.19.3 other than the additional method and usage of said method. If the original proposal ever gets traction and is accepted you're welcome to ping me and/or just use the code as-is/modified with/without crediting me.

Hi all, we kicked off a discussion for a possible "encoding/json/v2" package that addresses the spirit of this proposal.

In v2, we propose that Unmarshal perform a case-sensitive match on the field name by default.

For flexibility, the v2 prototype provides the ability to alter this behavior with:

• json.Unmarshal(v, json.MatchCaseInsensitiveNames(false)) at the unmarshal-call level OR
• struct{ FooField T `json:",nocase"` } at the struct-field level