Skip to content

Commit 0718c70

Browse files
salmart-devbackportbot[bot]
authored andcommitted
fix: check node on same storage when uploading chunks
fix: check node on same storage when uploading chunks Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com> [skip ci]
1 parent 4b702bf commit 0718c70

2 files changed

Lines changed: 36 additions & 7 deletions

File tree

apps/dav/lib/Upload/ChunkingV2Plugin.php

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
use OCP\IConfig;
4646
use OCP\Lock\ILockingProvider;
4747
use Sabre\DAV\Exception\BadRequest;
48+
use Sabre\DAV\Exception\Forbidden;
4849
use Sabre\DAV\Exception\InsufficientStorage;
4950
use Sabre\DAV\Exception\MethodNotAllowed;
5051
use Sabre\DAV\Exception\NotFound;
@@ -118,18 +119,26 @@ public function beforeGet(RequestInterface $request) {
118119
* @param string $path
119120
* @param bool $createIfNotExists
120121
* @return FutureFile|UploadFile|ICollection|INode
122+
* @throws Forbidden if the file already exists, but is not updateable
121123
*/
122124
private function getUploadFile(string $path, bool $createIfNotExists = false) {
123125
try {
124126
$actualFile = $this->server->tree->getNodeForPath($path);
125-
// Only directly upload to the target file if it is on the same storage
126-
// There may be further potential to optimize here by also uploading
127-
// to other storages directly. This would require to also carefully pick
128-
// the storage/path used in getStorage()
129-
if ($actualFile instanceof File && $this->uploadFolder->getStorage()->getId() === $actualFile->getNode()->getStorage()->getId()) {
130-
return $actualFile;
127+
if ($actualFile instanceof File) {
128+
$node = $actualFile->getNode();
129+
// check that the node has update permissions
130+
if (!$node->isUpdateable()) {
131+
throw new Forbidden();
132+
}
133+
// Only directly upload to the target file if it is on the same storage
134+
// There may be further potential to optimize here by also uploading
135+
// to other storages directly. This would require to also carefully pick
136+
// the storage/path used in getStorage()
137+
if ($this->uploadFolder->getStorage()->getId() === $node->getStorage()->getId()) {
138+
return $actualFile;
139+
}
131140
}
132-
} catch (NotFound $e) {
141+
} catch (NotFound) {
133142
// If there is no target file we upload to the upload folder first
134143
}
135144

build/integration/features/webdav-related.feature

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,3 +726,23 @@ Feature: webdav-related
726726
When As an "user0"
727727
And Downloading file "/üäöé/äöü.txt"
728728
Then Downloaded content should be the created file
729+
730+
@s3-multipart
731+
Scenario: Cannot overwrite a file the user may not update with new chunking v2
732+
Given using new dav path
733+
And user "user0" exists
734+
And user "user1" exists
735+
And As an "user1"
736+
And user "user1" created a folder "/testshare"
737+
And User "user1" copies file "/welcome.txt" to "/testshare/test.txt"
738+
And as "user1" creating a share with
739+
| path | testshare |
740+
| shareType | 0 |
741+
| permissions | 5 |
742+
| shareWith | user0 |
743+
And user "user0" accepts last share
744+
And As an "user0"
745+
And user "user0" creates a file locally with "3" x 5 MB chunks
746+
When user "user0" creates a new chunking v2 upload with id "chunking-update" and destination "/testshare/test.txt"
747+
Then the HTTP status code should be "403"
748+
And Downloaded content when downloading file "/testshare/test.txt" with range "bytes=0-6" should be "Welcome"

0 commit comments

Comments
 (0)