Merge pull request #55800 from nextcloud/fix/file-drop/chunked-upload

Fix chunked upload for file drop shares
pull/55803/head
Arthur Schiwon 2025-10-16 09:43:22 +07:00 committed by GitHub
commit 84be993633
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 52 additions and 77 deletions

@ -81,7 +81,7 @@ $linkCheckPlugin = new PublicLinkCheckPlugin();
$filesDropPlugin = new FilesDropPlugin();
/** @var string $baseuri defined in public.php */
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($authBackend, $linkCheckPlugin, $filesDropPlugin) {
$server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin, function (\Sabre\DAV\Server $server) use ($baseuri, $requestUri, $authBackend, $linkCheckPlugin, $filesDropPlugin) {
// GET must be allowed for e.g. showing images and allowing Zip downloads
if ($server->httpRequest->getMethod() !== 'GET') {
// If this is *not* a GET request we only allow access to public DAV from AJAX or when Server2Server is allowed
@ -103,8 +103,16 @@ $server = $serverFactory->createServer(true, $baseuri, $requestUri, $authPlugin,
$previousLog = Filesystem::logWarningWhenAddingStorageWrapper(false);
/** @psalm-suppress MissingClosureParamType */
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($share) {
return new PermissionsMask(['storage' => $storage, 'mask' => $share->getPermissions() | Constants::PERMISSION_SHARE]);
Filesystem::addStorageWrapper('sharePermissions', function ($mountPoint, $storage) use ($requestUri, $baseuri, $share) {
$mask = $share->getPermissions() | Constants::PERMISSION_SHARE;
// For chunked uploads it is necessary to have read and delete permission,
// so the temporary directory, chunks and destination file can be read and delete after the assembly.
if (str_starts_with(substr($requestUri, strlen($baseuri) - 1), '/uploads/')) {
$mask |= Constants::PERMISSION_READ | Constants::PERMISSION_DELETE;
}
return new PermissionsMask(['storage' => $storage, 'mask' => $mask]);
});
/** @psalm-suppress MissingClosureParamType */

@ -42,6 +42,10 @@ class FilesDropPlugin extends ServerPlugin {
}
public function onMkcol(RequestInterface $request, ResponseInterface $response) {
if ($this->isChunkedUpload($request)) {
return;
}
if (!$this->enabled || $this->share === null) {
return;
}
@ -58,7 +62,18 @@ class FilesDropPlugin extends ServerPlugin {
return false;
}
private function isChunkedUpload(RequestInterface $request): bool {
return str_starts_with(substr($request->getUrl(), strlen($request->getBaseUrl()) - 1), '/uploads/');
}
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
$isChunkedUpload = $this->isChunkedUpload($request);
// For the final MOVE request of a chunked upload it is necessary to modify the Destination header.
if ($isChunkedUpload && $request->getMethod() !== 'MOVE') {
return;
}
if (!$this->enabled || $this->share === null) {
return;
}
@ -68,21 +83,8 @@ class FilesDropPlugin extends ServerPlugin {
return;
}
// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;
if ($request->getMethod() !== 'PUT') {
// If uploading subfolders we need to ensure they get created
// within the nickname folder
if ($request->getMethod() === 'MKCOL') {
if (!$nickname) {
throw new BadRequest('A nickname header is required when uploading subfolders');
}
} else {
throw new MethodNotAllowed('Only PUT is allowed on files drop');
}
if ($request->getMethod() !== 'PUT' && $request->getMethod() !== 'MKCOL' && (!$isChunkedUpload || $request->getMethod() !== 'MOVE')) {
throw new MethodNotAllowed('Only PUT, MKCOL and MOVE are allowed on files drop');
}
// If this is a folder creation request
@ -95,8 +97,16 @@ class FilesDropPlugin extends ServerPlugin {
// full path along the way. We'll only handle conflict
// resolution on file conflicts, but not on folders.
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = $request->getPath();
if ($isChunkedUpload) {
$destination = $request->getHeader('destination');
$baseUrl = $request->getBaseUrl();
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = substr($destination, strpos($destination, $baseUrl) + strlen($baseUrl));
} else {
// e.g files/dCP8yn3N86EK9sL/Folder/image.jpg
$path = $request->getPath();
}
$token = $this->share->getToken();
// e.g files/dCP8yn3N86EK9sL
@ -112,6 +122,11 @@ class FilesDropPlugin extends ServerPlugin {
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
}
// Retrieve the nickname from the request
$nickname = $request->hasHeader('X-NC-Nickname')
? trim(urldecode($request->getHeader('X-NC-Nickname')))
: null;
// We need a valid nickname for file requests
if ($isFileRequest && !$nickname) {
throw new BadRequest('A nickname header is required for file requests');
@ -187,7 +202,11 @@ class FilesDropPlugin extends ServerPlugin {
$relativePath = substr($folder->getPath(), strlen($node->getPath()));
$path = '/files/' . $token . '/' . $relativePath . '/' . $uniqueName;
$url = rtrim($request->getBaseUrl(), '/') . str_replace('//', '/', $path);
$request->setUrl($url);
if ($isChunkedUpload) {
$request->setHeader('destination', $url);
} else {
$request->setUrl($url);
}
}
private function getPathSegments(string $path): array {

@ -13,7 +13,6 @@ use OCP\Files\NotFoundException;
use OCP\Share\IAttributes;
use OCP\Share\IShare;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\Server;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
@ -56,13 +55,6 @@ class FilesDropPluginTest extends TestCase {
->willReturn('token');
}
public function testNotEnabled(): void {
$this->request->expects($this->never())
->method($this->anything());
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testValid(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);
@ -112,32 +104,13 @@ class FilesDropPluginTest extends TestCase {
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testNoMKCOLWithoutNickname(): void {
public function testMKCOL(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);
$this->request->method('getMethod')
->willReturn('MKCOL');
$this->expectException(BadRequest::class);
$this->plugin->beforeMethod($this->request, $this->response);
}
public function testMKCOLWithNickname(): void {
$this->plugin->enable();
$this->plugin->setShare($this->share);
$this->request->method('getMethod')
->willReturn('MKCOL');
$this->request->method('hasHeader')
->with('X-NC-Nickname')
->willReturn(true);
$this->request->method('getHeader')
->with('X-NC-Nickname')
->willReturn('nickname');
$this->expectNotToPerformAssertions();
$this->plugin->beforeMethod($this->request, $this->response);

@ -57,7 +57,7 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
/**
* @When Creating folder :folder in drop
*/
public function creatingFolderInDrop($folder, $nickname = null) {
public function creatingFolderInDrop($folder) {
$client = new Client();
$options = [];
if (count($this->lastShareData->data->element) > 0) {
@ -73,22 +73,10 @@ class FilesDropContext implements Context, SnippetAcceptingContext {
'X-REQUESTED-WITH' => 'XMLHttpRequest',
];
if ($nickname) {
$options['headers']['X-NC-NICKNAME'] = $nickname;
}
try {
$this->response = $client->request('MKCOL', $fullUrl, $options);
} catch (\GuzzleHttp\Exception\ClientException $e) {
$this->response = $e->getResponse();
}
}
/**
* @When Creating folder :folder in drop as :nickName
*/
public function creatingFolderInDropWithNickname($folder, $nickname) {
return $this->creatingFolderInDrop($folder, $nickname);
}
}

@ -46,7 +46,7 @@ Feature: FilesDrop
When Dropping file "/folder/a.txt" with "abc"
Then the HTTP status code should be "400"
Scenario: Files drop forbid MKCOL without a nickname
Scenario: Files drop allows MKCOL
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
@ -57,19 +57,6 @@ Feature: FilesDrop
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop
Then the HTTP status code should be "400"
Scenario: Files drop allows MKCOL with a nickname
Given user "user0" exists
And As an "user0"
And user "user0" created a folder "/drop"
And as "user0" creating a share with
| path | drop |
| shareType | 3 |
| publicUpload | true |
And Updating last share with
| permissions | 4 |
When Creating folder "folder" in drop as "nickname"
Then the HTTP status code should be "201"
Scenario: Files drop forbid subfolder creation without a nickname
@ -139,7 +126,7 @@ Feature: FilesDrop
When Downloading file "/drop/Alice/folder (2)"
Then the HTTP status code should be "200"
And Downloaded content should be "its a file"
Scenario: Put file same file multiple times via files drop
Given user "user0" exists
And As an "user0"