Skip to content
New issue

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

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

Already on GitHub? # to your account

Handle Vobjects without closing tag #559

Merged
merged 2 commits into from
Aug 17, 2022
Merged

Conversation

sash04ek
Copy link
Contributor

There are cases when objects do not have closing tags (for example, END: VCALENDAR), this fix solves this issue.

@staabm
Copy link
Member

staabm commented Dec 16, 2021

Thanks for the contribution.

Could you come up with a reproducible example?
Could you build a unit test for it?

@sash04ek
Copy link
Contributor Author

sash04ek commented Dec 16, 2021

Could you come up with a reproducible example?

For example, below is the content of the invitation received by email.

BEGIN:VCALENDAR
METHOD:REQUEST
PRODID:Microsoft Exchange Server 2010
VERSION:2.0
BEGIN:VTIMEZONE
TZID:W. Europe Standard Time
BEGIN:STANDARD
DTSTART:16010101T030000
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-1SU;BYMONTH=10
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:16010101T020000
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
RRULE:FREQ=YEARLY;INTERVAL=1;BYDAY=-1SU;BYMONTH=3
END:DAYLIGHT
END:VTIMEZONE
BEGIN:VEVENT
ORGANIZER;CN=Test:mailto:test@mail.com
ATTENDEE;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=TRUE;CN=Test2:mailto:test2@mail.com
DESCRIPTION;LANGUAGE=de-DE:Description
RRULE:FREQ=WEEKLY;UNTIL=20211118T080000Z;INTERVAL=1;BYDAY=TH;WKST=SU
UID:040000008200E00074C5B7101A82E008000000004F85CD5A66D1D701000000000000000
 010000000A11CE9A71EFE5441B44ABF525F5F642E
SUMMARY;LANGUAGE=de-DE:Summary
DTSTART;TZID=W. Europe Standard Time:20211118T090000
DTEND;TZID=W. Europe Standard Time:20211118T103000
CLASS:PUBLIC
PRIORITY:5
DTSTAMP:20211209T080925Z
TRANSP:OPAQUE
STATUS:CONFIRMED
SEQUENCE:4
LOCATION;LANGUAGE=de-DE:
X-MICROSOFT-CDO-APPT-SEQUENCE:4
X-MICROSOFT-CDO-OWNERAPPTID:2119901775
X-MICROSOFT-CDO-BUSYSTATUS:TENTATIVE
X-MICROSOFT-CDO-INTENDEDSTATUS:BUSY
X-MICROSOFT-CDO-ALLDAYEVENT:FALSE
X-MICROSOFT-CDO-IMPORTANCE:1
X-MICROSOFT-CDO-INSTTYPE:1
X-MICROSOFT-BODYCONTAINSMEETINGMESSAGEONLYNOTES:TRUE
X-MICROSOFT-RECURRENCETRUNCATED:
END:VEVENT

@sash04ek
Copy link
Contributor Author

Could you build a unit test for it?

I have not build tests for it. I created a simple example that works without problems after fixing.

$vcalendar = \Sabre\VObject\Reader::read(
    fopen('test.ics','r')
);

@phil-davis
Copy link
Contributor

There was 1 failure:

1) Sabre\VObject\ReaderTest::testReadBrokenLine
Failed asserting that exception of type "Sabre\VObject\ParseException" is thrown.

phpvfscomposer:///home/runner/work/vobject/vobject/vendor/phpunit/phpunit/phpunit:97

FAILURES!
Tests: 1735, Assertions: 2677, Failures: 1.

This test now fails - it is:

    public function testReadBrokenLine()
    {
        $this->expectException(ParseException::class);
        $data = "BEGIN:VCALENDAR\r\nPROPNAME;propValue";
        $result = Reader::read($data);
    }

..
with the code change here, we do not throw an exception any longer. So we have a "reverse" test case already here. I will have a look and adjust...

There are cases when objects do not have closing tags (for example, END: VCALENDAR), this fix solves this issue.
@phil-davis phil-davis changed the title Update MimeDir.php Handle Vobjects without closing tag Aug 17, 2022
@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #559 (ccaec1c) into master (3d5eda5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #559   +/-   ##
=========================================
  Coverage     98.35%   98.35%           
- Complexity     1896     1897    +1     
=========================================
  Files            71       71           
  Lines          4547     4549    +2     
=========================================
+ Hits           4472     4474    +2     
  Misses           75       75           
Impacted Files Coverage Δ
lib/Parser/MimeDir.php 99.57% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@phil-davis
Copy link
Contributor

This change allows the last END:VCALENDAR or END:VCARD to be missing and it will "simulate" that when the parser hits end-of-input. I guess that there are clients that can send such partly-malformed Vobjects, and it does not hurt if we accept them - the code here just "guesses" a missing end tag. It does not have to actually make-up any real data.

@phil-davis phil-davis merged commit 86d8c52 into sabre-io:master Aug 17, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants