untitaker / rust-vobject

VObject parser and generator for Rust
https://docs.rs/vobject/
MIT License
16 stars 7 forks source link

Parser silently ignores multiple N: fields #32

Open rogarb opened 2 years ago

rogarb commented 2 years ago

Parsing a string containing two name fields ("N:") doesn't produce an error but silently ignores the name fields and produces a VCard with name() returning None:

use vobject::vcard::Vcard;                                                      

fn main() {                                                                     
    let input = "\                                                              
BEGIN:VCARD\r\n\                                                                
VERSION:3.0\r\n\                                                                
FN:Test person\r\n\                                                             
N:Person;Test;;;\r\n\                                                           
N:P;Test;;;\r\n\                                                                
TEL:+30666777888\r\n\                                                           
TEL:+34666777888\r\n\                                                           
TEL:+33666777888\r\n\                                                           
EMAIL;TYPE=INTERNET:alpha_beta@test.com\r\n\                                    
EMAIL;TYPE=INTERNET:alpha_beta3@test.com\r\n\                                   
END:VCARD\r\n\                                                                  
";                                                                              
    let vcard = Vcard::build(input).unwrap();                                   
    assert!(vcard.name().is_some());                                            
}     

When executed, the code above panics due to the assertion:

thread 'main' panicked at 'assertion failed: vcard.name().is_some()', src/main.rs:18:5

A pretty print shows that both "N:" fields are effectively parsed and added to the underlying Component:

Vcard(
    Component {
        name: "VCARD",
        props: {
           [...]
            "N": [
                Property {
                    name: "N",
                    params: {},
                    raw_value: "Person;Test;;;",
                    prop_group: None,
                },
                Property {
                    name: "N",
                    params: {},
                    raw_value: "P;Test;;;",
                    prop_group: None,
                },
            ],
           [...]
        },
        subcomponents: [],
    },
)

Shouldn't either the parser return a parsing error or name() return Some(Name)?

lucc commented 1 month ago

I think the problem is that vcard.name() is implemented by make_getter_function_for_optional! but it should be implemented by make_getter_function_for_values! instead. If you read these macros you can see that you might be able to get all N values with something like vcard.get_all("N").