Skip to content

Commit 5d9dd80

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 a2eb38f commit 5d9dd80

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

0 commit comments

Comments
 (0)