[OpenID-Specs-eKYC-IDA] Issue #1177: Avoid using null to mean something (openid/ekyc-ida)

Marcus Almgren issues-reply at bitbucket.org
Sun Mar 1 15:54:26 UTC 2020


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

Marcus Almgren:

So, this is a “shots fired” type of issue, I presume. But please bear with me for a second:

The specification currently suggests to use `null` to mean something, i.e. if you as an RP need `given_name`, `family_name` and `birthdate`, then you would send a request like this \(`claims` part only\):

**Example 1:**

```
{
  "claims": {
    "given_name": null,
    "family_name": null,
    "birthdate": null
  }
}
```

But that approach — the approach to rely on the presence of a key to carry a significant meaning — is fragile as soon as we look beyond JSON and see how this would be implemented, as this will require control on the \(de\)serialization level in order to get it right.

What I mean is, let’s say that you want to implement this spec as an OP, and your organization uses a statically typed language, let’s say C# or Java or something quite common. Chances are that I would like the incoming request to be deserialized into a certain type, which for claims could be something like \(Hang in there, I’m trying to get to the point\):

**Example 2:**

```
class Claims {
  Claim GivenName { get; set; }
  Claim FamilyName { get; set; }
  Claim BirthDate { get; set; }
}

class Claim {
  bool? Essential { get; set; }
}
```

That class structure could be used to capture a request such as in Example 1, as well the following input, as per the spec:

**Example 3:**

```
{
  "claims": {
    "given_name": {
      "essential": true
    },
    "family_name": {
      "essential": true
    },
    "birthdate": null
  }
}
```

But now here \(finally\) comes the problem: This won’t work, because after deserialization, there is no way to distinguish between:

**Example 4:**

```
{
  "claims": {
    "given_name": null,
    "family_name": null,
    "birthdate": null
  }
}
```

and

**Example 5:**

```
{
  "claims": {
  }
}
```

I assume I’m stating the obvious here, but in terms of static typing I cannot distinguish between an instance member that has not been set, or an instance member that has been set to `null`. That would leave me only with the option of using `Map<String, Object>` to represent claims, which will in turn result in code littered with class casting and string comparisons in order to figure out what the request means.

Therefore, and here comes the aforementioned shots being fired: Would it be possible to drop the convention of using `null` to mean something, and instead say that **the empty JSON element** must be used instead? E.g:

**Example 6:**

```
{
  "claims": {
    "given_name": {},
    "family_name": {},
    "birthdate": {}
  }
}
```

Additionally, I think the problem goes both ways \(i.e. the currently suggested approach will cause problems for RPs as well as OPs\), since even the most common JSON serializers for .NET and Java consider “null serializing” a configuration of the serializer, and “serialize null or not” is a common setting for these. Additionally, the serialization and deserialization of OIDC related concepts has a tendency to happen in middleware, possibly outside the control of the programmer, so it might be hard or impossible for the programmer to force the serializer and middleware to serialize _some_ keys with `null` values, while ignoring others \(that are also null\).

I’m sorry for this wall of text, but I hope I’ve managed to make myself understood, and of course I sincerely hope I didn’t misunderstand anything fundamental. To back this issue up with “code”, I am also submitting a PR in which I replaced all the `null`s in the examples, which seemingly did not have any negativ impact on the current test suite.




More information about the Openid-specs-ekyc-ida mailing list