Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unusual REV value format not detected as Revision #73

Closed
rfc2822 opened this issue Nov 27, 2016 · 14 comments
Closed

Unusual REV value format not detected as Revision #73

rfc2822 opened this issue Nov 27, 2016 · 14 comments

Comments

@rfc2822
Copy link
Contributor

rfc2822 commented Nov 27, 2016

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)

  • allows the following format: YY-MM-DDThh:mm:ssZ,
  • says that "If necessary for a particular application a decimal fraction of hour, minute or second may be included."

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 rfc2822 changed the title Unusual REV value not detected as Revision Unusual REV value format not detected as Revision Nov 27, 2016
@rfc2822
Copy link
Contributor Author

rfc2822 commented Nov 27, 2016

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
Copy link
Owner

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
Copy link
Contributor Author

rfc2822 commented Dec 18, 2016

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

  • testREV_TZHourOffset and
  • testREV_WithoutTZ

mangstadt added a commit that referenced this issue Dec 25, 2016
* All date formats are now parsed using a single regular expression.
This significantly improves performance.
* Added support for parsing dates that lack the minute component of the
UTC offset.
* Added support for parsing dates that lack UTC offsets. These dates are
parsed under the local computer's timezone.

#73
@mangstadt
Copy link
Owner

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

@rfc2822
Copy link
Contributor Author

rfc2822 commented Dec 25, 2016

Thank you very much!

@zeehio
Copy link

zeehio commented Dec 25, 2016

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) nextcloud/contacts#65

@mangstadt
Copy link
Owner

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

@mangstadt mangstadt reopened this Dec 27, 2016
@zeehio
Copy link

zeehio commented Dec 27, 2016

20161227T155712.034Z

I would express it like this:

\d{4}\d{2}\d{2}T\d{2}\d{2}\d{2}(\.\d*)?Z

@mangstadt
Copy link
Owner

Will the milliseconds always be 3 digits long?

@zeehio
Copy link

zeehio commented Dec 27, 2016

No, 1-6 digits can be expected

@mangstadt
Copy link
Owner

Can you elaborate? Thanks. :)

@zeehio
Copy link

zeehio commented Dec 27, 2016

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
Copy link
Owner

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

@zeehio
Copy link

zeehio commented Dec 28, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants