Skip to content

Commit 583e0b2

Browse files
miaulalalabackportbot[bot]
authored andcommitted
fix(appstore): catch GenericFileException when reading cache file in Fetcher
When the appstore cache file exists but getContent() throws a GenericFileException (I/O error or OS-level permission failure), explicitly delete the file and recreate it before writing fresh data — mirroring the NotFoundException recovery path. If deletion itself fails, return [] cleanly. Previously, the unhandled exception caused the entire apps settings page to crash. The new test covers both the recovery path and deletion failure. Signed-off-by: Anna Larch <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent eec0f10 commit 583e0b2

2 files changed

Lines changed: 94 additions & 1 deletion

File tree

lib/private/App/AppStore/Fetcher/Fetcher.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OC\Files\AppData\Factory;
1313
use OCP\AppFramework\Http;
1414
use OCP\AppFramework\Utility\ITimeFactory;
15+
use OCP\Files\GenericFileException;
1516
use OCP\Files\IAppData;
1617
use OCP\Files\NotFoundException;
1718
use OCP\Http\Client\IClientService;
@@ -161,7 +162,15 @@ public function get($allowUnstable = false) {
161162
}
162163
}
163164
} catch (NotFoundException $e) {
164-
// File does not already exists
165+
// File does not already exist
166+
$file = $rootFolder->newFile($this->fileName);
167+
} catch (GenericFileException $e) {
168+
$this->logger->warning('Could not read appstore cache file, it will be refreshed', ['app' => 'appstoreFetcher', 'exception' => $e]);
169+
try {
170+
$file->delete();
171+
} catch (\Exception $deleteException) {
172+
return [];
173+
}
165174
$file = $rootFolder->newFile($this->fileName);
166175
}
167176

tests/lib/App/AppStore/Fetcher/FetcherBase.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OC\Files\AppData\AppData;
1414
use OC\Files\AppData\Factory;
1515
use OCP\AppFramework\Utility\ITimeFactory;
16+
use OCP\Files\GenericFileException;
1617
use OCP\Files\IAppData;
1718
use OCP\Files\NotFoundException;
1819
use OCP\Files\SimpleFS\ISimpleFile;
@@ -683,4 +684,87 @@ public function testFetchAfterUpgradeNoETag(): void {
683684
];
684685
$this->assertSame($expected, $this->fetcher->get());
685686
}
687+
688+
public function testGetWithUnreadableCacheFileRecreatesAndFetches(): void {
689+
$this->config
690+
->method('getSystemValueString')
691+
->willReturnCallback(function ($var, $default) {
692+
if ($var === 'appstoreurl') {
693+
return 'https://apps.nextcloud.com/api/v1';
694+
} elseif ($var === 'version') {
695+
return '11.0.0.2';
696+
}
697+
return $default;
698+
});
699+
$this->config->method('getSystemValueBool')
700+
->willReturnArgument(1);
701+
702+
$folder = $this->createMock(ISimpleFolder::class);
703+
$corruptedFile = $this->createMock(ISimpleFile::class);
704+
$freshFile = $this->createMock(ISimpleFile::class);
705+
$this->appData
706+
->expects($this->once())
707+
->method('getFolder')
708+
->with('/')
709+
->willReturn($folder);
710+
$folder
711+
->expects($this->once())
712+
->method('getFile')
713+
->with($this->fileName)
714+
->willReturn($corruptedFile);
715+
$corruptedFile
716+
->expects($this->once())
717+
->method('getContent')
718+
->willThrowException(new GenericFileException());
719+
$corruptedFile
720+
->expects($this->once())
721+
->method('delete');
722+
$folder
723+
->expects($this->once())
724+
->method('newFile')
725+
->with($this->fileName)
726+
->willReturn($freshFile);
727+
$client = $this->createMock(IClient::class);
728+
$this->clientService
729+
->expects($this->once())
730+
->method('newClient')
731+
->willReturn($client);
732+
$response = $this->createMock(IResponse::class);
733+
$client
734+
->expects($this->once())
735+
->method('get')
736+
->with($this->endpoint)
737+
->willReturn($response);
738+
$response
739+
->expects($this->once())
740+
->method('getBody')
741+
->willReturn('[{"id":"MyNewApp", "foo": "foo"}, {"id":"bar"}]');
742+
$response->method('getHeader')
743+
->with($this->equalTo('ETag'))
744+
->willReturn('"myETag"');
745+
$fileData = '{"data":[{"id":"MyNewApp","foo":"foo"},{"id":"bar"}],"timestamp":1502,"ncversion":"11.0.0.2","ETag":"\"myETag\""}';
746+
$freshFile
747+
->expects($this->once())
748+
->method('putContent')
749+
->with($fileData);
750+
$freshFile
751+
->expects($this->once())
752+
->method('getContent')
753+
->willReturn($fileData);
754+
$this->timeFactory
755+
->expects($this->once())
756+
->method('getTime')
757+
->willReturn(1502);
758+
759+
$expected = [
760+
[
761+
'id' => 'MyNewApp',
762+
'foo' => 'foo',
763+
],
764+
[
765+
'id' => 'bar',
766+
],
767+
];
768+
$this->assertSame($expected, $this->fetcher->get());
769+
}
686770
}

0 commit comments

Comments
 (0)