From b3a7706cbb95b611a7a22c41869dd397d4dda9ec Mon Sep 17 00:00:00 2001 From: Olivier Dolbeau Date: Fri, 2 Oct 2026 19:02:25 +0200 Subject: [PATCH 1/2] fix: do not crash on a repeated VALUE parameter Some producers repeat the VALUE parameter, such as ez-vcard with PRODID;VALUE=text;VALUE=TEXT. The parser then passes an array to createProperty(), and getClassNameForPropertyValue() throws a TypeError. Use the first value to pick the property class. Fixes #635 Co-Authored-By: Claude Opus 5.5 --- lib/Document.php | 9 ++++++++- tests/VObject/DocumentTest.php | 10 ++++++++++ tests/VObject/ReaderTest.php | 13 +++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/Document.php b/lib/Document.php index 8319b49be..c620413c9 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -188,7 +188,14 @@ public function createProperty(string $name, $value = null, ?array $parameters = $valueType = $parameters['VALUE'] ?? null; } - if ($valueType) { + // The VALUE parameter must not occur more than once, but some producers + // repeat it (e.g. VALUE=text;VALUE=TEXT). The parser then passes all its + // values as an array: use the first one. + if (\is_array($valueType)) { + $valueType = reset($valueType); + } + + if (\is_string($valueType) && '' !== $valueType) { // The valueType argument comes first to figure out the correct // class. $class = $this->getClassNameForPropertyValue($valueType); diff --git a/tests/VObject/DocumentTest.php b/tests/VObject/DocumentTest.php index d03a88836..462c47c6b 100644 --- a/tests/VObject/DocumentTest.php +++ b/tests/VObject/DocumentTest.php @@ -52,6 +52,16 @@ public function testCreate(): void self::assertInstanceOf(Property\Text::class, $prop); } + public function testCreatePropertyWithRepeatedValueParameter(): void + { + $vcard = new Component\VCard([], false); + + $prop = $vcard->createProperty('PRODID', 'foo', ['VALUE' => ['text', 'TEXT']]); + + self::assertInstanceOf(Property\Text::class, $prop); + self::assertEquals('foo', $prop->getValue()); + } + public function testGetClassNameForPropertyValue(): void { $vcal = new Component\VCalendar([], false); diff --git a/tests/VObject/ReaderTest.php b/tests/VObject/ReaderTest.php index 7b6c953fd..0b3e59b3e 100644 --- a/tests/VObject/ReaderTest.php +++ b/tests/VObject/ReaderTest.php @@ -230,6 +230,19 @@ public function testReadPropertyNoName(): void self::assertEquals('PRODIGY', $result->parameters['TYPE']); } + public function testReadPropertyWithRepeatedValueParameter(): void + { + // ez-vcard writes the VALUE parameter twice. + $data = "BEGIN:VCARD\r\nVERSION:4.0\r\nPRODID;VALUE=text;VALUE=TEXT:ez-vcard 0.12.1\r\nEND:VCARD"; + $result = Reader::read($data); + + $result = $result->PRODID; + + self::assertInstanceOf(Property::class, $result); + self::assertEquals('ez-vcard 0.12.1', $result->getValue()); + self::assertEquals(['text', 'TEXT'], $result['VALUE']->getParts()); + } + public function testReadPropertyParameterExtraColon(): void { $data = "BEGIN:VCALENDAR\r\nPROPNAME;PARAMNAME=paramvalue:propValue:anotherrandomstring\r\nEND:VCALENDAR"; From ad49ff7ecfc257a195259507c3ebc130a5aeb129 Mon Sep 17 00:00:00 2001 From: Olivier Dolbeau Date: Sat, 3 Oct 2026 20:02:20 +0200 Subject: [PATCH 2/2] fix: also cover a VALUE parameter listing several types ez-vcard also writes several value types in a single VALUE parameter (URL;VALUE=uri,text:...). The parser hands createProperty() the same array as for a repeated VALUE, so the same fix applies: test it. Co-Authored-By: Claude Opus 5.5 --- lib/Document.php | 7 ++++--- tests/VObject/ReaderTest.php | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/Document.php b/lib/Document.php index c620413c9..af47d2611 100644 --- a/lib/Document.php +++ b/lib/Document.php @@ -188,9 +188,10 @@ public function createProperty(string $name, $value = null, ?array $parameters = $valueType = $parameters['VALUE'] ?? null; } - // The VALUE parameter must not occur more than once, but some producers - // repeat it (e.g. VALUE=text;VALUE=TEXT). The parser then passes all its - // values as an array: use the first one. + // The VALUE parameter must have a single value, but some producers repeat + // it (e.g. VALUE=text;VALUE=TEXT) or list several types in it (e.g. + // VALUE=uri,text). The parser then passes all its values as an array: use + // the first one. if (\is_array($valueType)) { $valueType = reset($valueType); } diff --git a/tests/VObject/ReaderTest.php b/tests/VObject/ReaderTest.php index 0b3e59b3e..82fd9b733 100644 --- a/tests/VObject/ReaderTest.php +++ b/tests/VObject/ReaderTest.php @@ -243,6 +243,20 @@ public function testReadPropertyWithRepeatedValueParameter(): void self::assertEquals(['text', 'TEXT'], $result['VALUE']->getParts()); } + public function testReadPropertyWithListedValueParameter(): void + { + // ez-vcard lists several value types in a single VALUE parameter. + $data = "BEGIN:VCARD\r\nVERSION:3.0\r\nURL;VALUE=uri,text:https://jane.example\r\nEND:VCARD"; + $result = Reader::read($data); + + $result = $result->URL; + + self::assertInstanceOf(Property::class, $result); + self::assertEquals('https://jane.example', $result->getValue()); + self::assertEquals(['uri', 'text'], $result['VALUE']->getParts()); + self::assertEquals("URL;VALUE=uri,text:https://jane.example\r\n", $result->serialize()); + } + public function testReadPropertyParameterExtraColon(): void { $data = "BEGIN:VCALENDAR\r\nPROPNAME;PARAMNAME=paramvalue:propValue:anotherrandomstring\r\nEND:VCALENDAR";