Use of JSON 'null' in request syntax

[Joseph Heenan]

I fully appreciate this has been discussed before:

https://bitbucket.org/openid/ekyc-ida/issues/1177/avoid-using-null-to-mean-something

https://bitbucket.org/openid/ekyc-ida/issues/1110/identity-assurance-giving-null-and-or

but the use of ‘null’ values in JSON is problematic, and the new data I’d like to add is that this caused a whole bunch of unnecessary difficultly implementing the eKYC conformance tests.

I also appreciate that this is how things are defined in core.

I’d like to make the following points:

1. This is actually a problem in the real world; many libraries that handle JSON tend to (at best) default to ignoring entries that have null values (as well as a number of JSON libraries also making is easy for programmers to accidentally generate entries with null values - which is probably why many libraries ignore the null values by default…). The certification suite was actually in a better place than many people as previously we had already solved several issues with null values disappearing before they arrived at our code in order to be able to detect places were people were accidentally sending null values.
2. This particular part of OIDC Core is currently not widely used/supported in the real world; many RP and OP implementations do not support it.
3. This particular part of OIDC Core is NOT tested by any existing conformance tests.
4. The point of implementors draft specifications is to figure out what problems happen implementing them in the real world and try to change specifications to lessen those problems.
5. It’s explicitly in the certification team's remit to bring issues like this back to the working groups to be considered.

In short I think using this syntax for eKYC is going to cause a lot of people a lot of unnecessary pain in the short, medium and long term.

‌

[Torsten Lodderstedt]

Do you have a proposal for an alternative in mind?

Note: there are existing implementations in the wild, so the spec basically works. I’m very reluctant to change the spec design now if there are no super convincing arguments as any change would most likely be a big breaking change.

[Joseph Heenan]

Do you have a proposal for an alternative in mind?

Sadly I have not had time to give that any thought.

‌

Note: there are existing implementations in the wild, so the spec basically works. I’m very reluctant to change the spec design now if there are no super convincing arguments as any change would most likely be a big breaking change.

I agree with both that the spec basically works and that any solution is likely a big breaking change, but I do see a notable gap between “spec basically works” and “spec is easy to implement and has no known crocodile filled pits”.

[Kai Lehmann]

We have implements in Java and JavaScript which are able to work with

"given_name": null

We are able to differentiate between a claim not being requested at all and a claim being requested as non-essential.

However, it would be possible to make the request more explicit like so:

"given_name": {
  "essential": false
},

This is essentially (no pun intended) the same request as when using null.

[Nat Sakimura]

Is there a list of libraries with this behaviour?

[Joseph Heenan]

It seems like you are highlighting an issue with OIDC core claims request object syntax, is that correct? I’m not trying to dodge the issue just want to be clear about where it lies and where any fix may be needed.

Mostly. There is at least one case where eKYC introduces a new use of null (for ‘access_token’).

‌

[Joseph Heenan]

@Nat

Is there a list of libraries with this behaviour?

I’m not sure there’s a list. I’m not sure I’m even aware of any libraries that don’t (by default) have some behaviour around nulls that makes implementing this difficult.

Java GSON: By default GsonBuilder omits null values, can be overridden by using gsonBuilder.serializeNulls()

Java Bson: Seemed to refuse to handle null values; we had to map to a sentinel value and back again to workaround: https://gitlab.com/openid/conformance-suite/-/commit/fe02e73b42398cf857c930bc6386c2656bb358f4

Go: https://www.calhoun.io/how-to-determine-if-a-json-key-has-been-set-to-null-or-not-provided/ - you have to write a custom unmarshaller to explicitly handle any nulls.

‌

[Stuart Low]

Java Jackson: Default behaviour is to drop them entirely unless modified within JsonInclude tweaks

[Stuart Low]

As discussed in the call an additional option may be:
"given_name": {}

‌

[Mark Haine]

Next step is to implement tests in conformance suite specifically for “essential:false“ and empty object “{}“ and have participants in the interop event exercise these.

Based on the outcome of the interop there may be changes to the draft to drive specific implementation details in the direction of the preferred option.

[Joseph Heenan]

The tests Mark mentions have been added to the suite:

https://gitlab.com/openid/conformance-suite/-/commit/8108a6929f3988e50c8172387fd1e38521c22912

‌

[Mark Haine]

At this stage testing by more implementers is needed to determione the real world state of this issue

[Joseph Heenan]

As a result of a test for the claims parameter that’s part of the FAPI2 suite, I’m now aware of one implementation in the wild that supports both empty object and the essential: false form, but (unintentionally) ignores the claim request if a null value is used.

Interestingly the OpenID Connect certification has very little testing of claims (presumably explaining why the problem with the null handling in this OP was not previously found) - the only test there is uses the essential: false form.

(I am told the bug is, yet again, most likely caused by a JSON parser silently dropping the claim that has the null value.)

[Torsten Lodderstedt]

just as a datapoint - we use the null form as defined in core and utilised in eKYC quite extensively in our ecosystem (across 4 different IDP implementations and a number of RP using different programming languages). I’m not aware of any issues with that so far.

[Daniel Fett]

As another datapoint, I have not had any problems with Python libraries with this, both for JSON as well as for YAML (which I use quite extensively in various tools for our ecosystem).

[Joseph Heenan]

One solution (at least for within verified_claims) would be to make ‘essential’ a mandatory claim (meaning we always have a non-empty object), and deprecate the null syntax.

[Torsten Lodderstedt]

One solution (at least for within verified_claims) would be to make ‘essential’ a mandatory claim (meaning we always have a non-empty object), and deprecate the null syntax.

That would deprecate the exiting implementations in the yes ecosystem without a tangible benefit.

[Joseph Heenan]

I think there are tangible benefits (for the standard as a whole - I appreciated many of these won’t apply to the yes ecosystem):

1. It’s a little more intuitive to read (the use of ‘no value’ to mean “I want this value” isn’t really intuitive)
2. It’ll avoid OP implementations having to deal with null values; we have enough data now to show that there are a non-trivial number of implementations that don’t handle null correctly, and a number of those that do handle it correctly had to jump through extra hoops to do so.
3. It’ll avoid RP implementations having to deal with the null values.

I think the only question is to whether the benefits of the change outweigh the cost. My gut feeling is that given there are a relatively low number of implementations (compared to the total number we are aiming for) making a change that makes implementing the standard easier is worthwhile.

Can we clarify the impact on the yes ecosystem? I think the impact is that when RPs want to start interacting with non-yes servers then any currently using the null syntax will have to update the syntax they use, but I think this is only one of the changes that such RPs would have to make to interact with non-yes servers (unless the kind of proxy module we are talking about in GAIN was used as a gateway from the yes ecosystem to other ecosystems, in which case the gateway could do the null → essential:false transformation I think).

[Torsten Lodderstedt]

re 1: that’s very subjective

re 2: our experience indicates it’s not a problem

re 3: se 3

First of all, the null value was introduced by the Core Spec. Is your proposal to change this behavior for all claims in the claims parameter? I’m asking since we offer both (as unverified and verified), making the change to verified_claims only would cause confusion (I think) and introduce a new error source because null would be allows outside verified_claims but not inside. Changing it at all would require a change to core, wouldn’t it?

re GAIN: we don’t want or intend to proxy RP to IDP communication (for a couple of reasons, esp. security, privacy and performance). So changing this behavior requires at least RP implementations to be changed.

Beside that, this change will render the whole ecosystem non-compliant that makes me concerned.

[Daniel Fett]

I’m still not convinced we should switch away from ‘null’. I agree that ‘null’ is not very intuitive, but even more so is prescribing ‘essential: false’ for some parts of the request.

Also, there are cases where the claims parameter is sent in the frontend and not in a PAR request. With ‘essential: false’, the request becomes much longer and will be more prone to cross the 2KB boundary for the URL length, causing problems in some browsers and firewalls.

[Joseph Heenan]

Thanks Torsten/Daniel.

‌

re: size; I’m not sure the 2KB length is still in play now IE is officially dead (although as you mention there might still be some very old proxies out there that may have a limit, but these are definitely the exception these days). If it is an issue that would suggest a preference for the "given_name": {} option.

‌

[Daniel Fett]

It’s less the browsers in our experience, mostly web application firewalls and other middle boxes, and it definitely still happens (the latest bug because of this was found ~10 days ago).

[Takahiko Kawasaki]

FYI

JSON serialization by Gson:

import java.util.HashMap;
import java.util.Map;
import com.google.gson.GsonBuilder;

