Skip to content

Commit 5113b18

Browse files
committed
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 f16c6c9 commit 5113b18

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;
@@ -167,7 +168,15 @@ public function get($allowUnstable = false): array {
167168
}
168169
}
169170
} catch (NotFoundException $e) {
170-
// File does not already exists
171+
// File does not already exist
172+
$file = $rootFolder->newFile($this->fileName);
173+
} catch (GenericFileException $e) {
174+
$this->logger->warning('Could not read appstore cache file, it will be refreshed', ['app' => 'appstoreFetcher', 'exception' => $e]);
175+
try {
176+
$file->delete();
177+
} catch (\Exception $deleteException) {
178+
return [];
179+
}
171180
$file = $rootFolder->newFile($this->fileName);
172181
}
173182

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

0 commit comments

Comments
 (0)