Skip to content

Commit

Permalink
Merge pull request #28186 from owncloud/stabe10-tests-m-time-fix
Browse files Browse the repository at this point in the history 
[stable10] make sure mtime is always an int
  • Loading branch information
Vincent Petry authored Jun 27, 2017
2 parents ba133ee + 332d4ba commit 090f223
Show file tree
Hide file tree
Showing 2 changed files with 165 additions and 8 deletions.
46 changes: 40 additions & 6 deletions apps/dav/lib/Connector/Sabre/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,28 @@
use Sabre\DAV\Exception\ServiceUnavailable;
use Sabre\DAV\IFile;
use Sabre\DAV\Exception\NotFound;
use OC\AppFramework\Http\Request;

class File extends Node implements IFile {

protected $request;

/**
* Sets up the node, expects a full path name
*
* @param \OC\Files\View $view
* @param \OCP\Files\FileInfo $info
* @param \OCP\Share\IManager $shareManager
*/
public function __construct($view, $info, $shareManager = null, Request $request = null) {
if (isset($request)) {
$this->request = $request;
} else {
$this->request = \OC::$server->getRequest();
}
parent::__construct($view, $info, $shareManager);
}

/**
* Updates the data
*
Expand Down Expand Up @@ -210,9 +229,11 @@ public function put($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($this->path, $request->server['HTTP_X_OC_MTIME'])) {
if (isset($this->request->server['HTTP_X_OC_MTIME'])) {
$mtime = $this->sanitizeMtime(
$this->request->server ['HTTP_X_OC_MTIME']
);
if ($this->fileView->touch($this->path, $mtime)) {
header('X-OC-MTime: accepted');
}
}
Expand Down Expand Up @@ -473,9 +494,11 @@ 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 ($targetStorage->touch($targetInternalPath, $request->server['HTTP_X_OC_MTIME'])) {
if (isset($this->request->server['HTTP_X_OC_MTIME'])) {
$mtime = $this->sanitizeMtime(
$this->request->server ['HTTP_X_OC_MTIME']
);
if ($targetStorage->touch($targetInternalPath, $mtime)) {
header('X-OC-MTime: accepted');
}
}
Expand Down Expand Up @@ -604,6 +627,17 @@ private function convertToSabreException(\Exception $e) {
throw new \Sabre\DAV\Exception($e->getMessage(), 0, $e);
}

private function sanitizeMtime ($mtimeFromRequest) {
$mtime = (float) $mtimeFromRequest;
if ($mtime >= PHP_INT_MAX) {
$mtime = PHP_INT_MAX;
} elseif ($mtime <= (PHP_INT_MAX*-1)) {
$mtime = (PHP_INT_MAX*-1);
} else {
$mtime = (int) $mtimeFromRequest;
}
return $mtime;
}

/**
* Set $algo to get a specific checksum, leave null to get all checksums
Expand Down
127 changes: 125 additions & 2 deletions apps/dav/tests/unit/Connector/Sabre/FileTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ class FileTest extends TestCase {
*/
private $user;

/** @var IConfig | \PHPUnit_Framework_MockObject_MockObject */
protected $config;

public function setUp() {
parent::setUp();
unset($_SERVER['HTTP_OC_CHUNKED']);
Expand All @@ -75,6 +78,8 @@ public function setUp() {
$userManager->createUser($this->user, 'pass');

$this->loginAsUser($this->user);

$this->config = $this->getMockBuilder('\OCP\IConfig')->getMock();
}

public function tearDown() {
Expand Down Expand Up @@ -299,7 +304,7 @@ function ($path) use ($storage) {
*
* @return null|string of the PUT operaiton which is usually the etag
*/
private function doPut($path, $viewRoot = null) {
private function doPut($path, $viewRoot = null, \OC\AppFramework\Http\Request $request = null) {
$view = Filesystem::getView();
if (!is_null($viewRoot)) {
$view = new View($viewRoot);
Expand All @@ -315,7 +320,7 @@ private function doPut($path, $viewRoot = null) {
null
);

$file = new File($view, $info);
$file = new File($view, $info, null, $request);

// beforeMethod locks
$view->lockFile($path, ILockingProvider::LOCK_SHARED);
Expand All @@ -335,6 +340,105 @@ public function testPutSingleFile() {
$this->assertNotEmpty($this->doPut('/foo.txt'));
}

/**
* Determine if the underlying storage supports a negative mtime value
*
* @return boolean true if negative mtime is supported
*/
private function supportsNegativeMtime() {
return (getenv("PRIMARY_STORAGE_CONFIG") !== "swift");
}

public function legalMtimeProvider() {
return [
"string" => [
'HTTP_X_OC_MTIME' => "string",
'expected result' => 0
],
"castable string (int)" => [
'HTTP_X_OC_MTIME' => "34",
'expected result' => 34
],
"castable string (float)" => [
'HTTP_X_OC_MTIME' => "34.56",
'expected result' => 34
],
"float" => [
'HTTP_X_OC_MTIME' => 34.56,
'expected result' => 34
],
"zero" => [
'HTTP_X_OC_MTIME' => 0,
'expected result' => 0
],
"zero string" => [
'HTTP_X_OC_MTIME' => "0",
'expected result' => 0
],
"negative zero string" => [
'HTTP_X_OC_MTIME' => "-0",
'expected result' => 0
],
"string starting with number following by char" => [
'HTTP_X_OC_MTIME' => "2345asdf",
'expected result' => 2345
],
"string castable hex int" => [
'HTTP_X_OC_MTIME' => "0x45adf",
'expected result' => 0
],
"string that looks like invalid hex int" => [
'HTTP_X_OC_MTIME' => "0x123g",
'expected result' => 0
],
"negative int" => [
'HTTP_X_OC_MTIME' => -34,
'expected result' => ($this->supportsNegativeMtime() ? -34 : 0)
],
"negative float" => [
'HTTP_X_OC_MTIME' => -34.43,
'expected result' => ($this->supportsNegativeMtime() ? -34 : 0)
],
];
}

/**
* Test putting a file with string Mtime
* @runInSeparateProcess
* @preserveGlobalState disabled
* @dataProvider legalMtimeProvider
*/
public function testPutSingleFileLegalMtime($requestMtime, $resultMtime) {
$request = new \OC\AppFramework\Http\Request([
'server' => [
'HTTP_X_OC_MTIME' => $requestMtime,
]
], null, $this->config, null);
$file = 'foo.txt';
$this->doPut($file, null, $request);
$this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']);
}

/**
* Test putting a file with string Mtime using chunking
* @runInSeparateProcess
* @preserveGlobalState disabled
* @dataProvider legalMtimeProvider
*/
public function testChunkedPutLegalMtime($requestMtime, $resultMtime) {
$request = new \OC\AppFramework\Http\Request([
'server' => [
'HTTP_X_OC_MTIME' => $requestMtime,
]
], null, $this->config, null);

$_SERVER['HTTP_OC_CHUNKED'] = true;
$file = 'foo.txt';
$this->doPut($file.'-chunking-12345-2-0', null, $request);
$this->doPut($file.'-chunking-12345-2-1', null, $request);
$this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']);
}

/**
* Test putting a file using chunking
*/
Expand Down Expand Up @@ -973,6 +1077,25 @@ private function listPartFiles(View $userView = null, $path = '') {
return $files;
}

/**
* returns an array of file information filesize, mtime, filetype, mimetype
*
* @param string $path
* @param View $userView
* @return array
*/
private function getFileInfos($path = '', View $userView = null) {
if ($userView === null) {
$userView = Filesystem::getView();
}
return [
"filesize" => $userView->filesize($path),
"mtime" => $userView->filemtime($path),
"filetype" => $userView->filetype($path),
"mimetype" => $userView->getMimeType($path)
];
}

/**
* @expectedException \Sabre\DAV\Exception\ServiceUnavailable
*/
Expand Down

0 comments on commit 090f223

Please sign in to comment.