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

Fails to parse COMMA-separated list of calendar addresses in quoted-strings (e.g. MEMBER parameter) #634

Open
st3iny opened this issue Oct 27, 2023 · 7 comments · May be fixed by #702
Open
Labels

Comments

@st3iny
Copy link

st3iny commented Oct 27, 2023

RFC: https://www.rfc-editor.org/rfc/rfc5545#section-3.2.11

The builtin ics parser does not handle parsing COMMA-separated list of calendar addresses in quoted-strings correctly.

Note: The ics example data (attendee property) is valid and was copied as is from the RFC.

Example

const ics = 'ATTENDEE;MEMBER="mailto:projectA@example.com","mailto:projectB@example.com":mailto:janedoe@example.com'
const prop = ICAL.Property.fromString(ics)

Expected

[
  "attendee",
  {
    "member": [
      "mailto:projectA@example.com",
      "mailto:projectB@example.com"
    ]
  },
  "cal-address",
  "mailto:janedoe@example.com"
]

Received

[
  "attendee",
  {
    "member": [
      "mailto:projectA@example.com",
      "mailto:projectB@example.com"
    ]
  },
  "cal-address",
  "projectA@example.com\",\"mailto:projectB@example.com\":mailto:janedoe@example.com"
]

The parser stops at the colon inside the first quoted-string MEMBER parameter. But for some reason both member parameters are still parsed correctly. Only the property value is not parsed correctly.

@onny
Copy link

onny commented Oct 31, 2023

@st3iny I'm unable to reproduce the issue. Here is my script:

const ICAL = require('ical.js');

var iCalendarData = [
    'BEGIN:VCALENDAR',
    'CALSCALE:GREGORIAN',
    'PRODID:-//Example Inc.//Example Calendar//EN',
    'VERSION:2.0',
    'BEGIN:VEVENT',
    'DTSTAMP:20080205T191224Z',
    'DTSTART:20081006',
    'SUMMARY:Planning meeting',
    'ATTENDEE;CN=MyGroup;CUTYPE=GROUP;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANTRSVP=TRUE;SCHEDULE-STATUS1.1:mailto:mygroup@localhost',
    'ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost";CN=user1;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en;SCHEDULE-STATUS=1.1:mailto:user1@localhost',
    'ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost";CN=user2;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en;SCHEDULE-STATUS=1.1:mailto:user2@localhost',
    'ORGANIZER;CN=admin:mailto:admin@localhost',
    'UID:4088E990AD89CB3DBB484909',
    'END:VEVENT',
    'END:VCALENDAR'
].join("\r\n");


var jcalData = ICAL.parse(iCalendarData);
var vcalendar = new ICAL.Component(jcalData);
var vevent = vcalendar.getFirstSubcomponent('vevent');
var attendees = vevent.getAllProperties('attendee');

attendees.forEach(function(attendee) {
  console.log('Attendee:', attendee);
});

Here is the result of one attendee:

Attendee: <ref *1> e {
  _parent: t {
    _hydratedPropertyCount: 3,
    _hydratedComponentCount: 0,
    _timezoneCache: null,
    jCal: [ 'vevent', [Array], [] ],
    parent: t {
      _hydratedPropertyCount: 0,
      _hydratedComponentCount: 1,
      _timezoneCache: Map(0) {},
      jCal: [Array],
      parent: null,
      _components: [Array]
    },
    _properties: [ <3 empty items>, [e], [e], [Circular *1] ]
  },
  jCal: [
    'attendee',
    {
      member: [Array],
      cn: 'user2',
      cutype: 'INDIVIDUAL',
      partstat: 'NEEDS-ACTION',
      role: 'REQ-PARTICIPANT',
      rsvp: 'TRUE',
      language: 'en',
      'schedule-status': '1.1'
    },
    'cal-address',
    'mailto:user2@localhost'
  ],
  isDecorated: false,
  isMultiValue: false,
  isStructuredValue: false
}

@st3iny
Copy link
Author

st3iny commented Nov 1, 2023

Thanks for investigating further.

It seems to break only if the MEMBER parameter is the last one before the actual value of the ATTENDEE property.

Take a look at the following example:

const ICAL = require('ical.js');

var iCalendarData = [
    'ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost";CN=user1;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en;SCHEDULE-STATUS=1.1:mailto:user1@localhost',
    'ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost":mailto:user2@localhost',
]

for (const attendeeIcs of iCalendarData) {
    const attendee = ICAL.Property.fromString(attendeeIcs);
    console.log('expected:', attendeeIcs)
    console.log('actual:  ', attendee.toICALString())
    console.log('equal?   ', attendee.toICALString() === attendeeIcs)
    console.log('--------------------------------------------------')
}

It will yield:

expected: ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost";CN=user1;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en;SCHEDULE-STATUS=1.1:mailto:user1@localhost
actual:   ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost";CN=user1;CUTYPE=INDIVIDUAL;PARTSTAT=NEEDS-ACTION;ROLE=REQ-PARTICIPANT;RSVP=TRUE;LANGUAGE=en;SCHEDULE-STATUS=1.1:mailto:user1@localhost
equal?    true
--------------------------------------------------
expected: ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost":mailto:user2@localhost
actual:   ATTENDEE;MEMBER="mailto:mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost":mygroup@localhost","mailto:mygroup2@localhost","mailto:mygroup3@localhost":mailto:user2@localhost
equal?    false
--------------------------------------------------

@kewisch kewisch added the bug label Apr 1, 2024
@kewisch
Copy link
Owner

kewisch commented Apr 14, 2024

@dilyanpalauzov this seems like it is right up your alley with the escaping and parameter parsing fixes, would you be willing to take a look?

@kewisch
Copy link
Owner

kewisch commented Apr 14, 2024

    test.only('with quoted multi-value parameter', function() {
      let attendee = ICAL.Property.fromString(
        'ATTENDEE;MEMBER=' +
        '"mailto:mygroup@localhost",' +
        '"mailto:mygroup2@localhost",' +
        '"mailto:mygroup3@localhost":' +
        'mailto:user2@localhost'
      );

      let expected = [
        'attendee',
        {
          member: [
            'mailto:mygroup@localhost',
            'mailto:mygroup2@localhost',
            'mailto:mygroup3@localhost'
          ]
        },
        'cal-address',
        'mailto:user2@localhost'
      ]

      assert.deepEqual(attendee.toJSON(), expected);
    });

@dilyanpalauzov
Copy link
Contributor

@dilyanpalauzov this seems like it is right up your alley with the escaping and parameter parsing fixes, would you be willing to take a look?

I was using ical.js back in 2020-2022. Now my focus moved to different things. I am glad that some of the changes, proposed by me at that time, were integrated today. As I have seen no progress with PRs, I have stopped creating new ones. My changes are visible at https://github.com/dilyanpalauzov/ical.js/commits/main/ . I think actually, there is only one change I have not uploaded: recur_expansion: EXDATE can be DATE and DTSTART can be DATE-TIME. That said, I am not willing to take a look.

The last example above : expected = [ … ] does not contain the property value 'mailto:user2@localhost'.

@kewisch
Copy link
Owner

kewisch commented May 1, 2024

Thanks, I've updated the test case. Hoping to show that I'm more active on this repo now so it is worth putting some new effort into this :)

kewisch added a commit that referenced this issue May 1, 2024
@onny
Copy link

onny commented Jun 10, 2024

Bug still present in 2.0.1, going to look into a fix for this but not sure if I can manage 😅

@onny onny linked a pull request Jun 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants