Skip to content

Commit fbdef2a

Browse files
committed
feat: improve ZipFolderPlugin behaviour for different cases
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
1 parent 36577b1 commit fbdef2a

15 files changed

Lines changed: 974 additions & 86 deletions

File tree

apps/dav/composer/composer/autoload_classmap.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@
215215
'OCA\\DAV\\Connector\\Sabre\\Auth' => $baseDir . '/../lib/Connector/Sabre/Auth.php',
216216
'OCA\\DAV\\Connector\\Sabre\\BearerAuth' => $baseDir . '/../lib/Connector/Sabre/BearerAuth.php',
217217
'OCA\\DAV\\Connector\\Sabre\\BlockLegacyClientPlugin' => $baseDir . '/../lib/Connector/Sabre/BlockLegacyClientPlugin.php',
218+
'OCA\\DAV\\Connector\\Sabre\\ByteCounterFilter' => $baseDir . '/../lib/Connector/Sabre/ByteCounterFilter.php',
218219
'OCA\\DAV\\Connector\\Sabre\\CachingTree' => $baseDir . '/../lib/Connector/Sabre/CachingTree.php',
219220
'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => $baseDir . '/../lib/Connector/Sabre/ChecksumList.php',
220221
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => $baseDir . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',
@@ -253,6 +254,7 @@
253254
'OCA\\DAV\\Connector\\Sabre\\ShareTypeList' => $baseDir . '/../lib/Connector/Sabre/ShareTypeList.php',
254255
'OCA\\DAV\\Connector\\Sabre\\ShareeList' => $baseDir . '/../lib/Connector/Sabre/ShareeList.php',
255256
'OCA\\DAV\\Connector\\Sabre\\SharesPlugin' => $baseDir . '/../lib/Connector/Sabre/SharesPlugin.php',
257+
'OCA\\DAV\\Connector\\Sabre\\StreamByteCounter' => $baseDir . '/../lib/Connector/Sabre/StreamByteCounter.php',
256258
'OCA\\DAV\\Connector\\Sabre\\TagList' => $baseDir . '/../lib/Connector/Sabre/TagList.php',
257259
'OCA\\DAV\\Connector\\Sabre\\TagsPlugin' => $baseDir . '/../lib/Connector/Sabre/TagsPlugin.php',
258260
'OCA\\DAV\\Connector\\Sabre\\UserIdHeaderPlugin' => $baseDir . '/../lib/Connector/Sabre/UserIdHeaderPlugin.php',

apps/dav/composer/composer/autoload_static.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ class ComposerStaticInitDAV
230230
'OCA\\DAV\\Connector\\Sabre\\Auth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Auth.php',
231231
'OCA\\DAV\\Connector\\Sabre\\BearerAuth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/BearerAuth.php',
232232
'OCA\\DAV\\Connector\\Sabre\\BlockLegacyClientPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/BlockLegacyClientPlugin.php',
233+
'OCA\\DAV\\Connector\\Sabre\\ByteCounterFilter' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ByteCounterFilter.php',
233234
'OCA\\DAV\\Connector\\Sabre\\CachingTree' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CachingTree.php',
234235
'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumList.php',
235236
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',
@@ -268,6 +269,7 @@ class ComposerStaticInitDAV
268269
'OCA\\DAV\\Connector\\Sabre\\ShareTypeList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ShareTypeList.php',
269270
'OCA\\DAV\\Connector\\Sabre\\ShareeList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ShareeList.php',
270271
'OCA\\DAV\\Connector\\Sabre\\SharesPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/SharesPlugin.php',
272+
'OCA\\DAV\\Connector\\Sabre\\StreamByteCounter' => __DIR__ . '/..' . '/../lib/Connector/Sabre/StreamByteCounter.php',
271273
'OCA\\DAV\\Connector\\Sabre\\TagList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/TagList.php',
272274
'OCA\\DAV\\Connector\\Sabre\\TagsPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/TagsPlugin.php',
273275
'OCA\\DAV\\Connector\\Sabre\\UserIdHeaderPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/UserIdHeaderPlugin.php',
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
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+
namespace OCA\DAV\Connector\Sabre;
10+
11+
/**
12+
* A stream filter to track how many bytes have been streamed from a stream.
13+
*/
14+
class ByteCounterFilter extends \php_user_filter {
15+
public string $filtername = 'ByteCounter';
16+
17+
public function filter($in, $out, &$consumed, bool $closing): int {
18+
$counter = $this->params['counter'] ?? null;
19+
20+
while ($bucket = stream_bucket_make_writeable($in)) {
21+
$length = $bucket->datalen;
22+
$consumed += $length;
23+
if ($counter instanceof StreamByteCounter) {
24+
$counter->bytes += $length;
25+
}
26+
stream_bucket_append($out, $bucket);
27+
}
28+
29+
return PSFS_PASS_ON;
30+
}
31+
}

