Skip to content

Commit

Permalink
Merge pull request #12006 from owncloud/dav-put-storage
Browse files Browse the repository at this point in the history
Work directly on the storage when uploading over webdav
  • Loading branch information
MorrisJobke committed Apr 15, 2015
2 parents 843290a + eeecca0 commit e33e5b4
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 44 deletions.
96 changes: 61 additions & 35 deletions lib/private/connector/sabre/file.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ class File extends Node implements IFile {
*/
public function put($data) {
try {
if ($this->info && $this->fileView->file_exists($this->path) &&
!$this->info->isUpdateable()) {
$exists = $this->fileView->file_exists($this->path);
if ($this->info && $exists && !$this->info->isUpdateable()) {
throw new Forbidden();
}
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("File is not updatable: ".$e->getMessage());
throw new ServiceUnavailable("File is not updatable: " . $e->getMessage());
}

// verify path of the target
Expand All @@ -110,14 +110,31 @@ public function put($data) {
$partFilePath = $this->path;
}

/** @var \OC\Files\Storage\Storage $storage */
list($storage, $internalPartPath) = $this->fileView->resolvePath($partFilePath);
list(, $internalPath) = $this->fileView->resolvePath($this->path);
try {
$putOkay = $this->fileView->file_put_contents($partFilePath, $data);
if ($putOkay === false) {
\OC_Log::write('webdav', '\OC\Files\Filesystem::file_put_contents() failed', \OC_Log::ERROR);
$target = $storage->fopen($internalPartPath, 'wb');
if ($target === false) {
\OC_Log::write('webdav', '\OC\Files\Filesystem::fopen() failed', \OC_Log::ERROR);
$this->fileView->unlink($partFilePath);
// because we have no clue about the cause we can only throw back a 500/Internal Server Error
throw new Exception('Could not write file contents');
}
list($count, ) = \OC_Helper::streamCopy($data, $target);
fclose($target);

// if content length is sent by client:
// double check if the file was fully received
// compare expected and actual size
if (isset($_SERVER['CONTENT_LENGTH']) && $_SERVER['REQUEST_METHOD'] !== 'LOCK') {
$expected = $_SERVER['CONTENT_LENGTH'];
if ($count != $expected) {
$storage->unlink($internalPartPath);
throw new BadRequest('expected filesize ' . $expected . ' got ' . $count);
}
}

} catch (NotPermittedException $e) {
// a more general case - due to whatever reason the content could not be written
throw new Forbidden($e->getMessage());
Expand All @@ -138,56 +155,64 @@ public function put($data) {
// returning 503 will allow retry of the operation at a later point in time
throw new ServiceUnavailable("Encryption not ready: " . $e->getMessage());
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("Failed to write file contents: ".$e->getMessage());
throw new ServiceUnavailable("Failed to write file contents: " . $e->getMessage());
}

try {
// if content length is sent by client:
// double check if the file was fully received
// compare expected and actual size
if (isset($_SERVER['CONTENT_LENGTH']) && $_SERVER['REQUEST_METHOD'] !== 'LOCK') {
$expected = $_SERVER['CONTENT_LENGTH'];
$actual = $this->fileView->filesize($partFilePath);
if ($actual != $expected) {
$this->fileView->unlink($partFilePath);
throw new BadRequest('expected filesize ' . $expected . ' got ' . $actual);
}
}

if ($needsPartFile) {
// rename to correct path
try {
$renameOkay = $this->fileView->rename($partFilePath, $this->path);
$fileExists = $this->fileView->file_exists($this->path);
$renameOkay = $storage->rename($internalPartPath, $internalPath);
$fileExists = $storage->file_exists($internalPath);
if ($renameOkay === false || $fileExists === false) {
\OC_Log::write('webdav', '\OC\Files\Filesystem::rename() failed', \OC_Log::ERROR);
$this->fileView->unlink($partFilePath);
throw new Exception('Could not rename part file to final file');
}
}
catch (LockNotAcquiredException $e) {
} catch (\OCP\Files\LockNotAcquiredException $e) {
// the file is currently being written to by another process
throw new FileLocked($e->getMessage(), $e->getCode(), $e);
}
}

// since we skipped the view we need to scan and emit the hooks ourselves
$storage->getScanner()->scanFile($internalPath);

$view = \OC\Files\Filesystem::getView();
if ($view) {
$hookPath = $view->getRelativePath($this->fileView->getAbsolutePath($this->path));
if (!$exists) {
\OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_post_create, array(
\OC\Files\Filesystem::signal_param_path => $hookPath
));
} else {
\OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_post_update, array(
\OC\Files\Filesystem::signal_param_path => $hookPath
));
}
\OC_Hook::emit(\OC\Files\Filesystem::CLASSNAME, \OC\Files\Filesystem::signal_post_write, array(
\OC\Files\Filesystem::signal_param_path => $hookPath
));
}

// allow sync clients to send the mtime along in a header
$request = \OC::$server->getRequest();
if (isset($request->server['HTTP_X_OC_MTIME'])) {
if($this->fileView->touch($this->path, $request->server['HTTP_X_OC_MTIME'])) {
if ($this->fileView->touch($this->path, $request->server['HTTP_X_OC_MTIME'])) {
header('X-OC-MTime: accepted');
}
}
$this->refreshInfo();
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("Failed to check file size: ".$e->getMessage());
throw new ServiceUnavailable("Failed to check file size: " . $e->getMessage());
}

return '"' . $this->info->getEtag() . '"';
}

/**
* Returns the data
*
* @return string|resource
* @throws Forbidden
* @throws ServiceUnavailable
Expand All @@ -201,12 +226,13 @@ public function get() {
// returning 503 will allow retry of the operation at a later point in time
throw new ServiceUnavailable("Encryption not ready: " . $e->getMessage());
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("Failed to open file: ".$e->getMessage());
throw new ServiceUnavailable("Failed to open file: " . $e->getMessage());
}
}

/**
* Delete the current file
*
* @throws Forbidden
* @throws ServiceUnavailable
*/
Expand All @@ -221,7 +247,7 @@ public function delete() {
throw new Forbidden();
}
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("Failed to unlink: ".$e->getMessage());
throw new ServiceUnavailable("Failed to unlink: " . $e->getMessage());
}
}

Expand All @@ -236,7 +262,7 @@ public function getContentType() {
$mimeType = $this->info->getMimetype();

// PROPFIND needs to return the correct mime type, for consistency with the web UI
if (isset($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PROPFIND' ) {
if (isset($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PROPFIND') {
return $mimeType;
}
return \OC_Helper::getSecureMimeType($mimeType);
Expand Down Expand Up @@ -266,8 +292,7 @@ public function getDirectDownload() {
* @throws NotImplemented
* @throws ServiceUnavailable
*/
private function createFileChunked($data)
{
private function createFileChunked($data) {
list($path, $name) = \Sabre\HTTP\URLUtil::splitPath($this->path);

$info = \OC_FileChunking::decodeName($name);
Expand All @@ -278,7 +303,7 @@ private function createFileChunked($data)
$bytesWritten = $chunk_handler->store($info['index'], $data);

//detect aborted upload
if (isset ($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PUT' ) {
if (isset ($_SERVER['REQUEST_METHOD']) && $_SERVER['REQUEST_METHOD'] === 'PUT') {
if (isset($_SERVER['CONTENT_LENGTH'])) {
$expected = $_SERVER['CONTENT_LENGTH'];
if ($bytesWritten != $expected) {
Expand All @@ -290,7 +315,7 @@ private function createFileChunked($data)
}

if ($chunk_handler->isComplete()) {
list($storage, ) = $this->fileView->resolvePath($path);
list($storage,) = $this->fileView->resolvePath($path);
$needsPartFile = $this->needsPartFile($storage);

try {
Expand Down Expand Up @@ -319,15 +344,15 @@ private function createFileChunked($data)
// allow sync clients to send the mtime along in a header
$request = \OC::$server->getRequest();
if (isset($request->server['HTTP_X_OC_MTIME'])) {
if($this->fileView->touch($targetPath, $request->server['HTTP_X_OC_MTIME'])) {
if ($this->fileView->touch($targetPath, $request->server['HTTP_X_OC_MTIME'])) {
header('X-OC-MTime: accepted');
}
}

$info = $this->fileView->getFileInfo($targetPath);
return $info->getEtag();
} catch (StorageNotAvailableException $e) {
throw new ServiceUnavailable("Failed to put file: ".$e->getMessage());
throw new ServiceUnavailable("Failed to put file: " . $e->getMessage());
}
}

Expand All @@ -338,13 +363,14 @@ private function createFileChunked($data)
* Returns whether a part file is needed for the given storage
* or whether the file can be assembled/uploaded directly on the
* target storage.
*
* @param \OCP\Files\Storage $storage
* @return bool true if the storage needs part file handling
*/
private function needsPartFile($storage) {
// TODO: in the future use ChunkHandler provided by storage
// and/or add method on Storage called "needsPartFile()"
return !$storage->instanceOfStorage('OCA\Files_Sharing\External\Storage') &&
!$storage->instanceOfStorage('OC\Files\Storage\OwnCloud');
!$storage->instanceOfStorage('OC\Files\Storage\OwnCloud');
}
}
34 changes: 25 additions & 9 deletions tests/lib/connector/sabre/file.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,33 @@

class File extends \Test\TestCase {

private function getStream($string) {
$stream = fopen('php://temp', 'r+');
fwrite($stream, $string);
fseek($stream, 0);
return $stream;
}

/**
* @expectedException \Sabre\DAV\Exception
*/
public function testSimplePutFails() {
// setup
$view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath'), array());
$storage = $this->getMock('\OC\Files\Storage\Local', ['fopen'], [['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]);
$view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath', 'resolvePath'), array());
$view->expects($this->any())
->method('file_put_contents')
->method('resolvePath')
->will($this->returnValue(array($storage, '')));
$storage->expects($this->once())
->method('fopen')
->will($this->returnValue(false));

$view->expects($this->any())
->method('getRelativePath')
->will($this->returnValue('/test.txt'));

$info = new \OC\Files\FileInfo('/test.txt', null, null, array(
'permissions'=>\OCP\Constants::PERMISSION_ALL
'permissions' => \OCP\Constants::PERMISSION_ALL
), null);

$file = new \OC\Connector\Sabre\File($view, $info);
Expand All @@ -36,8 +47,9 @@ public function testSimplePutFails() {

public function testPutSingleFileShare() {
// setup
$storage = $this->getMock('\OCP\Files\Storage');
$view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath'), array());
$stream = fopen('php://temp', 'w+');
$storage = $this->getMock('\OC\Files\Storage\Local', ['fopen'], [['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]);
$view = $this->getMock('\OC\Files\View', array('file_put_contents', 'getRelativePath', 'resolvePath'), array());
$view->expects($this->any())
->method('resolvePath')
->with('')
Expand All @@ -49,14 +61,17 @@ public function testPutSingleFileShare() {
->method('file_put_contents')
->with('')
->will($this->returnValue(true));
$storage->expects($this->once())
->method('fopen')
->will($this->returnValue($stream));

$info = new \OC\Files\FileInfo('/foo.txt', null, null, array(
'permissions' => \OCP\Constants::PERMISSION_ALL
), null);

$file = new \OC\Connector\Sabre\File($view, $info);

$this->assertNotEmpty($file->put('test data'));
$this->assertNotEmpty($file->put($this->getStream('test data')));
}

/**
Expand Down Expand Up @@ -91,7 +106,7 @@ public function testSimplePutFailsOnRename() {
$file = new \OC\Connector\Sabre\File($view, $info);

// action
$file->put('test data');
$file->put($this->getStream('test data'));
}

/**
Expand All @@ -114,11 +129,12 @@ public function testSimplePutInvalidChars() {
$file = new \OC\Connector\Sabre\File($view, $info);

// action
$file->put('test data');
$file->put($this->getStream('test data'));
}

/**
* Test setting name with setName() with invalid chars
*
* @expectedException \OC\Connector\Sabre\Exception\InvalidPath
*/
public function testSetNameInvalidChars() {
Expand Down Expand Up @@ -168,7 +184,7 @@ public function testUploadAbort() {
$file = new \OC\Connector\Sabre\File($view, $info);

// action
$file->put('test data');
$file->put($this->getStream('test data'));
}

/**
Expand Down

0 comments on commit e33e5b4

Please sign in to comment.