Skip to content

Commit df9c1be

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

5 files changed

Lines changed: 31 additions & 64 deletions

File tree

apps/dav/composer/composer/autoload_classmap.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@
216216
'OCA\\DAV\\Connector\\Sabre\\Auth' => $baseDir . '/../lib/Connector/Sabre/Auth.php',
217217
'OCA\\DAV\\Connector\\Sabre\\BearerAuth' => $baseDir . '/../lib/Connector/Sabre/BearerAuth.php',
218218
'OCA\\DAV\\Connector\\Sabre\\BlockLegacyClientPlugin' => $baseDir . '/../lib/Connector/Sabre/BlockLegacyClientPlugin.php',
219-
'OCA\\DAV\\Connector\\Sabre\\ByteCounterFilter' => $baseDir . '/../lib/Connector/Sabre/ByteCounterFilter.php',
220219
'OCA\\DAV\\Connector\\Sabre\\CachingTree' => $baseDir . '/../lib/Connector/Sabre/CachingTree.php',
221220
'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => $baseDir . '/../lib/Connector/Sabre/ChecksumList.php',
222221
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => $baseDir . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',

apps/dav/composer/composer/autoload_static.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,6 @@ class ComposerStaticInitDAV
231231
'OCA\\DAV\\Connector\\Sabre\\Auth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/Auth.php',
232232
'OCA\\DAV\\Connector\\Sabre\\BearerAuth' => __DIR__ . '/..' . '/../lib/Connector/Sabre/BearerAuth.php',
233233
'OCA\\DAV\\Connector\\Sabre\\BlockLegacyClientPlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/BlockLegacyClientPlugin.php',
234-
'OCA\\DAV\\Connector\\Sabre\\ByteCounterFilter' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ByteCounterFilter.php',
235234
'OCA\\DAV\\Connector\\Sabre\\CachingTree' => __DIR__ . '/..' . '/../lib/Connector/Sabre/CachingTree.php',
236235
'OCA\\DAV\\Connector\\Sabre\\ChecksumList' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumList.php',
237236
'OCA\\DAV\\Connector\\Sabre\\ChecksumUpdatePlugin' => __DIR__ . '/..' . '/../lib/Connector/Sabre/ChecksumUpdatePlugin.php',

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

Lines changed: 0 additions & 32 deletions
This file was deleted.

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

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,19 @@
88
*/
99
namespace OCA\DAV\Connector\Sabre;
1010

11+
use Icewind\Streams\CountWrapper;
1112
use OC\Streamer;
1213
use OCA\DAV\Connector\Sabre\Exception\Forbidden;
1314
use OCP\EventDispatcher\IEventDispatcher;
1415
use OCP\Files\Events\BeforeZipCreatedEvent;
1516
use OCP\Files\File as NcFile;
1617
use OCP\Files\Folder as NcFolder;
1718
use OCP\Files\Node as NcNode;
19+
use OCP\Files\NotPermittedException;
1820
use OCP\IConfig;
1921
use OCP\IDateTimeZone;
2022
use OCP\IL10N;
23+
use OCP\Lock\LockedException;
2124
use Psr\Log\LoggerInterface;
2225
use Sabre\DAV\Server;
2326
use Sabre\DAV\ServerPlugin;
@@ -57,10 +60,6 @@ public function __construct(
5760
private IL10N $l10n,
5861
) {
5962
$this->reportMissingFiles = $this->config->getSystemValueBool('archive_report_missing_files', false);
60-
61-
if ($this->reportMissingFiles) {
62-
stream_filter_register('count.bytes', ByteCounterFilter::class);
63-
}
6463
}
6564

