Skip to content

Commit 264ee75

Browse files
skjnldsvskjnldsv
committed
feat(dav): allow uploading folders to public shares
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
1 parent a4d7c12 commit 264ee75

File tree

3 files changed

+179
-36
lines changed

3 files changed

+179
-36
lines changed

‎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: 84 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,57 +36,118 @@ 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) {
48+
// If this is a folder creation request we need
49+
// to fake a success so we can pretend every
50+
// folder now exists.
51+
$response->setStatus(201);
52+
return false;
53+
}
54+
55+
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
5156
if (!$this->enabled || $this->share === null || $this->view === null) {
5257
return;
5358
}
5459

55-
// Only allow file drop
60+
// Retrieve the nickname from the request
61+
$nickName = $request->hasHeader('X-NC-Nickname')
62+
? trim(urldecode($request->getHeader('X-NC-Nickname')))
63+
: null;
64+
65+
//
5666
if ($request->getMethod() !== 'PUT') {
57-
throw new MethodNotAllowed('Only PUT is allowed on files drop');
67+
// If uploading subfolders we need to ensure they get created
68+
// within the nickname folder
69+
if ($request->getMethod() === 'MKCOL') {
70+
if (!$nickName) {
71+
throw new MethodNotAllowed('A nickname header is required when uploading subfolders');
72+
}
73+
} else {
74+
throw new MethodNotAllowed('Only PUT is allowed on files drop');
75+
}
76+
}
77+
78+
// If this is a folder creation request
79+
// let's stop there and let the onMkcol handle it
80+
if ($request->getMethod() === 'MKCOL') {
81+
return;
5882
}
5983

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

6497
// Extract the attributes for the file request
6598
$isFileRequest = false;
6699
$attributes = $this->share->getAttributes();
67-
$nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null;
68100
if ($attributes !== null) {
69101
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
70102
}
71103

72104
// 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');
105+
if ($isFileRequest && !$nickName) {
106+
throw new MethodNotAllowed('A nickname header is required for file requests');
75107
}
76108

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-
}
109+
// If we have a nickname, let's put everything inside
110+
if ($nickName) {
83111
// Put all files in the subfolder
84-
$path = $nickName . '/' . $path;
112+
$relativePath = '/' . $nickName . '/' . $relativePath;
113+
$relativePath = str_replace('//', '/', $relativePath);
85114
}
86115

87-
$newName = \OC_Helper::buildNotExistingFileNameForView('/', $path, $this->view);
88-
$url = $request->getBaseUrl() . '/files/' . $this->share->getToken() . $newName;
116+
// Create the folders along the way
117+
$folders = $this->getPathSegments(dirname($relativePath));
118+
foreach ($folders as $folder) {
119+
if ($folder === '') continue; // skip empty parts
120+
if (!$this->view->file_exists($folder)) {
121+
$this->view->mkdir($folder);
122+
}
123+
}
124+
125+
// Finally handle conflicts on the end files
126+
$noConflictPath = \OC_Helper::buildNotExistingFileNameForView(dirname($relativePath), basename($relativePath), $this->view);
127+
$path = '/files/' . $token . '/'. $noConflictPath;
128+
$url = $request->getBaseUrl() . str_replace('//', '/', $path);
89129
$request->setUrl($url);
90130
}
91131

132+
private function getPathSegments(string $path): array {
133+
// Normalize slashes and remove trailing slash
134+
$path = rtrim(str_replace('\\', '/', $path), '/');
135+
136+
// Handle absolute paths starting with /
137+
$isAbsolute = str_starts_with($path, '/');
138+
139+
$segments = explode('/', $path);
140+
141+
// Add back the leading slash for the first segment if needed
142+
$result = [];
143+
$current = $isAbsolute ? '/' : '';
144+
145+
foreach ($segments as $segment) {
146+
if ($segment === '') continue; // skip empty parts
147+
$current = rtrim($current, '/') . '/' . $segment;
148+
$result[] = $current;
149+
}
150+
151+
return $result;
152+
}
92153
}

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

Lines changed: 89 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,7 +152,27 @@ 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);
@@ -165,7 +188,7 @@ public function testNoSubdirPut(): void {
165188

166189
$this->view->method('file_exists')
167190
->willReturnCallback(function ($path) {
168-
if ($path === 'file.txt' || $path === '/file.txt') {
191+
if ($path === 'file.txt' || $path === '/folder/file.txt') {
169192
return true;
170193
} else {
171194
return false;
@@ -174,8 +197,66 @@ public function testNoSubdirPut(): void {
174197

175198
$this->request->expects($this->once())
176199
->method('setUrl')
177-
->with($this->equalTo('https://example.com/files/token/file (2).txt'));
200+
->with($this->equalTo('https://example.com/files/token/folder/file (2).txt'));
201+
202+
$this->plugin->beforeMethod($this->request, $this->response);
203+
}
204+
205+
public function testRecursiveFolderCreation(): void {
206+
$this->plugin->enable();
207+
$this->plugin->setView($this->view);
208+
$this->plugin->setShare($this->share);
178209

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

0 commit comments

Comments
 (0)