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

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
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

Successfully merging this pull request may close these issues.

None yet

3 participants