sabre-io / vobject

:date: The VObject library for PHP allows you to easily parse and manipulate iCalendar and vCard objects
http://sabre.io/vobject/
BSD 3-Clause "New" or "Revised" License
570 stars 126 forks source link

Do not escape ',' and ';' when serializing to JSON #417

Open snailuj opened 6 years ago

snailuj commented 6 years ago

I want to send Non-Standard Properties with JSON string values to a browser-based client as part of a JSON-serialized VEVENT object (i.e. a Non-Standard prop with a JSON value nested in a jCal VObject).

But when I call jsonSerialize() on my VObject, the Property value is serialized like the following: "{\"location\":\"The Domain\"\\,\"meetingTime\":\"05:30:00 GMT+1000\"\\,\"priceAUD\":220\\}"

Basically, the JSON serialize routine in VObject is inserting a literal backslash before each comma (note the "\\," parts of that string)

Now, I know that the iCalendar spec says that Property values MUST have escaped commas. But the current behaviour is a prob for my use case, because when I try to parse the prop value in Javascript, JSON.parse() barfs if it gets escaped commas like the above.

Not sure where the jCal spec stands on this, but I suppose it has the same requirement about escaping commas. However, I don't imagine there is actually any need for that, since (in contrast to a Property value inside an iCalendar file) commas hold no special meaning in JSON string literals?

I've overridden getJsonValue() in Sabre\VObject\Property\Unknown to call a modified getJsonFriendlylMimeDirValue() instead of inheriting it from Property\Text.

Does anybody know if this is this an OK approach, or will it break down-stream functionality?

PS I hit ENTER too soon and accidentally submitted an incomplete issue. Fumbly.

snailuj commented 6 years ago

Just found this reference in the jCal spec:

To correctly implement this format, it is critical that if the default type is not known that the type "unknown" is used. If this requirement is ignored and for example "text" is used, additional escaping may occur which breaks round-tripping values.

To be clear, the type "unknown" is being correctly chosen and output for Non-Std Props by VObject::jsonSerialize(). But in the Unknown class, the escaping logic is inherited from Text, which I believe may be a mistake.

And also:

When converting from iCalendar to jCal: First, iCalendar lines MUST be unfolded. Afterwards, any iCalendar escaping MUST be unescaped. Finally, JSON escaping, as described in Section 7 of [RFC7159], MUST be applied. The reverse order applies when converting from jCal to iCalendar, which is further described in Section 4.

(My emphasis added)

If I'm reading correctly, the jCal spec states that Unknown Properties should not be using the same escaping logic as Text.

I'm now wondering about whether during serialization to jCal there is any need to escape ',' in Text or FlatText properties either. Is it the case that escaping comma (and semi-colon for that matter) are iCalendar-specific requirements that don't server any purpose in a jCal context?