[Pkg-owncloud-commits] [php-sabredav] 16/24: Fixed the implementation of PATCH so it makes sense again. #439.
David Prévot
taffit at moszumanska.debian.org
Tue May 20 17:19:43 UTC 2014
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch 1.7
in repository php-sabredav.
commit 2c07d4d1de419db3db601f010454d4498ed450f4
Author: Evert Pot <evert at rooftopsolutions.nl>
Date: Sun Apr 27 22:30:16 2014 -0400
Fixed the implementation of PATCH so it makes sense again. #439.
---
lib/Sabre/DAV/Exception/LengthRequired.php | 28 ++++++
lib/Sabre/DAV/FSExt/File.php | 50 +++++++---
lib/Sabre/DAV/PartialUpdate/IFile.php | 5 +-
.../PartialUpdate/{IFile.php => IPatchSupport.php} | 24 +++--
lib/Sabre/DAV/PartialUpdate/Plugin.php | 104 ++++++++++++++-------
tests/Sabre/DAV/PartialUpdate/FullStackTest.php | 49 ----------
.../Sabre/DAV/PartialUpdate/SpecificationTest.php | 83 ++++++++++++++++
7 files changed, 237 insertions(+), 106 deletions(-)
diff --git a/lib/Sabre/DAV/Exception/LengthRequired.php b/lib/Sabre/DAV/Exception/LengthRequired.php
new file mode 100644
index 0000000..6d45e6c
--- /dev/null
+++ b/lib/Sabre/DAV/Exception/LengthRequired.php
@@ -0,0 +1,28 @@
+<?php
+
+/**
+ * LengthRequired
+ *
+ * This exception is thrown when a request was made that required a
+ * Content-Length header, but did not contain one.
+ *
+ * @package Sabre
+ * @subpackage DAV
+ * @copyright Copyright (C) 2007-2014 fruux GmbH (https://fruux.com/).
+ * @author Evert Pot (http://evertpot.com/)
+ * @license http://sabre.io/license/ Modified BSD License
+ */
+class Sabre_DAV_Exception_LengthRequired extends Sabre_DAV_Exception {
+
+ /**
+ * Returns the HTTP statuscode for this exception
+ *
+ * @return int
+ */
+ public function getHTTPCode() {
+
+ return 411;
+
+ }
+
+}
diff --git a/lib/Sabre/DAV/FSExt/File.php b/lib/Sabre/DAV/FSExt/File.php
index 8de07c5..329afcd 100644
--- a/lib/Sabre/DAV/FSExt/File.php
+++ b/lib/Sabre/DAV/FSExt/File.php
@@ -9,7 +9,7 @@
* @author Evert Pot (http://evertpot.com/)
* @license http://sabre.io/license/ Modified BSD License
*/
-class Sabre_DAV_FSExt_File extends Sabre_DAV_FSExt_Node implements Sabre_DAV_PartialUpdate_IFile {
+class Sabre_DAV_FSExt_File extends Sabre_DAV_FSExt_Node implements Sabre_DAV_PartialUpdate_IPatchSupport {
/**
* Updates the data
@@ -27,19 +27,47 @@ class Sabre_DAV_FSExt_File extends Sabre_DAV_FSExt_Node implements Sabre_DAV_Par
}
/**
- * Updates the data at a given offset
+ * Updates the file based on a range specification.
*
- * The data argument is a readable stream resource.
- * The offset argument is a 0-based offset where the data should be
- * written.
+ * The first argument is the data, which is either a readable stream
+ * resource or a string.
*
- * param resource|string $data
- * @return void
+ * The second argument is the type of update we're doing.
+ * This is either:
+ * * 1. append
+ * * 2. update based on a start byte
+ * * 3. update based on an end byte
+ *;
+ * The third argument is the start or end byte.
+ *
+ * After a successful put operation, you may choose to return an ETag. The
+ * etag must always be surrounded by double-quotes. These quotes must
+ * appear in the actual string you're returning.
+ *
+ * Clients may use the ETag from a PUT request to later on make sure that
+ * when they update the file, the contents haven't changed in the mean
+ * time.
+ *
+ * @param resource|string $data
+ * @param int $rangeType
+ * @param int $offset
+ * @return string|null
*/
- public function putRange($data, $offset) {
-
- $f = fopen($this->path, 'c');
- fseek($f,$offset-1);
+ public function patch($data, $rangeType, $offset = null) {
+
+ switch($rangeType) {
+ case 1 :
+ $f = fopen($this->path, 'a');
+ break;
+ case 2 :
+ $f = fopen($this->path, 'c');
+ fseek($f,$offset);
+ break;
+ case 3 :
+ $f = fopen($this->path, 'c');
+ fseek($f, $offset, SEEK_END);
+ break;
+ }
if (is_string($data)) {
fwrite($f, $data);
} else {
diff --git a/lib/Sabre/DAV/PartialUpdate/IFile.php b/lib/Sabre/DAV/PartialUpdate/IFile.php
index 8f38e1b..389a885 100644
--- a/lib/Sabre/DAV/PartialUpdate/IFile.php
+++ b/lib/Sabre/DAV/PartialUpdate/IFile.php
@@ -1,15 +1,14 @@
<?php
/**
- * This interface provides a way to modify only part of a target resource
- * It may be used to update a file chunk, upload big a file into smaller
- * chunks or resume an upload
+ * This interface is deprecated. Use IPatchSupport instead.
*
* @package Sabre
* @subpackage DAV
* @copyright Copyright (C) 2007-2014 fruux GmbH (https://fruux.com/).
* @author Jean-Tiare LE BIGOT (http://www.jtlebi.fr/)
* @license http://sabre.io/license/ Modified BSD License
+ * @deprecated
*/
interface Sabre_DAV_PartialUpdate_IFile extends Sabre_DAV_IFile {
diff --git a/lib/Sabre/DAV/PartialUpdate/IFile.php b/lib/Sabre/DAV/PartialUpdate/IPatchSupport.php
similarity index 57%
copy from lib/Sabre/DAV/PartialUpdate/IFile.php
copy to lib/Sabre/DAV/PartialUpdate/IPatchSupport.php
index 8f38e1b..2dee02f 100644
--- a/lib/Sabre/DAV/PartialUpdate/IFile.php
+++ b/lib/Sabre/DAV/PartialUpdate/IPatchSupport.php
@@ -11,14 +11,21 @@
* @author Jean-Tiare LE BIGOT (http://www.jtlebi.fr/)
* @license http://sabre.io/license/ Modified BSD License
*/
-interface Sabre_DAV_PartialUpdate_IFile extends Sabre_DAV_IFile {
+interface Sabre_DAV_PartialUpdate_IPatchSupport extends Sabre_DAV_IFile {
/**
- * Updates the data at a given offset
+ * Updates the file based on a range specification.
*
- * The data argument is a readable stream resource.
- * The offset argument is an integer describing the offset. Contrary to
- * what's sent in the request, the offset here is a 0-based index.
+ * The first argument is the data, which is either a readable stream
+ * resource or a string.
+ *
+ * The second argument is the type of update we're doing.
+ * This is either:
+ * * 1. append
+ * * 2. update based on a start byte
+ * * 3. update based on an end byte
+ *;
+ * The third argument is the start or end byte.
*
* After a successful put operation, you may choose to return an ETag. The
* etag must always be surrounded by double-quotes. These quotes must
@@ -28,11 +35,12 @@ interface Sabre_DAV_PartialUpdate_IFile extends Sabre_DAV_IFile {
* when they update the file, the contents haven't changed in the mean
* time.
*
- * @param resource $data
- * @param integer $offset
+ * @param resource|string $data
+ * @param int $rangeType
+ * @param int $offset
* @return string|null
*/
- function putRange($data, $offset);
+ function patch($data, $rangeType, $offset = null);
}
diff --git a/lib/Sabre/DAV/PartialUpdate/Plugin.php b/lib/Sabre/DAV/PartialUpdate/Plugin.php
index 3509e5c..416fdd8 100644
--- a/lib/Sabre/DAV/PartialUpdate/Plugin.php
+++ b/lib/Sabre/DAV/PartialUpdate/Plugin.php
@@ -17,6 +17,10 @@
*/
class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
+ const RANGE_APPEND = 1;
+ const RANGE_START = 2;
+ const RANGE_END = 3;
+
/**
* Reference to server
*
@@ -66,7 +70,7 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
public function unknownMethod($method, $uri) {
switch($method) {
-
+
case 'PATCH':
return $this->httpPatch($uri);
@@ -80,7 +84,7 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
*
* This method is passed a uri. It should only return HTTP methods that are
* available for the specified uri.
- *
+ *
* We claim to support PATCH method (partial update) if and only if
* - the node exist
* - the node implements our partial update interface
@@ -89,14 +93,14 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
* @return array
*/
public function getHTTPMethods($uri) {
-
+
$tree = $this->server->tree;
-
- if ($tree->nodeExists($uri) &&
+
+ if ($tree->nodeExists($uri) &&
$tree->getNodeForPath($uri) instanceof Sabre_DAV_PartialUpdate_IFile) {
return array('PATCH');
}
-
+
return array();
}
@@ -115,7 +119,7 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
/**
* Patch an uri
*
- * The WebDAV patch request can be used to modify only a part of an
+ * The WebDAV patch request can be used to modify only a part of an
* existing resource. If the resource does not exist yet and the first
* offset is not 0, the request fails
*
@@ -126,7 +130,7 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
// Get the node. Will throw a 404 if not found
$node = $this->server->tree->getNodeForPath($uri);
- if (!($node instanceof Sabre_DAV_PartialUpdate_IFile)) {
+ if (!$node instanceof Sabre_DAV_PartialUpdate_IFile && !$node instanceof Sabre_DAV_PartialUpdate_IPatchSupport) {
throw new Sabre_DAV_Exception_MethodNotAllowed('The target resource does not support the PATCH method.');
}
@@ -135,27 +139,33 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
if (!$range) {
throw new Sabre_DAV_Exception_BadRequest('No valid "X-Update-Range" found in the headers');
}
-
+
$contentType = strtolower(
$this->server->httpRequest->getHeader('Content-Type')
);
-
+
if ($contentType != 'application/x-sabredav-partialupdate') {
throw new Sabre_DAV_Exception_UnsupportedMediaType('Unknown Content-Type header "' . $contentType . '"');
}
$len = $this->server->httpRequest->getHeader('Content-Length');
-
- // Load the begin and end data
- $start = ($range[0])?$range[0]:0;
- $end = ($range[1])?$range[1]:$start+$len-1;
-
- // Check consistency
- if($end < $start)
- throw new Sabre_DAV_Exception_RequestedRangeNotSatisfiable('The end offset (' . $range[1] . ') is lower than the start offset (' . $range[0] . ')');
- if($end - $start + 1 != $len)
- throw new Sabre_DAV_Exception_RequestedRangeNotSatisfiable('Actual data length (' . $len . ') is not consistent with begin (' . $range[0] . ') and end (' . $range[1] . ') offsets');
-
+ if (!$len) throw new Sabre_DAV_Exception_LengthRequired('A Content-Length header is required');
+
+ switch($range[0]) {
+ case self::RANGE_START :
+ // Calculate the end-range if it doesn't exist.
+ if (!$range[2]) {
+ $range[2] = $range[1] + $len - 1;
+ } else {
+ if ($range[2] < $range[1]) {
+ throw new Sabre_DAV_Exception_RequestedRangeNotSatisfiable('The end offset (' . $range[2] . ') is lower than the start offset (' . $range[1] . ')');
+ }
+ if($range[2] - $range[1] + 1 != $len) {
+ throw new Sabre_DAV_Exception_RequestedRangeNotSatisfiable('Actual data length (' . $len . ') is not consistent with begin (' . $range[1] . ') and end (' . $range[2] . ') offsets');
+ }
+ }
+ break;
+ }
// Checking If-None-Match and related headers.
if (!$this->server->checkPreconditions()) return;
@@ -163,7 +173,23 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
return;
$body = $this->server->httpRequest->getBody();
- $etag = $node->putRange($body, $start-1);
+
+
+ if ($node instanceof Sabre_DAV_PartialUpdate_IPatchSupport) {
+ $etag = $node->patch($body, $range[0], isset($range[1])?$range[1]:null);
+ } else {
+ // The old interface
+ switch($range[0]) {
+ case self::RANGE_APPEND :
+ throw new Sabre_DAV_Exception_NotImplemented('This node does not support the append syntax. Please upgrade it to IPatchSupport');
+ case self::RANGE_START :
+ $etag = $node->putRange($body, $range[1]);
+ break;
+ case self::RANGE_END :
+ throw new Sabre_DAV_Exception_NotImplemented('This node does not support the end-range syntax. Please upgrade it to IPatchSupport');
+ break;
+ }
+ }
$this->server->broadcastEvent('afterWriteContent',array($uri, $node));
@@ -174,18 +200,23 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
return false;
}
-
+
/**
* Returns the HTTP custom range update header
*
* This method returns null if there is no well-formed HTTP range request
- * header or array($start, $end).
+ * header. It returns array(1) if it was an append request, array(2,
+ * $start, $end) if it's a start and end range, lastly it's array(3,
+ * $endoffset) if the offset was negative, and should be calculated from
+ * the end of the file.
*
- * The first number is the offset of the first byte in the range.
- * The second number is the offset of the last byte in the range.
+ * Examples:
*
- * If the second offset is null, it should be treated as the offset of the last byte of the entity
- * If the first offset is null, the second offset should be used to retrieve the last x bytes of the entity
+ * null - invalid
+ * array(1) - append
+ * array(2,10,15) - update bytes 10, 11, 12, 13, 14, 15
+ * array(2,10,null) - update bytes 10 until the end of the patch body
+ * array(3,-5) - update from 5 bytes from the end of the file.
*
* @return array|null
*/
@@ -196,14 +227,17 @@ class Sabre_DAV_PartialUpdate_Plugin extends Sabre_DAV_ServerPlugin {
// Matching "Range: bytes=1234-5678: both numbers are optional
- if (!preg_match('/^bytes=([0-9]*)-([0-9]*)$/i',$range,$matches)) return null;
-
- if ($matches[1]==='' && $matches[2]==='') return null;
+ if (!preg_match('/^(append)|(?:bytes=([0-9]+)-([0-9]*))|(?:bytes=(-[0-9]+))$/i',$range,$matches)) return null;
- return array(
- $matches[1]!==''?$matches[1]:null,
- $matches[2]!==''?$matches[2]:null,
- );
+ if ($matches[1]==='append') {
+ return array(self::RANGE_APPEND);
+ } elseif (strlen($matches[2])>0) {
+ return array(self::RANGE_START, $matches[2], $matches[3]?:null);
+ } elseif ($matches[4]) {
+ return array(self::RANGE_END, $matches[4]);
+ } else {
+ return null;
+ }
}
}
diff --git a/tests/Sabre/DAV/PartialUpdate/FullStackTest.php b/tests/Sabre/DAV/PartialUpdate/FullStackTest.php
deleted file mode 100644
index 318de9f..0000000
--- a/tests/Sabre/DAV/PartialUpdate/FullStackTest.php
+++ /dev/null
@@ -1,49 +0,0 @@
-<?php
-
-/**
- * This test sets up the entire stack and tests every bit.
- */
-class Sabre_DAV_PartialUpdate_FullStackTest extends PHPUnit_Framework_TestCase {
-
- protected $server;
-
- public function setUp() {
-
- $tree = array(
- new Sabre_DAV_FSExt_File(SABRE_TEMPDIR . '/foobar.txt')
- );
- $server = new Sabre_DAV_Server($tree);
- $server->addPlugin(new Sabre_DAV_PartialUpdate_Plugin());
-
- $tree[0]->put('1234567890');
-
- $this->server = $server;
-
- }
-
- public function tearDown() {
-
- Sabre_TestUtil::clearTempDir();
-
- }
-
- public function testUpdateRange() {
-
- $request = new Sabre_HTTP_Request(array(
- 'REQUEST_METHOD' => 'PATCH',
- 'HTTP_CONTENT_TYPE' => 'application/x-sabredav-partialupdate',
- 'HTTP_X_UPDATE_RANGE' => 'bytes=3-4',
- 'REQUEST_URI' => '/foobar.txt',
- 'HTTP_CONTENT_LENGTH' => '2',
- ));
- $request->setBody('--');
- $this->server->httpRequest = $request;
- $this->server->httpResponse = new Sabre_HTTP_ResponseMock();
- $this->server->exec();
-
- $this->assertEquals('HTTP/1.1 204 No Content', $this->server->httpResponse->status, 'Incorrect http status received: ' . $this->server->httpResponse->body);
- $this->assertEquals('12--4567890', file_get_contents(SABRE_TEMPDIR . '/foobar.txt'));
-
- }
-
-}
diff --git a/tests/Sabre/DAV/PartialUpdate/SpecificationTest.php b/tests/Sabre/DAV/PartialUpdate/SpecificationTest.php
new file mode 100644
index 0000000..b90d1ed
--- /dev/null
+++ b/tests/Sabre/DAV/PartialUpdate/SpecificationTest.php
@@ -0,0 +1,83 @@
+<?php
+
+/**
+ * This test is an end-to-end sabredav test that goes through all
+ * the cases in the specification.
+ *
+ * See: http://sabre.io/dav/http-patch/
+ */
+class Sabre_DAV_PartialUpdate_SpecificationTest extends PHPUnit_Framework_TestCase {
+
+ protected $server;
+
+ public function setUp() {
+
+ $tree = array(
+ new Sabre_DAV_FSExt_File(SABRE_TEMPDIR . '/foobar.txt')
+ );
+ $server = new Sabre_DAV_Server($tree);
+ $server->debugExceptions = true;
+ $server->addPlugin(new Sabre_DAV_PartialUpdate_Plugin());
+
+ $tree[0]->put('1234567890');
+
+ $this->server = $server;
+
+ }
+
+ public function tearDown() {
+
+ Sabre_TestUtil::clearTempDir();
+
+ }
+
+ /**
+ * @dataProvider data
+ */
+ public function testUpdateRange($headerValue, $httpStatus, $endResult, $contentLength = 4) {
+
+ $vars = array(
+ 'REQUEST_METHOD' => 'PATCH',
+ 'HTTP_CONTENT_TYPE' => 'application/x-sabredav-partialupdate',
+ 'HTTP_X_UPDATE_RANGE' => $headerValue,
+ 'REQUEST_URI' => '/foobar.txt',
+ );
+ if ($contentLength) {
+ $vars['HTTP_CONTENT_LENGTH'] = (string)$contentLength;
+ }
+
+ $request = new Sabre_HTTP_Request($vars);
+
+ $request->setBody('----');
+ $this->server->httpRequest = $request;
+ $this->server->httpResponse = new Sabre_HTTP_ResponseMock();
+ $this->server->exec();
+
+ $this->assertEquals($httpStatus, $this->server->httpResponse->status, 'Incorrect http status received: ' . $this->server->httpResponse->body);
+ if (!is_null($endResult)) {
+ $this->assertEquals($endResult, file_get_contents(SABRE_TEMPDIR . '/foobar.txt'));
+ }
+
+ }
+
+ public function data() {
+
+ return array(
+ // Problems
+ array('foo', 'HTTP/1.1 400 Bad request', null),
+ array('bytes=0-3', 'HTTP/1.1 411 Length Required', null, 0),
+ array('bytes=4-1', 'HTTP/1.1 416 Requested Range Not Satisfiable', null),
+
+ array('bytes=0-3', 'HTTP/1.1 204 No Content', '----567890'),
+ array('bytes=1-4', 'HTTP/1.1 204 No Content', '1----67890'),
+ array('bytes=0-', 'HTTP/1.1 204 No Content', '----567890'),
+ array('bytes=-4', 'HTTP/1.1 204 No Content', '123456----'),
+ array('bytes=-2', 'HTTP/1.1 204 No Content', '12345678----'),
+ array('bytes=2-', 'HTTP/1.1 204 No Content', '12----7890'),
+ array('append', 'HTTP/1.1 204 No Content', '1234567890----'),
+
+ );
+
+ }
+
+}
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/php-sabredav.git
More information about the Pkg-owncloud-commits
mailing list