search

kewisch / ical.js

Javascript parser for ics (rfc5545) and vcard (rfc6350) data
https://kewisch.github.io/ical.js/
Mozilla Public License 2.0
992 stars 137 forks source link

Fail to convert jCard to vCard #266

Closed dbanetto closed 4 months ago

dbanetto commented 7 years ago

Tested on master and version 1.2.2.

Was trying to convert a from a jCard to a vCard string and have been using the jCard provided in the RFC

Expected behaviour:

The Component will return a string containing a vCard.

Actual behaviour: Throws an error in the browser console.

TypeError: comps is undefined ical.js:1389:9
    stringify.component() ical.js:1389
    Component.prototype.toString() ical.js:2642
    window.onload() _display:52

Example:

jsfiddle example: http://jsfiddle.net/gegxngqj/

kewisch commented 7 years ago

This is indeed a bug in the library, I'm surprised this didn't come up earlier. Maybe it was a recent regression. The problem is that the component stringifier assumes that it is getting an object like:

["vcalendar", [ ...properties... ], [ ...subcomponents... ].

The vcard object is just:

["vcard", [ ...properties... ] ]

A simple fix would be to accept not passing subcomponents, by checking components.length > 1 around the place where the error occurs. Some tests to ensure this doesn't happen again would be great too. The other direction should also be tested, making sure that getting the jCard data from a vCard string produces arrays so that there is no third member.

jCal requires the third element in the array to exist, even if it is an empty array. I can accept being lenient here, but ideally we'd indicate an error if the third element is missing for jCal.

Would you be willing to work on a fix for this?

dbanetto commented 7 years ago

Thank you for the great explanation. I'll have a go at fixing this and comment here if I run into any problems.

kewisch commented 7 years ago

Thanks! Also feel free to catch me on IRC if you have questions. My nick is Fallen on irc.mozilla.org.

kewisch commented 4 months ago

This was fixed in 2016, we just didn't close the issue. Thanks again!