mangstadt / ez-vcard

A vCard parser library for Java
Other
399 stars 92 forks source link

Unusual REV value format not detected as Revision #73

Closed rfc2822 closed 7 years ago

rfc2822 commented 7 years ago

Hello,

The following REV line (part of a VCard 4) seems not to be parsed as Revision by ez-vcard:

REV;VALUE=timestamp:2016-11-19T17:44:22.057Z

However, ISO.8601.2004 (which is referenced by the VCard4 REV/TIMESTAMP definition)

I don't know whether this really applies to REV values, but I think the above value is allowed and should be parsed by correctly.

Further reference: https://forums.bitfire.at/topic/1259/davdroid-1-3-4-1-ose-nextcloud-11-0-beta-carddav-sync-error/

rfc2822 commented 7 years ago

It seems that there are many ways to express date and time, see https://forums.bitfire.at/post/7504

I guess you will have to import a whole library or something like that to understand all these crazy formats … but a good start would be to parse the format with the milliseconds, too.

mangstadt commented 7 years ago

Note: I can't seem to find a freely available copy of ISO.8601.2004 online. I found something that looks like it, but not 100% sure it's exactly the same.

Section 4.3.2, which the vCard TIMESTAMP data type adheres to, doesn't say anything about decimal fractions, so my opinion is that REV doesn't have to support it. But what vCard producers choose to do in the wild is another thing, so it might be worth supporting if a lot of producers decide to break with the spec.

As you alluded to in your forum post, it appears there are other ways of representing a timestamp, which ez-vcard does not account for (see below). So yeah, that's a thing.

The time elements of a date and time of day expression shall be written in the following sequence. a) For calendar dates: year – month – day of the month – time designator – hour – minute – second – zone designator b) For ordinal dates: year – day of the year – time designator – hour – minute – second – zone designator c) For week dates: year – week designator – week – day of the week – time designator – hour – minute – second – zone designator

The Appendix B.2.3 you mention contains examples for reduced accuracy date formats, which are not applicable to the TIMESTAMP data type.

rfc2822 commented 7 years ago

In my tests, there are other time formats which don't work. Currently, Nextcloud Contacts uses the REV:20161218T201900 format, which is not recognized by ez-vcard. I have used this test class which tests the four formats mentioned in RFC 6350 4.3.5 TIMESTAMP:

package at.bitfire.vcard4android;

import static org.junit.Assert.*;
import org.junit.Test;

import ezvcard.Ezvcard;
import ezvcard.VCard;

public class EzVCardTest {

    @Test
    public void testREV_UTC() {
        VCard vCard = Ezvcard.parse("BEGIN:VCARD\r\n" +
                "VERSION:4.0\r\n" +
                "REV:20161218T201900Z\r\n" +
                "END:VCARD").first();
        assertNotNull(vCard.getRevision());
    }

    @Test
    public void testREV_WithoutTZ() {
        VCard vCard = Ezvcard.parse("BEGIN:VCARD\r\n" +
                "VERSION:4.0\r\n" +
                "REV:20161218T201900\r\n" +
                "END:VCARD").first();
        assertNotNull(vCard.getRevision());
    }

    @Test
    public void testREV_TZHourOffset() {
        VCard vCard = Ezvcard.parse("BEGIN:VCARD\r\n" +
                "VERSION:4.0\r\n" +
                "REV:20161218T201900-05\r\n" +
                "END:VCARD").first();
        assertNotNull(vCard.getRevision());
    }

    @Test
    public void testREV_TZHourAndMinOffset() {
        VCard vCard = Ezvcard.parse("BEGIN:VCARD\r\n" +
                "VERSION:4.0\r\n" +
                "REV:20161218T201900-0530\r\n" +
                "END:VCARD").first();
        assertNotNull(vCard.getRevision());
    }

}

and tests fail for

mangstadt commented 7 years ago

Added support for the above date formats that failed in 9a6977e4945bd4d3b00f5d92d7d8a7c6f90a9641. Timestamps without UTC offsets will be assumed to be in the local computer's timezone.

rfc2822 commented 7 years ago

Thank you very much!

zeehio commented 7 years ago

Thanks!

If I read the code properly, in the end timestamps with milliseconds do not seem to be supported. Let's hope no client uses them (nextcloud/contacts currently is using them) https://github.com/nextcloud/contacts/pull/65

mangstadt commented 7 years ago

Ok, I will add that. Can you remind me what that timestamp format looks like?

zeehio commented 7 years ago

20161227T155712.034Z

I would express it like this:

\d{4}\d{2}\d{2}T\d{2}\d{2}\d{2}(\.\d*)?Z
mangstadt commented 7 years ago

Will the milliseconds always be 3 digits long?

zeehio commented 7 years ago

No, 1-6 digits can be expected

mangstadt commented 7 years ago

Can you elaborate? Thanks. :)

zeehio commented 7 years ago

http://stackoverflow.com/questions/25842840/representing-fraction-of-second-with-iso-86012004

The ISO does not specify the number of digits, however I haven't seen more than six ever.

mangstadt commented 7 years ago

Added in 27f440413b56425ab551a507078db5e45c812fd4. If it's greater than 3 digits, then it is rounded to the nearest millisecond.

zeehio commented 7 years ago

Thanks!