Skip to content

Commit b9da941

Browse files
skjnldsvskjnldsv
authored
Merge pull request #52785 from nextcloud/feat/file-drop-recursive
2 parents 56897b6 + b286bca commit b9da941

File tree

7 files changed

+305
-66
lines changed

7 files changed

+305
-66
lines changed

‎apps/dav/lib/Connector/Sabre/ChecksumUpdatePlugin.php‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,6 @@ public function getPluginName(): string {
2626
return 'checksumupdate';
2727
}
2828

29-
/** @return string[] */
30-
public function getHTTPMethods($path): array {
31-
$tree = $this->server->tree;
32-
33-
if ($tree->nodeExists($path)) {
34-
$node = $tree->getNodeForPath($path);
35-
if ($node instanceof File) {
36-
return ['PATCH'];
37-
}
38-
}
39-
40-
return [];
41-
}
42-
4329
/** @return string[] */
4430
public function getFeatures(): array {
4531
return ['nextcloud-checksum-update'];

‎apps/dav/lib/Connector/Sabre/Directory.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,14 @@ public function createDirectory($name) {
176176
public function getChild($name, $info = null, ?IRequest $request = null, ?IL10N $l10n = null) {
177177
$storage = $this->info->getStorage();
178178
$allowDirectory = false;
179+
180+
// Checking if we're in a file drop
181+
// If we are, then only PUT and MKCOL are allowed (see plugin)
182+
// so we are safe to return the directory without a risk of
183+
// leaking files and folders structure.
179184
if ($storage instanceof PublicShareWrapper) {
180185
$share = $storage->getShare();
181-
$allowDirectory =
182-
// Only allow directories for file drops
183-
($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ &&
184-
// And only allow it for directories which are a direct child of the share root
185-
$this->info->getId() === $share->getNodeId();
186+
$allowDirectory = ($share->getPermissions() & Constants::PERMISSION_READ) !== Constants::PERMISSION_READ;
186187
}
187188

188189
// For file drop we need to be allowed to read the directory with the nickname

‎apps/dav/lib/Files/Sharing/FilesDropPlugin.php‎

Lines changed: 102 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,57 +36,136 @@ public function enable(): void {
3636

3737
/**
3838
* This initializes the plugin.
39-
*
40-
* @param \Sabre\DAV\Server $server Sabre server
41-
*
42-
* @return void
43-
* @throws MethodNotAllowed
39+
* It is ONLY initialized by the server on a file drop request.
4440
*/
4541
public function initialize(\Sabre\DAV\Server $server): void {
4642
$server->on('beforeMethod:*', [$this, 'beforeMethod'], 999);
43+
$server->on('method:MKCOL', [$this, 'onMkcol']);
4744
$this->enabled = false;
4845
}
4946

50-
public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
47+
public function onMkcol(RequestInterface $request, ResponseInterface $response) {
5148
if (!$this->enabled || $this->share === null || $this->view === null) {
5249
return;
5350
}
5451

55-
// Only allow file drop
52+
// If this is a folder creation request we need
53+
// to fake a success so we can pretend every
54+
// folder now exists.
55+
$response->setStatus(201);
56+
return false;
57+
}
58+
59+
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
60+
if (!$this->enabled || $this->share === null || $this->view === null) {
61+
return;
62+
}
63+
64+
// Retrieve the nickname from the request
65+
$nickname = $request->hasHeader('X-NC-Nickname')
66+
? trim(urldecode($request->getHeader('X-NC-Nickname')))
67+
: null;
68+
69+
//
5670
if ($request->getMethod() !== 'PUT') {
57-
throw new MethodNotAllowed('Only PUT is allowed on files drop');
71+
// If uploading subfolders we need to ensure they get created
72+
// within the nickname folder
73+
if ($request->getMethod() === 'MKCOL') {
74+
if (!$nickname) {
75+
throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
76+
}
77+
} else {
78+
throw new MethodNotAllowed('Only PUT is allowed on files drop');
79+
}
80+
}
81+
82+
// If this is a folder creation request
83+
// let's stop there and let the onMkcol handle it
84+
if ($request->getMethod() === 'MKCOL') {
85+
return;
5886
}
5987

60-
// Always upload at the root level
61-
$path = explode('/', $request->getPath());
62-
$path = array_pop($path);
88+
// Now if we create a file, we need to create the
89+
// full path along the way. We'll only handle conflict
90+
// resolution on file conflicts, but not on folders.
91+
92+
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
93+
$path = $request->getPath();
94+
$token = $this->share->getToken();
95+
96+
// e.g files/dCP8yn3N86EK9sL
97+
$rootPath = substr($path, 0, strpos($path, $token) + strlen($token));
98+
// e.g /Folder/image.jpg
99+
$relativePath = substr($path, strlen($rootPath));
100+
$isRootUpload = substr_count($relativePath, '/') === 1;
63101

64102
// Extract the attributes for the file request
65103
$isFileRequest = false;
66104
$attributes = $this->share->getAttributes();
67-
$nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null;
68105
if ($attributes !== null) {
69106
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
70107
}
71108

72109
// We need a valid nickname for file requests
73-
if ($isFileRequest && ($nickName == null || trim($nickName) === '')) {
74-
throw new MethodNotAllowed('Nickname is required for file requests');
110+
if ($isFileRequest && !$nickname) {
111+
throw new MethodNotAllowed('A nickname header is required for file requests');
75112
}
76113

77-
// If this is a file request we need to create a folder for the user
78-
if ($isFileRequest) {
79-
// Check if the folder already exists
80-
if (!($this->view->file_exists($nickName) === true)) {
81-
$this->view->mkdir($nickName);
82-
}
114+
// We're only allowing the upload of
115+
// long path with subfolders if a nickname is set.
116+
// This prevents confusion when uploading files and help
117+
// classify them by uploaders.
118+
if (!$nickname && !$isRootUpload) {
119+
throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
120+
}
121+
122+
// If we have a nickname, let's put everything inside
123+
if ($nickname) {
83124
// Put all files in the subfolder
84-
$path = $nickName . '/' . $path;
125+
$relativePath = '/' . $nickname . '/' . $relativePath;
126+
$relativePath = str_replace('//', '/', $relativePath);
85127
}
86128

87-
$newName = \OC_Helper::buildNotExistingFileNameForView('/', $path, $this->view);
88-
$url = $request->getBaseUrl() . '/files/' . $this->share->getToken() . $newName;
129+
// Create the folders along the way
130+
$folders = $this->getPathSegments(dirname($relativePath));
131+
foreach ($folders as $folder) {
132+
if ($folder === '') {
133+
continue;
134+
} // skip empty parts
135+
if (!$this->view->file_exists($folder)) {
136+
$this->view->mkdir($folder);
137+
}
138+
}
139+
140+
// Finally handle conflicts on the end files
141+
$noConflictPath = \OC_Helper::buildNotExistingFileNameForView(dirname($relativePath), basename($relativePath), $this->view);
142+
$path = '/files/' . $token . '/' . $noConflictPath;
143+
$url = $request->getBaseUrl() . str_replace('//', '/', $path);
89144
$request->setUrl($url);
90145
}
91146

147+
private function getPathSegments(string $path): array {
148+
// Normalize slashes and remove trailing slash
149+
$path = rtrim(str_replace('\\', '/', $path), '/');
150+
151+
// Handle absolute paths starting with /
152+
$isAbsolute = str_starts_with($path, '/');
153+
154+
$segments = explode('/', $path);
155+
156+
// Add back the leading slash for the first segment if needed
157+
$result = [];
158+
$current = $isAbsolute ? '/' : '';
159+
160+
foreach ($segments as $segment) {
161+
if ($segment === '') {
162+
// skip empty parts
163+
continue;
164+
}
165+
$current = rtrim($current, '/') . '/' . $segment;
166+
$result[] = $current;
167+
}
168+
169+
return $result;
170+
}
92171
}

‎apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php‎

Lines changed: 100 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ protected function setUp(): void {
4646
$this->request = $this->createMock(RequestInterface::class);
4747
$this->response = $this->createMock(ResponseInterface::class);
4848

49-
$this->response->expects($this->never())
50-
->method($this->anything());
51-
5249
$attributes = $this->createMock(IAttributes::class);
5350
$this->share->expects($this->any())
5451
->method('getAttributes')
@@ -60,13 +57,19 @@ protected function setUp(): void {
6057
}
6158

6259
public function testInitialize(): void {
63-
$this->server->expects($this->once())
60+
$this->server->expects($this->at(0))
6461
->method('on')
6562
->with(
6663
$this->equalTo('beforeMethod:*'),
6764
$this->equalTo([$this->plugin, 'beforeMethod']),
6865
$this->equalTo(999)
6966
);
67+
$this->server->expects($this->at(1))
68+
->method('on')
69+
->with(
70+
$this->equalTo('method:MKCOL'),
71+
$this->equalTo([$this->plugin, 'onMkcol']),
72+
);
7073

7174
$this->plugin->initialize($this->server);
7275
}
@@ -136,7 +139,7 @@ public function testFileAlreadyExistsValid(): void {
136139
$this->plugin->beforeMethod($this->request, $this->response);
137140
}
138141

139-
public function testNoMKCOL(): void {
142+
public function testNoMKCOLWithoutNickname(): void {
140143
$this->plugin->enable();
141144
$this->plugin->setView($this->view);
142145
$this->plugin->setShare($this->share);
@@ -149,14 +152,41 @@ public function testNoMKCOL(): void {
149152
$this->plugin->beforeMethod($this->request, $this->response);
150153
}
151154

152-
public function testNoSubdirPut(): void {
155+
public function testMKCOLWithNickname(): void {
156+
$this->plugin->enable();
157+
$this->plugin->setView($this->view);
158+
$this->plugin->setShare($this->share);
159+
160+
$this->request->method('getMethod')
161+
->willReturn('MKCOL');
162+
163+
$this->request->method('hasHeader')
164+
->with('X-NC-Nickname')
165+
->willReturn(true);
166+
$this->request->method('getHeader')
167+
->with('X-NC-Nickname')
168+
->willReturn('nickname');
169+
170+
$this->expectNotToPerformAssertions();
171+
172+
$this->plugin->beforeMethod($this->request, $this->response);
173+
}
174+
175+
public function testSubdirPut(): void {
153176
$this->plugin->enable();
154177
$this->plugin->setView($this->view);
155178
$this->plugin->setShare($this->share);
156179

157180
$this->request->method('getMethod')
158181
->willReturn('PUT');
159182

183+
$this->request->method('hasHeader')
184+
->with('X-NC-Nickname')
185+
->willReturn(true);
186+
$this->request->method('getHeader')
187+
->with('X-NC-Nickname')
188+
->willReturn('nickname');
189+
160190
$this->request->method('getPath')
161191
->willReturn('/files/token/folder/file.txt');
162192

@@ -165,7 +195,7 @@ public function testNoSubdirPut(): void {
165195

166196
$this->view->method('file_exists')
167197
->willReturnCallback(function ($path) {
168-
if ($path === 'file.txt' || $path === '/file.txt') {
198+
if ($path === 'file.txt' || $path === '/folder/file.txt') {
169199
return true;
170200
} else {
171201
return false;
@@ -174,8 +204,70 @@ public function testNoSubdirPut(): void {
174204

175205
$this->request->expects($this->once())
176206
->method('setUrl')
177-
->with($this->equalTo('https://example.com/files/token/file (2).txt'));
207+
->with($this->equalTo('https://example.com/files/token/nickname/folder/file.txt'));
178208

179209
$this->plugin->beforeMethod($this->request, $this->response);
180210
}
211+
212+
public function testRecursiveFolderCreation(): void {
213+
$this->plugin->enable();
214+
$this->plugin->setView($this->view);
215+
$this->plugin->setShare($this->share);
216+
217+
$this->request->method('getMethod')
218+
->willReturn('PUT');
219+
$this->request->method('hasHeader')
220+
->with('X-NC-Nickname')
221+
->willReturn(true);
222+
$this->request->method('getHeader')
223+
->with('X-NC-Nickname')
224+
->willReturn('nickname');
225+
226+
$this->request->method('getPath')
227+
->willReturn('/files/token/folder/subfolder/file.txt');
228+
$this->request->method('getBaseUrl')
229+
->willReturn('https://example.com');
230+
$this->view->method('file_exists')
231+
->willReturn(false);
232+
233+
$this->view->expects($this->exactly(4))
234+
->method('file_exists')
235+
->withConsecutive(
236+
['/nickname'],
237+
['/nickname/folder'],
238+
['/nickname/folder/subfolder'],
239+
['/nickname/folder/subfolder/file.txt']
240+
)
241+
->willReturnOnConsecutiveCalls(
242+
false,
243+
false,
244+
false,
245+
false,
246+
);
247+
$this->view->expects($this->exactly(3))
248+
->method('mkdir')
249+
->withConsecutive(
250+
['/nickname'],
251+
['/nickname/folder'],
252+
['/nickname/folder/subfolder'],
253+
);
254+
255+
$this->request->expects($this->once())
256+
->method('setUrl')
257+
->with($this->equalTo('https://example.com/files/token/nickname/folder/subfolder/file.txt'));
258+
$this->plugin->beforeMethod($this->request, $this->response);
259+
}
260+
261+
public function testOnMkcol(): void {
262+
$this->plugin->enable();
263+
$this->plugin->setView($this->view);
264+
$this->plugin->setShare($this->share);
265+
266+
$this->response->expects($this->once())
267+
->method('setStatus')
268+
->with(201);
269+
270+
$response = $this->plugin->onMkcol($this->request, $this->response);
271+
$this->assertFalse($response);
272+
}
181273
}

0 commit comments

Comments
 (0)