Make JSON RPC v2.0 schema reject unknown properties

[ncw]

This was easy for the Request, but the Response required a bit of re-organization to remove the "allOf" which doesn't allow an easy way of setting "additionalProperties": false

I've tested these changes against two different implementations of jsonrpc v2.0 so hopefully they are OK!

Rejecting unknown properties is desirable for exact protocol conformance, even though most clients and servers probably don't care.

[fge]

Hello,

I will need to reread the spec, it's been some time.

For answers you seem to require error, that doesn't look right...

[ncw]

A success Respone has "id", "jsonrpc", "result" whereas an error Response has "id", "jsonrpc", "error" which is as described in the spec in the "Response object" section: Either the result member or error member MUST be included, but both members MUST NOT be included.

I think that is what I've expressed in the json schema, but I'm a noob at it and I may have made a mistake!

I have used the schemas against some real jsonrpc servers and they appear to work, but that isn't a guarantee that they are correct.

Thanks

Nick

[fge]

Hmm OK, I have reread the spec but I cannot see the part where it says that there should not be more keys than the ones defined?

URL used: http://www.jsonrpc.org/specification

[fge]

See commit above. The changes are in fact rather simple:

• we move the required properties out of common and into each of result and error;
• we add additionalProperties for each result and error;
• we also remove the constraint from common that an object cannot be both a result and an error -- which is now enforced by additionalProperties.

Still unsure about the intent of the spec though.

[ncw]

That looks like a better way of doing it.

The spec says "The Response is expressed as a single JSON Object, with the following members:" which I read as saying that those and only those members are allowed.

Having the schema disallow any extra members seems like a good idea to me as it will catch any mis-spellings of the members for instance, and any attempt to sneak extra data in or out of the protocol.

I'll try the schemas in my application tomorrow or on Monday

Thanks

[fge]

The spec says "The Response is expressed as a single JSON Object, with the following members:" which I read as saying that those and only those members are allowed

Well, that is the problem isn't it? ;) I read it as saying that other members may be allowed... A ping to the authors would probably be in order here -- I do agree that your interpretation makes more sense, especially with regards to your further comments that spelling mistakes can be caught this way.

[fge]

OK, can you review the commit above? If you agree with the commit message, I'll rebase onto master and push it.

[ncw]

I tried the response schema on

{
    "id": "42",
    "jsonrpc":"2.0",
    "result":"potato"
}

Unfortunately neither of the validators I tried gojsonschema or python jsonschema agrees that it matches the schema whereas the previous version of the schema matches fine. I can't figure out what is wrong though :-( Any ideas?

[fge]

Hmm, what are the error messages? There are also error messages with my own validator here even though I cannot figure out why they are happening...

[ncw]

gojsonschema gave

0: ROOT : No additional property ( id ) is allowed on allOf
1: ROOT : No additional property ( jsonrpc ) is allowed on allOf
2: ROOT : No additional property ( result ) is allowed on allOf
3: ROOT : $ref failed to validate all of the schema
4: ROOT : (root) failed to validate exactly one of the schema

python jsonschema said

jsonschema.exceptions.ValidationError: {u'jsonrpc': u'2.0', u'id': u'42', u'result': u'potato'} is not valid under any of the given schemas

Failed validating u'oneOf' in schema:

Neither of which are wildly helpful!

[fge]

Uh, OK, I know what is happening.

It is because of additionalProperties. One thing to remember here is that schemas are not merged. As a result, additionalProperties depends on what is present in properties and patternProperties for the current schema. And in the schema where additionalProperties is defined, no properties are defined at all; as such, none are legal.

The workaround as I see it is to add to the #/definitions/success and #/definitions/error paths something like:

"properties": {
    "id": {},
    "jsonrpc": {},
    "result": {} // will be "error" in the corresponding subschema
}

Quite ugly, I agree.

[fge]

Confirmed: this works. I'll post an update to the patch soon.

[fge]

OK, can you try with this updated schema?

[ncw]

Yes that looks perfect - thanks