6665
/**
@@ -81,6 +80,7 @@ public function initialize(Server $server): void {
8180
/**
8281
* Adding a node to the archive streamer.
8382
* @return ?string an error message if an error occurred and reporting is enabled, null otherwise
83+
* @throws NotPermittedException|LockedException
8484
*/
8585
protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath): ?string {
8686
// Remove the root path from the filename to make it relative to the requested folder
@@ -94,39 +94,27 @@ protected function streamNode(Streamer $streamer, NcNode $node, string $rootPath
9494

9595
if ($node instanceof NcFile) {
9696
$nodeSize = $node->getSize();
97-
try {
98-
$stream = $node->fopen('rb');
99-
} catch (\Exception $e) {
100-
// opening failed, log the failure as reason for the missing file
101-
if ($this->reportMissingFiles) {
102-
$exceptionClass = get_class($e);
103-
return $this->l10n->t('Error while opening the file: %s', [$exceptionClass]);
104-
}
97+
$stream = $node->fopen('rb');
10598

106-
throw $e;
99+
if ($stream === false) {
100+
return $this->l10n->t('File could not be opened (fopen). Please check the server logs for more information.');
107101
}
108102

109-
if ($this->reportMissingFiles) {
110-
if ($stream === false) {
111-
return $this->l10n->t('File could not be opened (fopen). Please check the server logs for more information.');
112-
}
103+
$read = 0;
104+
$stream = CountWrapper::wrap($stream, function (int $written) use (&$read) {
105+
return $read += $written;
106+
});
113107

114-
$byteCounter = new StreamByteCounter();
115-
$wrapped = stream_filter_append($stream, 'count.bytes', STREAM_FILTER_READ, ['counter' => $byteCounter]);
116-
if ($wrapped === false) {
117-
return $this->l10n->t('Unable to check file for consistency check');
118-
}
108+
if ($stream === false) {
109+
return $this->l10n->t('Unable to check file for consistency check');
119110
}
120111

121112
$fileAddedToStream = $streamer->addFileFromStream($stream, $filename, $nodeSize, $mtime);
122-
if ($this->reportMissingFiles) {
123-
if (!$fileAddedToStream) {
124-
return $this->l10n->t('The archive was already finalized');
125-
}
126-
127-
return $this->logStreamErrors($stream, $filename, $nodeSize, $byteCounter->bytes);
113+
if (!$fileAddedToStream) {
114+
return $this->l10n->t('The archive was already finalized');
128115
}
129116

117+
return $this->logStreamErrors($stream, $filename, $nodeSize, $read);
130118
}
131119

132120
return null;
@@ -259,7 +247,20 @@ public function handleDownload(Request $request, Response $response): ?false {
259247
continue;
260248
}
261249

262-
$streamError = $this->streamNode($streamer, $node, $rootPath);
250+
try {
251+
$streamError = $this->streamNode($streamer, $node, $rootPath);
252+
} catch (\Exception $e) {
253+
if (!$this->reportMissingFiles) {
254+
throw $e;
255+
}
256+
257+
$logMessage = $this->l10n->t('Error while streaming the file');
258+
$this->logger->error($logMessage, ['exception' => $e]);
259+
$reason = $this->l10n->t('File could not be added to the archive. Please check the server logs for more information.');
260+
$this->missingInfo[$filename] = $reason;
261+
continue;
262+
}
263+
263264
if ($this->reportMissingFiles && $streamError !== null) {
264265
$this->missingInfo[$filename] = $streamError;
265266
}

apps/dav/tests/unit/Connector/Sabre/ZipFolderPluginTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public static function dataDownloadingAFolderWithMissingFilesReportingShouldSucc
153153
'children' => ['allowed.txt' => 'allowed', 'blocked.txt' => 'blocked', 'error.txt' => new \RuntimeException('read error')],
154154
'filesFilter' => [],
155155
'downloadBlocked' => [ 'blocked.txt' ],
156-
'expectedMissingFiles' => [ 'blocked.txt' => 'blocked', 'error.txt' => 'Error while opening the file: RuntimeException' ],
156+
'expectedMissingFiles' => [ 'blocked.txt' => 'blocked', 'error.txt' => 'File could not be added to the archive. Please check the server logs for more information.' ],
157157
],
158158
// files filtered out should not be reported as missing
159159
'filtering some files' => [

0 commit comments

Comments
 (0)