From 7113bfb5ba1da676e5699fda72df0478c7bab3fa Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Tue, 10 Sep 2024 16:24:31 +0200 Subject: [PATCH 1/3] Allow unknown value data types for VALUE Currently value-types that are not known will cause the parser to throw an InvalidDataException. In RFC5545, Section 3.2.20 is described though that the allowed values for the "VALUE" data-type can include x-names and iana-tokens. These can be any string that might - or might not - be understood by the parser. The description clearly states that "Applications MUST preserve the value data for x-name and iana-token value that they don't recognize without attempting to interpret or parse the value data" This means that the content of the "VALUE" part should not be interpreted at all the moment the parser doesn't find a match in the corresponding mapping table. But it should also not hard-fail when someone sets a VALUE that might not be understood. In such a case the default should be used instead. --- lib/Document.php | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/Document.php b/lib/Document.php index d2131f47..748105ce 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -200,23 +200,30 @@ public function createProperty($name, $value = null, ?array $parameters = null, $class = null; + // If a VALUE parameter is supplied, we have to use that + // According to https://datatracker.ietf.org/doc/html/rfc5545#section-3.2.20 + // If the property's value is the default value type, then this + // parameter need not be specified. However, if the property's + // default value type is overridden by some other allowable value + // type, then this parameter MUST be specified. + if (isset($parameters['VALUE'])) { + $class = $this->getClassNameForPropertyValue($parameters['VALUE']); + } + if ($valueType) { // The valueType argument comes first to figure out the correct // class. $class = $this->getClassNameForPropertyValue($valueType); } + // If the value parameter is not set or set to something we do not recognize + // we do not attempt to interpret or parse the datass value as specified in + // https://datatracker.ietf.org/doc/html/rfc5545#section-3.2.20 + // So when we so far did not get a class-name, we use the default for the property if (is_null($class)) { - // If a VALUE parameter is supplied, we should use that. - if (isset($parameters['VALUE'])) { - $class = $this->getClassNameForPropertyValue($parameters['VALUE']); - if (is_null($class)) { - throw new InvalidDataException('Unsupported VALUE parameter for '.$name.' property. You supplied "'.$parameters['VALUE'].'"'); - } - } else { - $class = $this->getClassNameForPropertyName($name); - } + $class = $this->getClassNameForPropertyName($name); } + if (is_null($parameters)) { $parameters = []; } From 2f24ce1ca8ad0d8e49a8621b8b3be7bc114ef863 Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Mon, 14 Oct 2024 21:22:50 +0200 Subject: [PATCH 2/3] Add tests for skipping unknown parameters THis adds a test to assure that an unknown value is skipped. The value `DATETIME` is not known. It should be `DATE-TIME`. Adding the value of DATETIME previopusly resulted in an Error. This now tests that it actually works and that the unknown parameter is actually also kept as specified by the RFC --- tests/VObject/PropertyTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/VObject/PropertyTest.php b/tests/VObject/PropertyTest.php index 1f6e0702..08238379 100644 --- a/tests/VObject/PropertyTest.php +++ b/tests/VObject/PropertyTest.php @@ -5,6 +5,7 @@ use PHPUnit\Framework\TestCase; use Sabre\VObject\Component\VCalendar; use Sabre\VObject\Component\VCard; +use Sabre\VObject\Property\ICalendar\DateTime; class PropertyTest extends TestCase { @@ -385,4 +386,18 @@ public function testValidateBadEncodingVCard21() $this->assertEquals('ENCODING=B is not valid for this document type.', $result[0]['message']); $this->assertEquals(3, $result[0]['level']); } + + public function testUnknownValuesWillBeIgnored(): void + { + $cal = new VCalendar(); + $property = $cal->createProperty('DTSTAMP', '20240101T000000Z', ['VALUE' => 'DATETIME']); + + self::assertEquals("DTSTAMP;VALUE=DATETIME:20240101T000000Z\r\n", $property->serialize()); + + self::assertInstanceOf(DateTime::class, $property); + self::assertCount(1, $property->parameters()); + self::assertInstanceOf(Parameter::class, $property->parameters['VALUE']); + self::assertEquals('VALUE', $property->parameters['VALUE']->name); + self::assertEquals('DATETIME', $property->parameters['VALUE']->getValue()); + } } From 4761ca2b532368676f2c3e08dcf042015605e169 Mon Sep 17 00:00:00 2001 From: Andreas Heigl Date: Mon, 14 Oct 2024 21:24:28 +0200 Subject: [PATCH 3/3] Rework handling of values that are not recognized This will remove the necesity to calculate the class name twice in immediate succession should the declared valuetype not be set. --- lib/Document.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Document.php b/lib/Document.php index 748105ce..36f20ddd 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -206,8 +206,8 @@ public function createProperty($name, $value = null, ?array $parameters = null, // parameter need not be specified. However, if the property's // default value type is overridden by some other allowable value // type, then this parameter MUST be specified. - if (isset($parameters['VALUE'])) { - $class = $this->getClassNameForPropertyValue($parameters['VALUE']); + if (!$valueType) { + $valueType = $parameters['VALUE'] ?? null; } if ($valueType) {