ably / specification

The Ably features spec for client library SDKs.
Apache License 2.0
0 stars 4 forks source link

Spec: "objects capable of JSON representation" should be clarified #75

Open SimonWoolf opened 7 years ago

SimonWoolf commented 7 years ago

http://docs.ably.io/client-lib-development-guide/features/#RSL4a has "Payloads must be binary, strings, or objects capable of JSON representation, or can be empty (omitted). Any other data type should not be permitted and result in an error"

Problem is that different versions of the JSON spec have different sets of "objects capable of JSON representation". In particular, the original Douglas Crockford rfc only allowed objects or arrays at the top level, but newer versions allow any valid JSON value (including numbers and booleans) at the top level, so true and 42 are valid JSON documents.

We should decide which behaviour we want, and be clear in the spec what is allowed at the top level.

See previous discussion here (which was concluded on the mistaken assumption that the 2006 rfc was the latest one), and https://github.com/ably/ably-ios/issues/553#issuecomment-267318463 for ios discussion that brought this up.

┆Issue is synchronized with this Jira Task by Unito

paddybyers commented 7 years ago

I seem to remember also that there were interoperability issues with some libraries only supporting objects and arrays.

SimonWoolf commented 7 years ago

Matt mentioned a ruby problem in the ably-js issue, (JSON.parse("42") throws), but playing around now, it looks like you can do JSON.parse("42", quirks_mode: true) and that works.

If we think other languages are going to have trouble with top-level numbers etc. though, then we can decide we only support the older spec - that's fine, as long as we agree on and document that. @mattheworiordan what do you think?

(Side note: would be nice if the ietf did document versioning better, there's nothing in https://www.ietf.org/rfc/rfc4627.txt to indicate there's a newer version of the rfc)

tcard commented 7 years ago

RSL4a is a bit underspeficying in what "objects capable of JSON representation" means (according to the JSON spec? implementing some JSONEncodable interface or similar? is that "object" as in "JSON object" or as in "value"?), but arguably then RSL4c3 RSL4d3 specify what it means more precisely. So we do choose a particular version of the JSON spec, although it would be nice to make that clearer.

Note that, if we do this, then strings and null can be encoded in two different ways: as currently, plus as JSON documents.

Personally I was surprised at first too that not any JSON value was supported.

As a side note, what I'd really like is to give the possibility to users to decode JSON as they want. Right now we convert JSON payloads to whatever in-language representation we deem suitable, but that may not match users' needs, and I'd say in the general case some surprising and potentially buggy stuff can happen, like ints being transformed to floats over the wire. (This is already a problem since numbers can be in JSON arrays and objects.) Normally you want to serialize and deserialize a class, and it may not be so easy (definitely not as efficient) to encode/decode it as/from JSON-like values (hash maps, untyped arrays, "numbers", etc.) instead of directly as/from JSON text.

mattheworiordan commented 7 years ago

I personally would keep things simple and go with the original spec at https://www.ietf.org/rfc/rfc4627.txt for now. Any chance at this stage would require work on all libraries and it's not a problem unless we're not clear. Also, if a string value was set for data, would we then encode it as a JSON string or just a string. I am not sure it would make any difference, but it gets confusing if users then provide an int for example.

paddybyers commented 7 years ago

RSL4a is a bit underspeficying in what "objects capable of JSON representation" means (according to the JSON spec? implementing some JSONEncodable interface or similar? is that "object" as in "JSON object" or as in "value"?)

We should be precise about which JSON values are allowed, but it is deliberately vague about how that is represented in the API in each language, because each language and library will have their own constraints.

Note that, if we do this, then strings and null can be encoded in two different ways: as currently, plus as JSON documents.

In the weakly typed languages I think we would still define a precedence that says that certain types are passed without JSON encoding where they can; and where this isn't possible, but a JSON encoding is possible, then they are encoded.

As a side note, what I'd really like is to give the possibility to users to decode JSON as they want.

Yes, but the issue is achieving the equivalent thing for encoding; in Java for example, we check to see whether or not a given value is an instance of (gson) JSONElement, which forces anyone using the client to use that library, at least for encoding.

I think the answer to both of these concerns, which we can save for a future API version, is to allow callers to control the encoding, and intercept individual decoding steps, so then they are free to do what they like, and assume responsibility themselves for interoperability between their various clients if ew permit non-standard encoding formats.

I personally would keep things simple and go with the original spec at https://www.ietf.org/rfc/rfc4627.txt for now.

I agree; there are just too many other more important things to be doing right now.

mattheworiordan commented 7 years ago

So is the action here simply to state that when we refer to object capable of JSON representation that means we adhere to the RFC 4627 spec and leave it like that?

paddybyers commented 7 years ago

So is the action here simply to state that when we refer to object capable of JSON representation that means we adhere to the RFC 4627 spec and leave it like that?

The RFC defines which JSON values are allowed, but the requirement is meant to capture the idea that the given value can be encoded as JSON in a way that is understood by the library. Ie the library defines "is capable of JSON representation" to mean x where x is "has a toJSON() method" or "is an instance of JSONElement .

funkyboy commented 6 years ago

I'd say we adhere to RFC 4627 and call it a day. Our SDKs will deal with (or avoid, or raise exceptions about) cases like 42.toJSON() internally.

sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-2820