public class JsonNullGson
{
    public static void main(String[] args)
    {
        Map<String, Object> inputMap = new HashMap<>();
        inputMap.put("key", null);

        String outputJson1 = new GsonBuilder().create().toJson(inputMap);
        String outputJson2 = new GsonBuilder().serializeNulls().create().toJson(inputMap);

        System.out.println("outputJson1 = " + outputJson1);
        System.out.println("outputJson2 = " + outputJson2);
    }
}

and its output:

outputJson1 = {}
outputJson2 = {"key":null}

‌

JSON serialization by Jackson:

import java.util.HashMap;
import java.util.Map;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.annotation.JsonInclude.Include;

public class JsonNullJackson
{
    public static void main(String[] args) throws Exception
    {
        Map<String, Object> inputMap = new HashMap<>();
        inputMap.put("key", null);

        String outputJson1 = new ObjectMapper().writeValueAsString(inputMap);
        String outputJson2 = new ObjectMapper().setSerializationInclusion(Include.NON_NULL).writeValueAsString(inputMap);

        System.out.println("outputJson1 = " + outputJson1);
        System.out.println("outputJson2 = " + outputJson2);
    }
}

and its output:

outputJson1 = {"key":null}
outputJson2 = {}

‌

It depends on respective libraries whether properties with null are serialized into JSON by default. The examples above indicate that the default settings of Gson does not serialize properties with null into JSON while the default settings of Jackson serializes properties with null into JSON. Developers who haven’t encountered “null serialization” issues in their life are just lucky.

As I pointed out in Issue #1110, Golang cannot tell differences between null and an empty string in JSON because the string type in Golang cannot become nil. It may be difficult to serialize a struct with null values into JSON directly in Golang, so Golang developers would have to manually write JSON if they want to include properties with null. But I may be wrong because I’m not so familiar with Golang.

[Daniel Fett]

Thanks for sharing this, Taka!

Regarding the issue in Golang, isn’t that just a variant of polymorphism then? I.e., the value is either a string or of the type that covers null?

More generally, does anybody know of a good reason why null values would not be serialized? Is there history precedent or something in a spec somewhere? It just seems very arbitrary to me.

[Mark Haine]

My suggestion is to provide some implementation guidance to help highlight this issue for implementers and to help them with some suggestions on how to avoid this issue.

Awareness of this issue

Careful cosideration of the libraries to be used

test behavior early in the dev lifecycle

[Joseph Heenan]

More generally, does anybody know of a good reason why null values would not be serialized? Is there history precedent or something in a spec somewhere? It just seems very arbitrary to me.

I suspect it’s an hang over from early implementations in languages that don’t have a built in mechanism to distinguish between “key not present” and “key is present with no value”, java maps or java objects both being examples of these, and null is generally chosen to mean “key not present” - so in java concepts like the JsonNull value were invented that you can put into native java maps/objects. I suspect many first iteration JSON libraries simply didn’t realise that there would ever be a need to serialise null values as something other than ‘key not present’, so it was a feature added later and disabled by default, perhaps for backwards compatibility.

‌

[Joseph Heenan]

First go at some text:

“This specification requires that implementations receiving requests are able to distinguish between JSON objects where a key is not present versus where a key is present with a null value, for example when using the claims parameter to request a claim is returned as per <link to relevant section>. Support for these null value requests is mandatory for identity providers, so implementors are encouraged to test this behaviour early in their development process, as in some programming languages many JSON libraries or HTTP frameworks will, at least by default, ignore null values and omit the relevant key. For relying parties, there are alternatives to using the null value, for example an object containing “essential”: “false” has the same meaning, though this will result in slightly larger objects which hit size limits if the request body is being passed in the URL query.

(I believe we don’t use nulls in responses, can anyone confirm?)

[Joseph Heenan]

Mark suggested putting this in the examples section, 7.

Taka suggested extending the replying parties sentence with example JSON.

[Joseph Heenan]

PR here: https://bitbucket.org/openid/ekyc-ida/pull-requests/127/mention-issues-with-null-json-values

I decided not to mention the alternatives to null for RPs as when looking at the example request:

‌

{
  "userinfo": {
    "verified_claims": {
      "verification": {
        "trust_framework": null,
        "time": null,
        "evidence": [
          {
            "type": {
              "value": "document"
            },
            "method": null,
            "document_details": {
              "type": null
            }
          }
        ]
      },
      "claims": {
        "given_name": null,
        "family_name": null,
        "birthdate": null
      }
    }
  }
}

I am not convinced that in all these locations changing null to { "essential": "false" } is actually valid.

‌

[Mark Haine]

Resolved by PR #127