mangstadt / ez-vcard

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

[Feature] Ignore invalid PREF parameter #76

Closed rfc2822 closed 7 years ago

rfc2822 commented 7 years ago

There are some clients/servers which generate invalid VCard4 PREF parameters like this:

TEL;CELL=;PREF=:+12345

In this case, ez-vcard throws an IllegalStateException, making the whole VCard unusable:

java.lang.IllegalStateException: [Error 15] PREF parameter value is malformed and could not be parsed. Retrieve its raw text values instead by calling property.getParameters().get("PREF").

Would it be possible to ignore such errors, and continue to parse the VCard while ignoring the invalid PREF value? Maybe with an extra setting?

mangstadt commented 7 years ago

The code should have been checking for invalid PREF values, but it was not. Fixed in 157d5ca4424ac4c91d070c9ad25d5c105227fed7

rfc2822 commented 7 years ago

Hello,

The problem still occurs when parsing VCards. You can use this test:

    @Test
    public void testInvalidPref() {
        VCard vCard = Ezvcard.parse("BEGIN:VCARD\r\n" +
                "VERSION:4.0\r\n" +
                "TEL;CELL=;PREF=:+12345\r\n" +
                "END:VCARD").first();
        assertEquals("+12345", vCard.getTelephoneNumbers().get(0).getText());
        assertNull(vCard.getTelephoneNumbers().get(0).getPref());
    }

getPref() then fails with an IllegalStateException, which is a runtime exception and thus unexpected.

mangstadt commented 7 years ago

This is expected behavior.

The vCard is parsed without error. Ezvcard.parse() executes without throwing an exception.

Upon calling getPref(), it tries to parse the PREF parameter's string value as an integer and cannot because the parameter value is an empty string.

I guess this should be documented in the Javadocs of Telephone#getPref(). It is documented in VCardParameters.getPref().

rfc2822 commented 7 years ago

I see. I was just confused because of the runtime exception – maybe a checked InvalidValueException would be more safe, but I will try/catch the getPref(), thanks.

mangstadt commented 7 years ago

I wanted to use an unchecked exception because checked exceptions can clutter your code if overused. I'd also rather not create a new exception class to avoid cluttering the code base. :thinking: