Skip to content

Commit 8f41ec0

Browse files
nickvergessenbackportbot[bot]
nickvergessen
authored and
backportbot[bot]
committed
fix(dav): Do not claim no plugin available when checksum algorithm is not supported
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Joas Schilling <coding@schilljs.com>
1 parent 381f2fd commit 8f41ec0

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;
@@ -31,25 +35,54 @@ public function getFeatures(): array {
3135
return ['nextcloud-checksum-update'];
3236
}
3337

38+
/**
39+
* @throws BadRequest if the requested hash algorithm is not supported
40+
* @throws DAVForbiddenException if the user may not read and update the node
41+
* @throws Exception if the checksum could not be calculated
42+
*/
3443
public function httpPatch(RequestInterface $request, ResponseInterface $response) {
3544
$path = $request->getPath();
3645

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

71+
try {
4372
$hash = $node->hash($type);
44-
if ($hash) {
45-
$checksum = strtoupper($type) . ':' . $hash;
46-
$node->setChecksum($checksum);
47-
$response->addHeader('OC-Checksum', $checksum);
48-
$response->setHeader('Content-Length', '0');
49-
$response->setStatus(Http::STATUS_NO_CONTENT);
50-
51-
return false;
73+
if ($hash === false) {
74+
throw new Exception('Could not calculate the ' . $type . ' checksum of "' . $path . '"');
5275
}
76+
77+
$checksum = strtoupper($type) . ':' . $hash;
78+
$node->setChecksum($checksum);
79+
80+
$response->addHeader('OC-Checksum', $checksum);
81+
$response->setHeader('Content-Length', '0');
82+
$response->setStatus(Http::STATUS_NO_CONTENT);
83+
return false;
84+
} catch (ForbiddenException $e) {
85+
throw new DAVForbiddenException($e->getMessage(), $e->getRetry(), $e);
5386
}
5487
}
5588
}

‎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)