fix: The WebDAV response element must only contain propstat OR `sta…

…tus` element(s) Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
sabre-io · Sep 18, 2023 · d534cbc · d534cbc
1 parent 85b33f7
commit d534cbc
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 18 deletions.
diff --git a/lib/DAV/Xml/Element/Response.php b/lib/DAV/Xml/Element/Response.php
@@ -40,7 +40,7 @@ class Response implements Element
  *
  * This is currently only used in WebDAV-Sync
  *
- * @var string
+ * @var string|null
  */
  protected $httpStatus;
 
@@ -112,13 +112,22 @@ public function getResponseProperties()
  */
  public function xmlSerialize(Writer $writer)
  {
- if ($status = $this->getHTTPStatus()) {
- $writer->writeElement('{DAV:}status', 'HTTP/1.1 '.$status.' '.\Sabre\HTTP\Response::$statusCodes[$status]);
- }
+ /**
+ * 
+ * Accordingly to the RFC the element looks like:
+ * <!ELEMENT response (href, ((href*, status)|(propstat+)), error?, responsedescription? , location?) >
+ *
+ * So the response
+ * - MUST contain a href and
+ * - EITHER a status and additional href(s)
+ * OR one or more propstat(s)
+ */
  $writer->writeElement('{DAV:}href', $writer->contextUri.\Sabre\HTTP\encodePath($this->getHref()));
 
  $empty = true;
+ $status = $this->getHTTPStatus();
 
+ // Add propstat elements
  foreach ($this->getResponseProperties() as $status => $properties) {
  // Skipping empty lists
  if (!$properties || (!is_int($status) && !ctype_digit($status))) {
@@ -130,19 +139,25 @@ public function xmlSerialize(Writer $writer)
  $writer->writeElement('{DAV:}status', 'HTTP/1.1 '.$status.' '.\Sabre\HTTP\Response::$statusCodes[$status]);
  $writer->endElement(); // {DAV:}propstat
  }
+
+ // The WebDAV spec only allows the status element on responses _without_ a propstat
  if ($empty) {
- /*
- * The WebDAV spec _requires_ at least one DAV:propstat to appear for
- * every DAV:response. In some circumstances however, there are no
- * properties to encode.
- *
- * In those cases we MUST specify at least one DAV:propstat anyway, with
- * no properties.
- */
- $writer->writeElement('{DAV:}propstat', [
- '{DAV:}prop' => [],
- '{DAV:}status' => 'HTTP/1.1 418 '.\Sabre\HTTP\Response::$statusCodes[418],
- ]);
+ if ($status !== null) {
+ $writer->writeElement('{DAV:}status', 'HTTP/1.1 '.$status.' '.\Sabre\HTTP\Response::$statusCodes[$status]);
+ } else {
+ /*
+ * The WebDAV spec _requires_ at least one DAV:propstat to appear for
+ * every DAV:response if there is no status.
+ * In some circumstances however, there are no properties to encode.
+ *
+ * In those cases we MUST specify at least one DAV:propstat anyway, with
+ * no properties.
+ */
+ $writer->writeElement('{DAV:}propstat', [
+ '{DAV:}prop' => [],
+ '{DAV:}status' => 'HTTP/1.1 418 '.\Sabre\HTTP\Response::$statusCodes[418],
+ ]);
+ }
  }
  }
 

diff --git a/tests/Sabre/DAV/Xml/Element/ResponseTest.php b/tests/Sabre/DAV/Xml/Element/ResponseTest.php
@@ -241,8 +241,8 @@ public function testSerializeUrlencoding()
  * @depends testSerialize
  *
  * The WebDAV spec _requires_ at least one DAV:propstat to appear for
- * every DAV:response. In some circumstances however, there are no
- * properties to encode.
+ * every DAV:response if there is no status.
+ * In some circumstances however, there are no properties to encode.
  *
  * In those cases we MUST specify at least one DAV:propstat anyway, with
  * no properties.
@@ -268,6 +268,65 @@ public function testSerializeNoProperties()
 ', $xml);
  }
 
+ /**
+ * @depends testSerialize
+ *
+ * The WebDAV spec _requires_ at least one DAV:propstat _OR_ a status to appear for
+ * every DAV:response.
+ * So if there are no properties but a status, the response should contain that status.
+ */
+ public function testSerializeNoPropertiesButStatus()
+ {
+ $innerProps = [];
+
+ $property = new Response('uri', $innerProps, "200 Ok");
+ $xml = $this->write(['{DAV:}root' => ['{DAV:}response' => $property]]);
+
+ self::assertXmlStringEqualsXmlString(
+'<?xml version="1.0"?>
+<d:root xmlns:d="DAV:">
+ <d:response>
+ <d:href>/uri</d:href>
+ <d:status>200 Ok</d:status>
+ </d:response>
+</d:root>
+', $xml);
+ }
+
+ /**
+ * @depends testSerialize
+ *
+ * The WebDAV standard only allow EITHER propstat(s) OR a status within the response.
+ * Make sure that if there are propstat(s), no status element is added.
+ */
+ public function testSerializePropertiesAndStatus()
+ {
+ $innerProps = [
+ 200 => [
+ '{DAV:}displayname' => 'my file',
+ ],
+ ];
+
+ $property = new Response('uri', $innerProps, "200 Ok");
+
+ $xml = $this->write(['{DAV:}root' => ['{DAV:}response' => $property]]);
+
+ self::assertXmlStringEqualsXmlString(
+'<?xml version="1.0"?>
+<d:root xmlns:d="DAV:">
+ <d:response>
+ <d:href>/uri</d:href>
+ <d:propstat>
+ <d:prop>
+ <d:displayname>my file</d:displayname>
+ </d:prop>
+ <d:status>HTTP/1.1 200 OK</d:status>
+ </d:propstat>
+ </d:response>
+</d:root>
+', $xml);
+ }
+
  /**
  * In the case of {DAV:}prop, a deserializer should never get called, if
  * the property element is empty.