Skip to content

Commit 6e8ee13

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 6e8ee13

15 files changed

Lines changed: 980 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: 134 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,25 @@ class ZipFolderPlugin extends ServerPlugin {
3740
* Reference to main server object
3841
*/
3942
private ?Server $server = null;
43+
private bool $reportMissingFiles;
44+
private array $missingInfo = [];
45+
private IL10N $l10n;
4046

4147
public function __construct(
4248
private Tree $tree,
4349
private LoggerInterface $logger,
4450
private IEventDispatcher $eventDispatcher,
4551
private IDateTimeZone $timezoneFactory,
52+
private IConfig $config,
53+
private IFactory $l10nFactory,
4654
) {
55+
$this->reportMissingFiles = $this->config->getSystemValueBool('archive_report_missing_files', false);
56+
57+
if ($this->reportMissingFiles) {
58+
stream_filter_register('count.bytes', ByteCounterFilter::class);
59+
}
60+
61+
$this->l10n = $this->l10nFactory->get('dav');
4762
}
4863

4964
/**
@@ -62,46 +77,78 @@ public function initialize(Server $server): void {
6277
}
6378

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

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;
93+
if ($node instanceof NcFile) {
94+
$nodeSize = $node->getSize();
95+
try {
96+
$stream = $node->fopen('rb');
97+
} catch (\Exception $e) {
98+
// opening failed, log the failure as reason for the missing file
99+
if ($this->reportMissingFiles) {
100+
$exceptionClass = get_class($e);
101+
return $this->l10n->t('Error while opening the file: %s', [$exceptionClass]);
102+
}
79103

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

87132
/**
88-
* Adding a node to the archive streamer.
133+
* Checks whether $stream was fully streamed or if there were other issues
134+
* with the stream, logging the error if necessary.
135+
*
89136
*/
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());
137+
private function logStreamErrors(mixed $stream, string $path, float|int $expectedFileSize, float|int $readFileSize): ?string {
138+
$streamMetadata = stream_get_meta_data($stream);
139+
if (!is_resource($stream) || get_resource_type($stream) !== 'stream') {
140+
return $this->l10n->t('Resource is not a stream or is closed.');
141+
}
93142

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);
143+
if ($streamMetadata['timed_out'] ?? false) {
144+
return $this->l10n->t('Timeout while reading from stream.');
145+
}
146+
147+
if (!($streamMetadata['eof'] ?? true) || $readFileSize != $expectedFileSize) {
148+
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]);
104149
}
150+
151+
return null;
105152
}
106153

107154
/**
@@ -155,14 +202,7 @@ public function handleDownload(Request $request, Response $response): ?bool {
155202
}
156203

157204
$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));
205+
$event = new BeforeZipCreatedEvent($folder, $files, $this->reportMissingFiles);
166206
$this->eventDispatcher->dispatchTyped($event);
167207
if ((!$event->isSuccessful()) || $event->getErrorMessage() !== null) {
168208
$errorMessage = $event->getErrorMessage();
@@ -175,6 +215,17 @@ public function handleDownload(Request $request, Response $response): ?bool {
175215
throw new Forbidden($errorMessage);
176216
}
177217

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

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

203275
/**
204-
* Tell sabre/dav not to trigger it's own response sending logic as the handleDownload will have already send the response
276+
* Given a set of nodes, produces a list of all nodes contained in them
277+
* recursively.
278+
*
279+
* @param NcNode[] $nodes
280+
* @return iterable<NcNode>
281+
*/
282+
private function getIterableFromNodes(array $nodes): iterable {
283+
foreach ($nodes as $node) {
284+
yield $node;
285+
286+
if ($node instanceof NcFolder) {
287+
foreach ($node->getDirectoryListing() as $child) {
288+
yield from $this->getIterableFromNodes([$child]);
289+
}
290+
}
291+
}
292+
}
293+
294+
/**
295+
* Tell sabre/dav not to trigger its own response sending logic as the handleDownload will have already send the response
205296
*
206297
* @return false|null
207298
*/
208299
public function afterDownload(Request $request, Response $response): ?bool {
209300
$node = $this->tree->getNodeForPath($request->getPath());
210-
if (!($node instanceof Directory)) {
301+
if ($node instanceof Directory) {
211302
// only handle directories
212-
return null;
213-
} else {
214303
return false;
215304
}
305+
306+
return null;
216307
}
217308
}

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),
257263
));
258264
$this->server->addPlugin(\OCP\Server::get(PaginatePlugin::class));
259265
$this->server->addPlugin(new PropFindPreloadNotifyPlugin());

0 commit comments

Comments
 (0)