apps/dav/lib/Connector/Sabre/ServerFactory.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ public function createServer(
110110
$this->logger,
111111
$this->eventDispatcher,
112112
\OCP\Server::get(IDateTimeZone::class),
113+
$this->config,
114+
$this->l10n,
113115
));
114116

115117
// Some WebDAV clients do require Class 2 WebDAV support (locking), since
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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+
namespace OCA\DAV\Connector\Sabre;
10+
11+
/**
12+
* Class to use in combination with ByteCounterFilter to keep track of how much
13+
* has been read from a stream.
14+
*
15+
* @see ByteCounterFilter
16+
*/
17+
class StreamByteCounter {
18+
public float|int $bytes = 0;
19+
}

apps/dav/lib/Connector/Sabre/ZipFolderPlugin.php

Lines changed: 132 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
use OCP\Files\File as NcFile;
1616
use OCP\Files\Folder as NcFolder;
1717
use OCP\Files\Node as NcNode;
18+
use OCP\IConfig;
1819
use OCP\IDateTimeZone;
20+
use OCP\IL10N;
21+
use OCP\L10N\IFactory;
1922
use Psr\Log\LoggerInterface;
2023
use Sabre\DAV\Server;
2124
use Sabre\DAV\ServerPlugin;
@@ -37,13 +40,22 @@ class ZipFolderPlugin extends ServerPlugin {
3740
* Reference to main server object
3841
*/
3942
private ?Server $server = null;
43+
private bool $reportMissingFiles;
44+
private array $missingInfo = [];
4045

4146
public function __construct(
4247
private Tree $tree,
4348
private LoggerInterface $logger,
4449
private IEventDispatcher $eventDispatcher,
4550
private IDateTimeZone $timezoneFactory,
51+
private IConfig $config,
52+
private IL10N $l10n,
4653
) {
54+
$this->reportMissingFiles = $this->config->getSystemValueBool('archive_report_missing_files', false);
55+
56+
if ($this->reportMissingFiles) {
57+
stream_filter_register('count.bytes', ByteCounterFilter::class);
58+
}
4759
}
4860

4961
/**
@@ -62,46 +74,79 @@ public function initialize(Server $server): void {
6274
}
6375

6476
/**
65-
* @return iterable<NcNode>
77+
* Adding a node to the archive streamer.
78+
* @return ?string an error message if an error occurred and reporting is enabled, null otherwise
6679
*/
67-
protected function createIterator(array $rootNodes): iterable {
68-
foreach ($rootNodes as $rootNode) {
69-
yield from $this->iterateNodes($rootNode);
80+
protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath): ?string {
81+
// Remove the root path from the filename to make it relative to the requested folder
82+
$filename = str_replace($rootPath, '', $node->getPath());
83+
84+
$mtime = $node->getMTime();
85+
if ($node instanceof NcFolder) {
86+
$streamer->addEmptyDir($filename, $mtime);
87+
return null;
7088
}
71-
}
7289

73-
/**
74-
* Recursively iterate over all nodes in a folder.
75-
* @return iterable<NcNode>
76-
*/
77-
protected function iterateNodes(NcNode $node): iterable {
78-
yield $node;
90+
if ($node instanceof NcFile) {
91+
$nodeSize = $node->getSize();
92+
try {
93+
$stream = $node->fopen('rb');
94+
} catch (\Exception $e) {
95+
// opening failed, log the failure as reason for the missing file
96+
if ($this->reportMissingFiles) {
97+
$exceptionClass = get_class($e);
98+
return $this->l10n->t('Error while opening the file: %s', [$exceptionClass]);
99+
}
79100

80-
if ($node instanceof NcFolder) {
81-
foreach ($node->getDirectoryListing() as $childNode) {
82-
yield from $this->iterateNodes($childNode);
101+
throw $e;
83102
}
103+
104+
if ($this->reportMissingFiles) {
105+
if ($stream === false) {
106+
return $this->l10n->t('File could not be opened (fopen). Please check the server logs for more information.');
107+
}
108+
109+
$byteCounter = new StreamByteCounter();
110+
$wrapped = stream_filter_append($stream, 'count.bytes', STREAM_FILTER_READ, ['counter' => $byteCounter]);
111+
if ($wrapped === false) {
112+
return $this->l10n->t('Unable to check file for consistency check');
113+
}
114+
}
115+
116+
$fileAddedToStream = $streamer->addFileFromStream($stream, $filename, $nodeSize, $mtime);
117+
if ($this->reportMissingFiles) {
118+
if (!$fileAddedToStream) {
119+
return $this->l10n->t('The archive was already finalized');
120+
}
121+
122+
return $this->logStreamErrors($stream, $filename, $nodeSize, $byteCounter->bytes);
123+
}
124+
84125
}
126+
127+
return null;
85128
}
86129

87130
/**
88-
* Adding a node to the archive streamer.
131+
* Checks whether $stream was fully streamed or if there were other issues
132+
* with the stream, logging the error if necessary.
133+
*
89134
*/
90-
protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath): void {
91-
// Remove the root path from the filename to make it relative to the requested folder
92-
$filename = str_replace($rootPath, '', $node->getPath());
135+
private function logStreamErrors(mixed $stream, string $path, float|int $expectedFileSize, float|int $readFileSize): ?string {
136+
$streamMetadata = stream_get_meta_data($stream);
137+
if (!is_resource($stream) || get_resource_type($stream) !== 'stream') {
138+
return $this->l10n->t('Resource is not a stream or is closed.');
139+
}
93140

94-
$mtime = $node->getMTime();
95-
if ($node instanceof NcFile) {
96-
$resource = $node->fopen('rb');
97-
if ($resource === false) {
98-
$this->logger->info('Cannot read file for zip stream', ['filePath' => $node->getPath()]);
99-
throw new \Sabre\DAV\Exception\ServiceUnavailable('Requested file can currently not be accessed.');
100-
}
101-
$streamer->addFileFromStream($resource, $filename, $node->getSize(), $mtime);
102-
} elseif ($node instanceof NcFolder) {
103-
$streamer->addEmptyDir($filename, $mtime);
141+
if ($streamMetadata['timed_out'] ?? false) {
142+
return $this->l10n->t('Timeout while reading from stream.');
104143
}
144+
145+
if (!($streamMetadata['eof'] ?? true) || $readFileSize != $expectedFileSize) {
146+
return $this->l10n->t('Read %d out of %d bytes from storage. This means the connection may have been closed due to a network/storage error.', [$expectedFileSize, $readFileSize]);
147+
}
148+
149+
return null;
105150
}
106151

107152
/**
@@ -155,14 +200,7 @@ public function handleDownload(Request $request, Response $response): ?bool {
155200
}
156201

157202
$folder = $node->getNode();
158-
$rootNodes = empty($files) ? $folder->getDirectoryListing() : [];
159-
foreach ($files as $path) {
160-
$child = $node->getChild($path);
161-
assert($child instanceof Node);
162-
$rootNodes[] = $child->getNode();
163-
}
164-
165-
$event = new BeforeZipCreatedEvent($folder, $files, $this->createIterator($rootNodes));
203+
$event = new BeforeZipCreatedEvent($folder, $files, $this->reportMissingFiles);
166204
$this->eventDispatcher->dispatchTyped($event);
167205
if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) {
168206
$errorMessage = $event->getErrorMessage();
@@ -175,6 +213,17 @@ public function handleDownload(Request $request, Response $response): ?bool {
175213
throw new Forbidden($errorMessage);
176214
}
177215

216+
// At this point either the event handlers did not block the download
217+
// or they support the new mechanism that filters out nodes that are not
218+
// downloadable, in either case we can use the new API to set the iterator
219+
$content = empty($files) ? $folder->getDirectoryListing() : [];
220+
foreach ($files as $path) {
221+
$child = $node->getChild($path);
222+
assert($child instanceof Node);
223+
$content[] = $child->getNode();
224+
}
225+
$event->setNodesIterable($this->getIterableFromNodes($content));
226+
178227
$archiveName = $folder->getName();
179228
if (count(explode('/', trim($folder->getPath(), '/'), 3)) === 2) {
180229
// this is a download of the root folder
@@ -187,31 +236,71 @@ public function handleDownload(Request $request, Response $response): ?bool {
187236
$rootPath = dirname($folder->getPath());
188237
}
189238

190-
$streamer = new Streamer($tarRequest, -1, count($rootNodes), $this->timezoneFactory);
239+
// numberOfFiles is irrelevant as size=-1 forces the use of zip64 already
240+
$streamer = new Streamer($tarRequest, -1, 0, $this->timezoneFactory);
191241
$streamer->sendHeaders($archiveName);
192242
// For full folder downloads we also add the folder itself to the archive
193243
if (empty($files)) {
194244
$streamer->addEmptyDir($archiveName);
195245
}
196-
foreach ($event->getNodes() as $node) {
197-
$this->streamNode($streamer, $node, $rootPath);
246+
247+
foreach ($event->getNodes() as $path => [$node, $reason]) {
248+
$filename = str_replace($rootPath, '', $path);
249+
if ($node === null) {
250+
if ($this->reportMissingFiles) {
251+
$this->missingInfo[$filename] = $reason;
252+
}
253+
continue;
254+
}
255+
256+
$streamError = $this->streamNode($streamer, $node, $rootPath);
257+
if ($this->reportMissingFiles && $streamError !== null) {
258+
$this->missingInfo[$filename] = $streamError;
259+
}
260+
}
261+
262+
if ($this->reportMissingFiles && !empty($this->missingInfo)) {
263+
$json = json_encode($this->missingInfo, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT);
264+
$stream = fopen('php://temp', 'r+');
265+
fwrite($stream, $json);
266+
rewind($stream);
267+
$streamer->addFileFromStream($stream, 'missing_files.json', (float)strlen($json), false);
198268
}
199269
$streamer->finalize();
200270
return false;
201271
}
202272

203273
/**
204-
* Tell sabre/dav not to trigger it's own response sending logic as the handleDownload will have already send the response
274+
* Given a set of nodes, produces a list of all nodes contained in them
275+
* recursively.
276+
*
277+
* @param NcNode[] $nodes
278+
* @return iterable<NcNode>
279+
*/
280+
private function getIterableFromNodes(array $nodes): iterable {
281+
foreach ($nodes as $node) {
282+
yield $node;
283+
284+
if ($node instanceof NcFolder) {
285+
foreach ($node->getDirectoryListing() as $child) {
286+
yield from $this->getIterableFromNodes([$child]);
287+
}
288+
}
289+
}
290+
}
291+
292+
/**
293+
* Tell sabre/dav not to trigger its own response sending logic as the handleDownload will have already send the response
205294
*
206295
* @return false|null
207296
*/
208297
public function afterDownload(Request $request, Response $response): ?bool {
209298
$node = $this->tree->getNodeForPath($request->getPath());
210-
if (!($node instanceof Directory)) {
299+
if ($node instanceof Directory) {
211300
// only handle directories
212-
return null;
213-
} else {
214301
return false;
215302
}
303+
304+
return null;
216305
}
217306
}

apps/dav/lib/Server.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace OCA\DAV;
99

1010
use OC\Files\Filesystem;
11+
use OC\L10N\L10N;
1112
use OCA\DAV\AppInfo\PluginManager;
1213
use OCA\DAV\BulkUpload\BulkUploadPlugin;
1314
use OCA\DAV\CalDAV\BirthdayCalendar\EnablePlugin;
@@ -87,12 +88,14 @@
8788
use OCP\IDateTimeZone;
8889
use OCP\IDBConnection;
8990
use OCP\IGroupManager;
91+
use OCP\IL10N;
9092
use OCP\IPreview;
9193
use OCP\IRequest;
9294
use OCP\ISession;
9395
use OCP\ITagManager;
9496
use OCP\IURLGenerator;
9597
use OCP\IUserSession;
98+
use OCP\L10N\IFactory;
9699
use OCP\Mail\IEmailValidator;
97100
use OCP\Mail\IMailer;
98101
use OCP\Profiler\IProfiler;
@@ -242,6 +245,7 @@ public function __construct(
242245
\OCP\Server::get(IUserSession::class)
243246
));
244247

248+
$config = \OCP\Server::get(IConfig::class);
245249
// performance improvement plugins
246250
$this->server->addPlugin(new CopyEtagHeaderPlugin());
247251
$this->server->addPlugin(new RequestIdHeaderPlugin(\OCP\Server::get(IRequest::class)));
@@ -254,6 +258,8 @@ public function __construct(
254258
$logger,
255259
$eventDispatcher,
256260
\OCP\Server::get(IDateTimeZone::class),
261+
$config,
262+
\OCP\Server::get(IFactory::class)->get('dav'),
257263
));
258264
$this->server->addPlugin(\OCP\Server::get(PaginatePlugin::class));
259265
$this->server->addPlugin(new PropFindPreloadNotifyPlugin());

0 commit comments

Comments
 (0)