From 388bbc78a508375269382c34fef51253a2698070 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 19 Jan 2017 16:34:48 +0100 Subject: [PATCH 1/4] .ocTransferIDXXXXX.part makes filename too long #25425 --- apps/dav/lib/Connector/Sabre/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 6c928c4d1b7a..f0d599e75fe4 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -111,7 +111,7 @@ public function put($data) { if ($needsPartFile) { // mark file as partial while uploading (ignored by the scanner) - $partFilePath = $this->getPartFileBasePath($this->path) . '.ocTransferId' . rand() . '.part'; + $partFilePath = sha1($this->getPartFileBasePath($this->path)) . '.part'; } else { // upload file directly as the final path $partFilePath = $this->path; From 371d48e044fb8dda0b1da90330b8392fd0783ae2 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 19 Jan 2017 17:55:14 +0100 Subject: [PATCH 2/4] .ocTransferIDXXXXX.part makes filename too long: include transferId #25425 --- apps/dav/lib/Connector/Sabre/File.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index f0d599e75fe4..e4d3dc0cea82 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -111,7 +111,7 @@ public function put($data) { if ($needsPartFile) { // mark file as partial while uploading (ignored by the scanner) - $partFilePath = sha1($this->getPartFileBasePath($this->path)) . '.part'; + $partFilePath = sha1($this->getPartFileBasePath($this->path)) . '.ocTransferId' . rand() . '.part'; } else { // upload file directly as the final path $partFilePath = $this->path; From c6b54c8f5db72f39a7d0a254faf45179a53e1d41 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Mon, 23 Jan 2017 16:57:13 +0100 Subject: [PATCH 3/4] .ocTransferIDXXXXX.part makes filename too long: include transferId + UnitTests #25425 --- apps/dav/lib/Connector/Sabre/File.php | 5 +- .../tests/unit/Connector/Sabre/FileTest.php | 83 +++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index e4d3dc0cea82..7d38803147d3 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -427,12 +427,11 @@ private function createFileChunked($data) { list($targetStorage, $targetInternalPath) = $this->fileView->resolvePath($targetPath); if ($needsPartFile) { - // we first assembly the target file as a part file - $partFile = $this->getPartFileBasePath($path . '/' . $info['name']) . '.ocTransferId' . $info['transferid'] . '.part'; + // we first assemble the target file as a part file + $partFile = $this->getPartFileBasePath($path . '/' . sha1($info['name'])) . '.ocTransferId' . $info['transferid'] . '.part'; /** @var \OC\Files\Storage\Storage $targetStorage */ list($partStorage, $partInternalPath) = $this->fileView->resolvePath($partFile); - $chunk_handler->file_assemble($partStorage, $partInternalPath); // here is the final atomic rename diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index e8ce11fb20f4..6de45c02ffbc 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -1002,4 +1002,87 @@ public function testGetFopenThrows() { $file->get(); } + + public function testPutCreatesPartFile() { + $stream = $this->getStream('ABCDEFG'); + $storage = $this->getMockBuilder(Local::class) + ->setMethods(['fopen', 'moveFromStorage', 'file_exists']) + ->setConstructorArgs([['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]) + ->getMock(); + + $storage->expects($this->atLeastOnce()) + ->method('fopen') + ->with($this->matchesRegularExpression('/^([0-9a-f]{40})(.ocTransferId)([0-9]+)(.part)$/i')) + ->willReturn($stream); + + $storage->method('moveFromStorage') + ->willReturn(true); + + $storage->method('file_exists') + ->willReturn(true); + + + $path = '/' . $this->user . '/files'; + $info = new FileInfo($path, $this->getMockStorage(), null, [ + 'permissions' => Constants::PERMISSION_ALL + ], null); + + $view = $this->getMockBuilder(View::class) + ->setMethods(['fopen', 'resolvePath']) + ->getMock(); + + + $view->expects($this->atLeastOnce()) + ->method('resolvePath') + ->will($this->returnCallback( + function ($path) use ($storage) { + return [$storage, $path]; + } + )); + + $file = new File($view, $info); + $file->put($stream); + } + + public function testPutWithChunkCreatesPartFile() { + $stream = $this->getStream('ABCDEFG'); + + $storage = $this->getMockBuilder(Local::class) + ->setMethods(['fopen', 'moveFromStorage', 'file_exists']) + ->setConstructorArgs([['datadir' => \OC::$server->getTempManager()->getTemporaryFolder()]]) + ->getMock(); + + $storage->expects($this->atLeastOnce()) + ->method('fopen') + ->with($this->matchesRegularExpression('/^(\/[0-9a-f]{40})(.ocTransferId)([0-9]+)(.part)$/i')) + ->willReturn($this->getStream('FOO1231')); + + $path = 'foo.dat-chunking-13242432-0-0'; + + $info = new FileInfo($path, $this->getMockStorage(), null, [ + 'permissions' => Constants::PERMISSION_ALL + ], null); + $info['checksum'] = 'abcdefg123'; + $info['etag'] = 'deadbeef1337'; + + $view = $this->getMockBuilder(View::class) + ->setMethods(['fopen', 'resolvePath', 'getFileInfo']) + ->getMock(); + + $view->method('getFileInfo') + ->willReturn($info); + + $view->expects($this->atLeastOnce()) + ->method('resolvePath') + ->will($this->returnCallback( + function ($path) use ($storage) { + return [$storage, $path]; + } + )); + + + $_SERVER['HTTP_OC_CHUNKED'] = true; + $file = new File($view, $info); + $file->put($stream); + } } From 8656192aa721dedfac24004f41580699b7ff7d0b Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Tue, 24 Jan 2017 14:44:04 +0100 Subject: [PATCH 4/4] .ocTransferIDXXXXX.part makes filename too long: Hash only the filename + simplified unit-tests #25425 --- apps/dav/lib/Connector/Sabre/File.php | 3 ++- apps/dav/tests/unit/Connector/Sabre/FileTest.php | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php index 7d38803147d3..090eb5fcac27 100644 --- a/apps/dav/lib/Connector/Sabre/File.php +++ b/apps/dav/lib/Connector/Sabre/File.php @@ -111,7 +111,8 @@ public function put($data) { if ($needsPartFile) { // mark file as partial while uploading (ignored by the scanner) - $partFilePath = sha1($this->getPartFileBasePath($this->path)) . '.ocTransferId' . rand() . '.part'; + $fullPath = $this->getPartFileBasePath($this->path); + $partFilePath = dirname($fullPath) . '/' . sha1(basename($fullPath)) . '.ocTransferId' . rand() . '.part'; } else { // upload file directly as the final path $partFilePath = $this->path; diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php index 6de45c02ffbc..9b82542ddb8f 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php @@ -1012,7 +1012,7 @@ public function testPutCreatesPartFile() { $storage->expects($this->atLeastOnce()) ->method('fopen') - ->with($this->matchesRegularExpression('/^([0-9a-f]{40})(.ocTransferId)([0-9]+)(.part)$/i')) + ->with($this->matchesRegularExpression('/a1f13b3bc20a296e08c212be9c56c706c10abc4f.ocTransferId([0-9]+).part$/i')) ->willReturn($stream); $storage->method('moveFromStorage') @@ -1054,7 +1054,7 @@ public function testPutWithChunkCreatesPartFile() { $storage->expects($this->atLeastOnce()) ->method('fopen') - ->with($this->matchesRegularExpression('/^(\/[0-9a-f]{40})(.ocTransferId)([0-9]+)(.part)$/i')) + ->with($this->matchesRegularExpression('/^\/1cf7c9c735f7fd721ebe33ea1e7d259397f24007.ocTransferId([0-9]+).part$/i')) ->willReturn($this->getStream('FOO1231')); $path = 'foo.dat-chunking-13242432-0-0';