Prevent infinite loop on empty xml elements

sabre-io · Jan 8, 2019 · 77be44c · 77be44c
1 parent fe7c953
commit 77be44c
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 5 deletions.
diff --git a/lib/Deserializer/functions.php b/lib/Deserializer/functions.php
@@ -64,9 +64,20 @@ function keyValue(Reader $reader, string $namespace = null): array
         return [];
     }
 
+    if (!$reader->read()) {
+        $reader->next();
+
+        return [];
+    }
+
+    if (Reader::END_ELEMENT === $reader->nodeType) {
+        $reader->next();
+
+        return [];
+    }
+
     $values = [];
 
-    $reader->read();
     do {
         if (Reader::ELEMENT === $reader->nodeType) {
             if (null !== $namespace && $reader->namespaceURI === $namespace) {
@@ -76,11 +87,15 @@ function keyValue(Reader $reader, string $namespace = null): array
                 $values[$clark] = $reader->parseCurrentElement()['value'];
             }
         } else {
-            $reader->read();
+            if (!$reader->read()) {
+                break;
+            }
         }
     } while (Reader::END_ELEMENT !== $reader->nodeType);
 
-    $reader->read();
+    if (!$reader->read()) {
+        return $values;
+    }
 
     return $values;
 }
@@ -139,7 +154,17 @@ function enum(Reader $reader, string $namespace = null): array
 
         return [];
     }
-    $reader->read();
+    if (!$reader->read()) {
+        $reader->next();
+
+        return [];
+    }
+
+    if (Reader::END_ELEMENT === $reader->nodeType) {
+        $reader->next();
+
+        return [];
+    }
     $currentDepth = $reader->depth;
 
     $values = [];
@@ -193,7 +218,9 @@ function valueObject(Reader $reader, string $className, string $namespace)
                 $reader->next();
             }
         } else {
-            $reader->read();
+            if (!$reader->read()) {
+                break;
+            }
         }
     } while (Reader::END_ELEMENT !== $reader->nodeType);
 

diff --git a/tests/Sabre/Xml/Deserializer/EnumTest.php b/tests/Sabre/Xml/Deserializer/EnumTest.php
@@ -55,4 +55,31 @@ public function testDeserializeDefaultNamespace()
 
         $this->assertEquals($expected, $result);
     }
+
+    public function testEmptyEnum()
+    {
+        $service = new Service();
+        $service->elementMap['{urn:test}enum'] = 'Sabre\Xml\Deserializer\enum';
+
+        $xml = <<<XML
+<?xml version="1.0"?>
+<root xmlns="urn:test">
+<inner>
+  <enum></enum>
+</inner>
+</root>
+XML;
+
+        $result = $service->parse($xml);
+
+        $this->assertEquals([[
+            'name' => '{urn:test}inner',
+            'value' => [[
+                'name' => '{urn:test}enum',
+                'value' => [],
+                'attributes' => [],
+            ]],
+            'attributes' => [],
+        ]], $result);
+    }
 }
diff --git a/tests/Sabre/Xml/Deserializer/KeyValueTest.php b/tests/Sabre/Xml/Deserializer/KeyValueTest.php
@@ -21,6 +21,7 @@ public function testKeyValue()
        <elem4>foo</elem4>
        <elem5>foo &amp; bar</elem5>
     </elem3>
+    <elem6></elem6>
   </struct>
 </root>
 BLA;
@@ -54,6 +55,7 @@ public function testKeyValue()
                                 'attributes' => [],
                             ],
                         ],
+                        'elem6' => null,
                     ],
                     'attributes' => [],
                 ],
@@ -107,4 +109,44 @@ public function testKeyValueLoop()
 
         $reader->parse();
     }
+
+    public function testEmptyKeyValue()
+    {
+        // the nested structure below is necessary to detect if one of the deserialization functions eats to much elements
+        $input = <<<BLA
+<?xml version="1.0"?>
+<root xmlns="http://sabredav.org/ns">
+  <inner>
+    <struct></struct>
+  </inner>
+</root>
+BLA;
+
+        $reader = new Reader();
+        $reader->elementMap = [
+            '{http://sabredav.org/ns}struct' => function (Reader $reader) {
+                return keyValue($reader, 'http://sabredav.org/ns');
+            },
+        ];
+        $reader->xml($input);
+        $output = $reader->parse();
+
+        $this->assertEquals([
+            'name' => '{http://sabredav.org/ns}root',
+            'value' => [
+                [
+                    'name' => '{http://sabredav.org/ns}inner',
+                    'value' => [
+                        [
+                            'name' => '{http://sabredav.org/ns}struct',
+                            'value' => [],
+                            'attributes' => [],
+                        ],
+                    ],
+                    'attributes' => [],
+                ],
+            ],
+            'attributes' => [],
+        ], $output);
+    }
 }
diff --git a/tests/Sabre/Xml/ServiceTest.php b/tests/Sabre/Xml/ServiceTest.php
@@ -140,6 +140,23 @@ public function testInvalidNameSpace()
         $result = $util->expect('{DAV:}propfind', $xml);
     }
 
+    /**
+     * @dataProvider providesEmptyPropfinds
+     */
+    public function testEmptyPropfind($xml)
+    {
+        $util = new Service();
+        $util->elementMap = [
+            '{DAV:}propfind' => PropFindTestAsset::class,
+        ];
+        $util->namespaceMap = [
+            'http://sabre.io/ns' => 's',
+        ];
+        $result = $util->expect('{DAV:}propfind', $xml);
+        $this->assertEquals(false, $result->allProp);
+        $this->assertEquals([], $result->properties);
+    }
+
     /**
      * @depends testGetReader
      */
@@ -314,6 +331,17 @@ public function testParseClarkNotationFail()
     {
         Service::parseClarkNotation('http://sabredav.org/ns}elem');
     }
+
+    public function providesEmptyPropfinds()
+    {
+        return [
+            ['<D:propfind xmlns:D="DAV:"><D:prop></D:prop></D:propfind>'],
+            ['<D:propfind xmlns:D="DAV:"><D:prop xmlns:s="http://sabredav.org/ns"></D:prop></D:propfind>'],
+            ['<D:propfind xmlns:D="DAV:"><D:prop/></D:propfind>'],
+            ['<D:propfind xmlns:D="DAV:"><D:prop xmlns:s="http://sabredav.org/ns"/></D:propfind>'],
+            ['<D:propfind xmlns:D="DAV:"><D:prop>     </D:prop></D:propfind>'],
+        ];
+    }
 }
 
 /**