Skip to content

Commit b7454a4

Browse files
susnuxsusnux
authored
Merge pull request #63908 from nextcloud/backport/63892/stable35
[stable35] fix(dav): Do not claim no plugin available when checksum algorithm is not supported
2 parents 034a698 + b450784 commit b7454a4

2 files changed

Lines changed: 202 additions & 12 deletions

File tree

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

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88

99
namespace OCA\DAV\Connector\Sabre;
1010

11+
use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
1112
use OCP\AppFramework\Http;
13+
use OCP\Files\ForbiddenException;
14+
use Sabre\DAV\Exception;
15+
use Sabre\DAV\Exception\BadRequest;
1216
use Sabre\DAV\Server;
1317
use Sabre\DAV\ServerPlugin;
1418
use Sabre\HTTP\RequestInterface;
@@ -34,25 +38,54 @@ public function getFeatures(): array {
3438
return ['nextcloud-checksum-update'];
3539
}
3640

41+
/**
42+
* @throws BadRequest if the requested hash algorithm is not supported
43+
* @throws DAVForbiddenException if the user may not read and update the node
44+
* @throws Exception if the checksum could not be calculated
45+
*/
3746
public function httpPatch(RequestInterface $request, ResponseInterface $response) {
3847
$path = $request->getPath();
3948

4049
$node = $this->server->tree->getNodeForPath($path);
41-
if ($node instanceof File) {
42-
$type = strtolower(
43-
(string)$request->getHeader('X-Recalculate-Hash')
44-
);
50+
if (!$node instanceof File) {
51+
return;
52+
}
53+
54+
$type = strtolower(
55+
(string)$request->getHeader('X-Recalculate-Hash')
56+
);
57+
58+
// Without the header this is not a checksum update, leave the request to other plugins
59+
if ($type === '') {
60+
return;
61+
}
62+
63+
if (!in_array($type, hash_algos(), true)) {
64+
throw new BadRequest('Unsupported hash algorithm "' . $type . '"');
65+
}
66+
67+
// Recalculating reads the whole file content and persists the result,
68+
// so it requires both read and write access to the node.
69+
$info = $node->getFileInfo();
70+
if (!$info->isReadable() || !$info->isUpdateable()) {
71+
throw new DAVForbiddenException('You are not allowed to recalculate the checksum of this file', false);
72+
}
4573

74+
try {
4675
$hash = $node->hash($type);
47-
if ($hash) {
48-
$checksum = strtoupper($type) . ':' . $hash;
49-
$node->setChecksum($checksum);
50-
$response->addHeader('OC-Checksum', $checksum);
51-
$response->setHeader('Content-Length', '0');
52-
$response->setStatus(Http::STATUS_NO_CONTENT);
53-
54-
return false;
76+
if ($hash === false) {
77+
throw new Exception('Could not calculate the ' . $type . ' checksum of "' . $path . '"');
5578
}
79+
80+
$checksum = strtoupper($type) . ':' . $hash;
81+
$node->setChecksum($checksum);
82+
83+
$response->addHeader('OC-Checksum', $checksum);
84+
$response->setHeader('Content-Length', '0');
85+
$response->setStatus(Http::STATUS_NO_CONTENT);
86+
return false;
87+
} catch (ForbiddenException $e) {
88+
throw new DAVForbiddenException($e->getMessage(), $e->getRetry(), $e);
5689
}
5790
}
5891
}

‎apps/dav/tests/unit/Connector/Sabre/ChecksumUpdatePluginTest.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Tests\unit\Connector\Sabre;
11+
12+
use OCA\DAV\Connector\Sabre\ChecksumUpdatePlugin;
13+
use OCA\DAV\Connector\Sabre\Directory;
14+
use OCA\DAV\Connector\Sabre\Exception\Forbidden as DAVForbiddenException;
15+
use OCA\DAV\Connector\Sabre\File;
16+
use OCP\AppFramework\Http;
17+
use OCP\Files\FileInfo;
18+
use OCP\Files\ForbiddenException;
19+
use PHPUnit\Framework\Attributes\DataProvider;
20+
use PHPUnit\Framework\MockObject\MockObject;
21+
use Sabre\DAV\Exception;
22+
use Sabre\DAV\Exception\BadRequest;
23+
use Sabre\DAV\Server;
24+
use Sabre\DAV\Tree;
25+
use Sabre\HTTP\RequestInterface;
26+
use Sabre\HTTP\ResponseInterface;
27+
use Test\TestCase;
28+
29+
class ChecksumUpdatePluginTest extends TestCase {
30+
private Tree&MockObject $tree;
31+
private RequestInterface&MockObject $request;
32+
private ResponseInterface&MockObject $response;
33+
private ChecksumUpdatePlugin $plugin;
34+
35+
protected function setUp(): void {
36+
parent::setUp();
37+
38+
$this->tree = $this->createMock(Tree::class);
39+
$this->request = $this->createMock(RequestInterface::class);
40+
$this->response = $this->createMock(ResponseInterface::class);
41+
42+
$server = $this->createMock(Server::class);
43+
$server->tree = $this->tree;
44+
45+
$this->plugin = new ChecksumUpdatePlugin();
46+
$this->plugin->initialize($server);
47+
48+
$this->request->method('getPath')->willReturn('files/admin/foobar.txt');
49+
}
50+
51+
private function createNode(bool $readable = true, bool $updateable = true): File&MockObject {
52+
$info = $this->createMock(FileInfo::class);
53+
$info->method('isReadable')->willReturn($readable);
54+
$info->method('isUpdateable')->willReturn($updateable);
55+
56+
$node = $this->createMock(File::class);
57+
$node->method('getFileInfo')->willReturn($info);
58+
59+
return $node;
60+
}
61+
62+
public function testRecalculatesTheChecksum(): void {
63+
$node = $this->createNode();
64+
$node->expects($this->once())
65+
->method('hash')
66+
->with('md5')
67+
->willReturn('d41d8cd98f00b204e9800998ecf8427e');
68+
$node->expects($this->once())
69+
->method('setChecksum')
70+
->with('MD5:d41d8cd98f00b204e9800998ecf8427e');
71+
72+
$this->tree->method('getNodeForPath')->willReturn($node);
73+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn('md5');
74+
75+
$this->response->expects($this->once())
76+
->method('addHeader')
77+
->with('OC-Checksum', 'MD5:d41d8cd98f00b204e9800998ecf8427e');
78+
$this->response->expects($this->once())
79+
->method('setStatus')
80+
->with(Http::STATUS_NO_CONTENT);
81+
82+
$this->assertFalse($this->plugin->httpPatch($this->request, $this->response));
83+
}
84+
85+
public static function dataMissingPermissions(): array {
86+
return [
87+
'not readable' => [false, true],
88+
'not updateable' => [true, false],
89+
'neither' => [false, false],
90+
];
91+
}
92+
93+
#[DataProvider('dataMissingPermissions')]
94+
public function testDeniesWithoutPermissions(bool $readable, bool $updateable): void {
95+
$node = $this->createNode($readable, $updateable);
96+
$node->expects($this->never())->method('hash');
97+
$node->expects($this->never())->method('setChecksum');
98+
99+
$this->tree->method('getNodeForPath')->willReturn($node);
100+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn('md5');
101+
102+
$this->expectException(DAVForbiddenException::class);
103+
$this->plugin->httpPatch($this->request, $this->response);
104+
}
105+
106+
public function testMapsStorageForbiddenExceptionToDav(): void {
107+
$node = $this->createNode();
108+
$node->method('hash')->willThrowException(new ForbiddenException('Access denied', false));
109+
$node->expects($this->never())->method('setChecksum');
110+
111+
$this->tree->method('getNodeForPath')->willReturn($node);
112+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn('md5');
113+
114+
$this->expectException(DAVForbiddenException::class);
115+
$this->plugin->httpPatch($this->request, $this->response);
116+
}
117+
118+
public function testFailedHashDoesNotFallThrough(): void {
119+
$node = $this->createNode();
120+
$node->method('hash')->willReturn(false);
121+
$node->expects($this->never())->method('setChecksum');
122+
123+
$this->tree->method('getNodeForPath')->willReturn($node);
124+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn('md5');
125+
126+
$this->expectException(Exception::class);
127+
$this->plugin->httpPatch($this->request, $this->response);
128+
}
129+
130+
public function testRejectsUnknownHashAlgorithm(): void {
131+
$node = $this->createNode();
132+
$node->expects($this->never())->method('hash');
133+
134+
$this->tree->method('getNodeForPath')->willReturn($node);
135+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn('notahash');
136+
137+
$this->expectException(BadRequest::class);
138+
$this->plugin->httpPatch($this->request, $this->response);
139+
}
140+
141+
public function testIgnoresRequestWithoutHeader(): void {
142+
$node = $this->createNode();
143+
$node->expects($this->never())->method('hash');
144+
145+
$this->tree->method('getNodeForPath')->willReturn($node);
146+
$this->request->method('getHeader')->with('X-Recalculate-Hash')->willReturn(null);
147+
148+
$this->assertNull($this->plugin->httpPatch($this->request, $this->response));
149+
}
150+
151+
public function testIgnoresNonFileNode(): void {
152+
$this->tree->method('getNodeForPath')->willReturn($this->createMock(Directory::class));
153+
$this->request->method('getHeader')->willReturn('md5');
154+
155+
$this->assertNull($this->plugin->httpPatch($this->request, $this->response));
156+
}
157+
}

0 commit comments

Comments
 (0)