diff --git a/.gitattributes b/.gitattributes index 602c23776..a830033a0 100644 --- a/.gitattributes +++ b/.gitattributes @@ -4,5 +4,6 @@ /.gitignore export-ignore /.php_cs.dist export-ignore /.travis.yml export-ignore +/.php-cs-fixer.dist.php export-ignore /CHANGELOG.md export-ignore /phpstan.neon export-ignore diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d665e841..775b3389f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT - name: Cache composer dependencies - uses: actions/cache@v3 + uses: actions/cache@v4 with: path: ${{ steps.composer-cache.outputs.dir }} # Use composer.json for key, if composer.lock is not committed. @@ -59,5 +59,5 @@ jobs: run: vendor/bin/phpunit --configuration tests/phpunit.xml --coverage-clover clover.xml - name: Code Coverage - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 if: matrix.coverage != 'none' diff --git a/composer.json b/composer.json index 263f6a166..65765bca3 100644 --- a/composer.json +++ b/composer.json @@ -38,10 +38,10 @@ "sabre/xml" : "^3.0 || ^4.0" }, "require-dev" : { - "friendsofphp/php-cs-fixer": "^3.38", + "friendsofphp/php-cs-fixer": "^3.54", "phpunit/phpunit" : "^9.6", "phpunit/php-invoker" : "^2.0 || ^3.1", - "phpstan/phpstan": "^1.10" + "phpstan/phpstan": "^1.11" }, "suggest" : { "hoa/bench" : "If you would like to run the benchmark scripts" diff --git a/lib/Document.php b/lib/Document.php index fa789a8de..370b02e0e 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -167,7 +167,7 @@ public function createComponent(string $name, ?array $children = null, bool $def * * @throws InvalidDataException */ - public function createProperty(string $name, $value = null, ?array $parameters = null, ?string $valueType = null): Property + public function createProperty(string $name, $value = null, ?array $parameters = null, ?string $valueType = null, ?int $lineIndex = null, ?string $lineString = null): Property { // If there's a . in the name, it means it's prefixed by a group name. if (false !== ($i = strpos($name, '.'))) { @@ -204,7 +204,7 @@ public function createProperty(string $name, $value = null, ?array $parameters = $parameters = []; } - return new $class($this, $name, $value, $parameters, $group); + return new $class($this, $name, $value, $parameters, $group, $lineIndex, $lineString); } /** diff --git a/lib/ITip/Broker.php b/lib/ITip/Broker.php index 9cbc3f72b..be6ba29d7 100644 --- a/lib/ITip/Broker.php +++ b/lib/ITip/Broker.php @@ -333,7 +333,10 @@ protected function processMessageReply(Message $itipMessage, ?VCalendar $existin // Finding all the instances the attendee replied to. foreach ($itipMessage->message->VEVENT as $vevent) { - $recurId = isset($vevent->{'RECURRENCE-ID'}) ? $vevent->{'RECURRENCE-ID'}->getValue() : 'master'; + // Use the Unix timestamp returned by getTimestamp as a unique identifier for the recurrence. + // The Unix timestamp will be the same for an event, even if the reply from the attendee + // used a different format/timezone to express the event date-time. + $recurId = isset($vevent->{'RECURRENCE-ID'}) ? $vevent->{'RECURRENCE-ID'}->getDateTime()->getTimestamp() : 'master'; $attendee = $vevent->ATTENDEE; $instances[$recurId] = $attendee['PARTSTAT']->getValue(); if (isset($vevent->{'REQUEST-STATUS'})) { @@ -346,7 +349,8 @@ protected function processMessageReply(Message $itipMessage, ?VCalendar $existin // all the instances where we have a reply for. $masterObject = null; foreach ($existingObject->VEVENT as $vevent) { - $recurId = isset($vevent->{'RECURRENCE-ID'}) ? $vevent->{'RECURRENCE-ID'}->getValue() : 'master'; + // Use the Unix timestamp returned by getTimestamp as a unique identifier for the recurrence. + $recurId = isset($vevent->{'RECURRENCE-ID'}) ? $vevent->{'RECURRENCE-ID'}->getDateTime()->getTimestamp() : 'master'; if ('master' === $recurId) { $masterObject = $vevent; } @@ -393,7 +397,10 @@ protected function processMessageReply(Message $itipMessage, ?VCalendar $existin $newObject = $recurrenceIterator->getEventObject(); $recurrenceIterator->next(); - if (isset($newObject->{'RECURRENCE-ID'}) && $newObject->{'RECURRENCE-ID'}->getValue() === $recurId) { + // Compare the Unix timestamp returned by getTimestamp with the previously calculated timestamp. + // If they are the same, then this is a matching recurrence, even though its date-time may have + // been expressed in a different format/timezone. + if (isset($newObject->{'RECURRENCE-ID'}) && $newObject->{'RECURRENCE-ID'}->getDateTime()->getTimestamp() === $recurId) { $found = true; } --$iterations; diff --git a/lib/Parser/MimeDir.php b/lib/Parser/MimeDir.php index 1ba2f1c59..db5ded667 100644 --- a/lib/Parser/MimeDir.php +++ b/lib/Parser/MimeDir.php @@ -80,6 +80,12 @@ public function parse($input = null, int $options = 0): ?Document $this->setInput($input); } + if (!\is_resource($this->input)) { + // Null was passed as input, but there was no existing input buffer + // There is nothing to parse. + throw new ParseException('No input provided to parse'); + } + if (0 !== $options) { $this->options = $options; } @@ -463,7 +469,7 @@ protected function readProperty(string $line) } } - $propObj = $this->root->createProperty($property['name'], null, $namedParameters); + $propObj = $this->root->createProperty($property['name'], null, $namedParameters, null, $this->startLine, $line); foreach ($namelessParameters as $namelessParameter) { $propObj->add(null, $namelessParameter); diff --git a/lib/Property.php b/lib/Property.php index 574099e65..f4d0bdc0b 100644 --- a/lib/Property.php +++ b/lib/Property.php @@ -51,6 +51,20 @@ abstract class Property extends Node */ public string $delimiter = ';'; + /** + * The line number in the original iCalendar / vCard file + * that corresponds with the current node + * if the node was read from a file. + */ + public ?int $lineIndex; + + /** + * The line string from the original iCalendar / vCard file + * that corresponds with the current node + * if the node was read from a file. + */ + public ?string $lineString; + /** * Creates the generic property. * @@ -61,7 +75,7 @@ abstract class Property extends Node * @param array $parameters List of parameters * @param string|null $group The vcard property group */ - public function __construct(Component $root, ?string $name, $value = null, array $parameters = [], ?string $group = null) + public function __construct(Component $root, ?string $name, $value = null, array $parameters = [], ?string $group = null, ?int $lineIndex = null, ?string $lineString = null) { $this->name = $name; $this->group = $group; @@ -75,6 +89,14 @@ public function __construct(Component $root, ?string $name, $value = null, array if (!is_null($value)) { $this->setValue($value); } + + if (!is_null($lineIndex)) { + $this->lineIndex = $lineIndex; + } + + if (!is_null($lineString)) { + $this->lineString = $lineString; + } } /** diff --git a/lib/Recur/RRuleIterator.php b/lib/Recur/RRuleIterator.php index ecf1affeb..e2d1bda5c 100644 --- a/lib/Recur/RRuleIterator.php +++ b/lib/Recur/RRuleIterator.php @@ -310,6 +310,14 @@ private function jumpForward(\DateTimeInterface $dt): void */ protected ?\DateTimeInterface $currentDate; + /** + * The number of hours that the next occurrence of an event + * jumped forward, usually because summer time started and + * the requested time-of-day like 0230 did not exist on that + * day. And so the event was scheduled 1 hour later at 0330. + */ + protected int $hourJump = 0; + /** * Frequency is one of: secondly, minutely, hourly, daily, weekly, monthly, * yearly. @@ -427,12 +435,65 @@ private function jumpForward(\DateTimeInterface $dt): void /* Functions that advance the iterator {{{ */ + /** + * Gets the original start time of the RRULE. + * + * The value is formatted as a string with 24-hour:minute:second + */ + protected function startTime(): string + { + return $this->startDate->format('H:i:s'); + } + + /** + * Advances currentDate by the interval. + * The time is set from the original startDate. + * If the recurrence is on a day when summer time started, then the + * time on that day may have jumped forward, for example, from 0230 to 0330. + * Using the original time means that the next recurrence will be calculated + * based on the original start time and the day/week/month/year interval. + * So the start time of the next occurrence can correctly revert to 0230. + */ + protected function advanceTheDate(string $interval): void + { + $this->currentDate = $this->currentDate->modify($interval.' '.$this->startTime()); + } + + /** + * Does the processing for adjusting the time of multi-hourly events when summer time starts. + */ + protected function adjustForTimeJumpsOfHourlyEvent(\DateTimeInterface $previousEventDateTime): void + { + if (0 === $this->hourJump) { + // Remember if the clock time jumped forward on the next occurrence. + // That happens if the next event time is on a day when summer time starts + // and the event time is in the non-existent hour of the day. + // For example, an event that normally starts at 02:30 will + // have to start at 03:30 on that day. + // If the interval is just 1 hour, then there is no "jumping back" to do. + // The events that day will happen, for example, at 0030 0130 0330 0430 0530... + if ($this->interval > 1) { + $expectedHourOfNextDate = ((int) $previousEventDateTime->format('G') + $this->interval) % 24; + $actualHourOfNextDate = (int) $this->currentDate->format('G'); + $this->hourJump = $actualHourOfNextDate - $expectedHourOfNextDate; + } + } else { + // The hour "jumped" for the previous occurrence, to avoid the non-existent time. + // currentDate got set ahead by (usually) 1 hour on that day. + // Adjust it back for this next occurrence. + $this->currentDate = $this->currentDate->sub(new \DateInterval('PT'.$this->hourJump.'H')); + $this->hourJump = 0; + } + } + /** * Does the processing for advancing the iterator for hourly frequency. */ protected function nextHourly($amount = 1): void { + $previousEventDateTime = clone $this->currentDate; $this->currentDate = $this->currentDate->modify('+'.$amount * $this->interval.' hours'); + $this->adjustForTimeJumpsOfHourlyEvent($previousEventDateTime); } /** @@ -441,7 +502,7 @@ protected function nextHourly($amount = 1): void protected function nextDaily($amount = 1): void { if (!$this->byHour && !$this->byDay) { - $this->currentDate = $this->currentDate->modify('+'.$amount * $this->interval.' days'); + $this->advanceTheDate('+'.$amount * $this->interval.' days'); return; } @@ -502,7 +563,7 @@ protected function nextDaily($amount = 1): void protected function nextWeekly($amount = 1): void { if (!$this->byHour && !$this->byDay) { - $this->currentDate = $this->currentDate->modify('+'.($amount * $this->interval).' weeks'); + $this->advanceTheDate('+'.$amount * $this->interval.' weeks'); return; } @@ -524,7 +585,7 @@ protected function nextWeekly($amount = 1): void if ($this->byHour) { $this->currentDate = $this->currentDate->modify('+1 hours'); } else { - $this->currentDate = $this->currentDate->modify('+1 days'); + $this->advanceTheDate('+1 days'); } // Current day of the week @@ -564,13 +625,13 @@ protected function nextMonthly($amount = 1): void // occur to the next month. We Must skip these invalid // entries. if ($currentDayOfMonth < 29) { - $this->currentDate = $this->currentDate->modify('+'.($amount * $this->interval).' months'); + $this->advanceTheDate('+'.($amount * $this->interval).' months'); } else { $increase = $amount - 1; do { ++$increase; $tempDate = clone $this->currentDate; - $tempDate = $tempDate->modify('+ '.($this->interval * $increase).' months'); + $tempDate = $tempDate->modify('+ '.($this->interval * $increase).' months '.$this->startTime()); } while ($tempDate->format('j') != $currentDayOfMonth); $this->currentDate = $tempDate; } @@ -641,6 +702,10 @@ protected function nextMonthly($amount = 1): void } } + // Set the currentDate to the year and month that we are in, and the day of the month that we have selected. + // That day could be a day when summer time starts, and if the time of the event is, for example, 0230, + // then 0230 will not be a valid time on that day. So always apply the start time from the original startDate. + // The "modify" method will set the time forward to 0330, for example, if needed. $this->currentDate = $this->currentDate->setDate( (int) $this->currentDate->format('Y'), (int) $this->currentDate->format('n'), @@ -767,7 +832,7 @@ protected function nextYearly($amount = 1): void } // The easiest form - $this->currentDate = $this->currentDate->modify('+'.($amount * $this->interval).' years'); + $this->advanceTheDate('+'.($amount * $this->interval).' years'); return; } @@ -858,7 +923,7 @@ protected function nextYearly($amount = 1): void (int) $currentYear, (int) $currentMonth, (int) $currentDayOfMonth - ); + )->modify($this->startTime()); return; } diff --git a/tests/VObject/Component/VCalendarTest.php b/tests/VObject/Component/VCalendarTest.php index bde6a4fa5..847e17d02 100644 --- a/tests/VObject/Component/VCalendarTest.php +++ b/tests/VObject/Component/VCalendarTest.php @@ -756,6 +756,49 @@ public function testCalDAVMETHOD(): void ); } + public function testNodeInValidationErrorHasLineIndexAndLineStringProps(): void + { + $defectiveInput = <<validate(); + $warningMessages = []; + foreach ($result as $error) { + $warningMessages[] = $error['message']; + } + self::assertCount(2, $result, 'We expected exactly 2 validation messages, instead we got '.count($result).' results:'.implode(', ', $warningMessages)); + foreach ($result as $idx => $warning) { + self::assertArrayHasKey('node', $warning); + self::assertInstanceOf(VObject\Property\ICalendar\DateTime::class, $warning['node']); + self::assertObjectHasProperty('lineIndex', $warning['node']); + self::assertObjectHasProperty('lineString', $warning['node']); + switch ($idx) { + case 0: + self::assertEquals('10', $warning['node']->lineIndex); + self::assertEquals('CREATED:', $warning['node']->lineString); + break; + case 1: + self::assertEquals('11', $warning['node']->lineIndex); + self::assertEquals('LAST-MODIFIED:', $warning['node']->lineString); + break; + } + } + } + public function assertValidate($ics, $options, $expectedLevel, ?string $expectedMessage = null): void { $vcal = VObject\Reader::read($ics); diff --git a/tests/VObject/ITip/BrokerProcessReplyTest.php b/tests/VObject/ITip/BrokerProcessReplyTest.php index 064d5cb48..1718f76d2 100644 --- a/tests/VObject/ITip/BrokerProcessReplyTest.php +++ b/tests/VObject/ITip/BrokerProcessReplyTest.php @@ -253,6 +253,73 @@ public function testReplyPartyCrasher(): void $this->process($itip, $old, $expected); } + public function testReplyExistingExceptionRecurrenceIdInUTC(): void + { + $itip = <<process($itip, $old, $expected); + } + public function testReplyNewException(): void { // This is a reply to 1 instance of a recurring event. This should @@ -373,6 +440,126 @@ public function testReplyNewExceptionTz(): void $this->process($itip, $old, $expected); } + public function testReplyNewExceptionRecurrenceIdInDifferentTz(): void + { + // This is a reply to 1 instance of a recurring event. This should + // automatically create an exception. + $itip = <<process($itip, $old, $expected); + } + + public function testReplyNewExceptionRecurrenceIdInUTC(): void + { + // This is a reply to 1 instance of a recurring event. This should + // automatically create an exception. + $itip = <<process($itip, $old, $expected); + } + public function testReplyPartyCrashCreateException(): void { // IN this test there's a recurring event that has an exception. The diff --git a/tests/VObject/Parser/MimeDirTest.php b/tests/VObject/Parser/MimeDirTest.php index 24656f886..db5735b0c 100644 --- a/tests/VObject/Parser/MimeDirTest.php +++ b/tests/VObject/Parser/MimeDirTest.php @@ -101,6 +101,25 @@ public function testDecodeUnsupportedInlineCharset(): void $mimeDir->parse($vcard); } + public function provideEmptyParserInput(): array + { + return [ + [null, 'No input provided to parse'], + ['', 'End of document reached prematurely'], + ]; + } + + /** + * @dataProvider provideEmptyParserInput + */ + public function testParseEmpty($input, $expectedExceptionMessage): void + { + $this->expectException(ParseException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + $mimeDir = new MimeDir(); + $mimeDir->parse($input); + } + public function testDecodeWindows1252(): void { $vcard = <<