From 9e72aca46937c6edd3bad66d79d9d56d7143bb9b 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 02eaf13c7..757bddb66 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -180,23 +180,30 @@ public function createProperty(string $name, $value = null, ?array $parameters = $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 f7cef9ce9e122e7837d15aef0305c371d074881d 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 5a34383f1..c1a3af8f2 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 { @@ -391,4 +392,18 @@ public function testValidateBadEncodingVCard21(): void self::assertEquals('ENCODING=B is not valid for this document type.', $result[0]['message']); self::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 1f22f4a59ef8d79f53788ece6cf1dc59fb43a096 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 757bddb66..29463059c 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -186,8 +186,8 @@ public function createProperty(string $name, $value = null, ?array $parameters = // 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) {