Skip to content

Commit 3d3a987

Browse files
nickvergessennickvergessen
committed
fix(manifest): Check if app exists instead of accessing null as an array
Signed-off-by: Joas Schilling <coding@schilljs.com>
1 parent ad12af8 commit 3d3a987

6 files changed

Lines changed: 37 additions & 16 deletions

File tree

‎apps/theming/lib/Controller/IconController.php‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,12 @@
3232
use OCA\Theming\IconBuilder;
3333
use OCA\Theming\ImageManager;
3434
use OCA\Theming\ThemingDefaults;
35+
use OCP\App\IAppManager;
3536
use OCP\AppFramework\Controller;
3637
use OCP\AppFramework\Http;
3738
use OCP\AppFramework\Http\DataDisplayResponse;
3839
use OCP\AppFramework\Http\FileDisplayResponse;
40+
use OCP\AppFramework\Http\JSONResponse;
3941
use OCP\AppFramework\Http\NotFoundResponse;
4042
use OCP\AppFramework\Http\Response;
4143
use OCP\Files\NotFoundException;
@@ -50,31 +52,25 @@ class IconController extends Controller {
5052
private $imageManager;
5153
/** @var FileAccessHelper */
5254
private $fileAccessHelper;
55+
/** @var IAppManager */
56+
private $appManager;
5357

54-
/**
55-
* IconController constructor.
56-
*
57-
* @param string $appName
58-
* @param IRequest $request
59-
* @param ThemingDefaults $themingDefaults
60-
* @param IconBuilder $iconBuilder
61-
* @param ImageManager $imageManager
62-
* @param FileAccessHelper $fileAccessHelper
63-
*/
6458
public function __construct(
6559
$appName,
6660
IRequest $request,
6761
ThemingDefaults $themingDefaults,
6862
IconBuilder $iconBuilder,
6963
ImageManager $imageManager,
70-
FileAccessHelper $fileAccessHelper
64+
FileAccessHelper $fileAccessHelper,
65+
IAppManager $appManager
7166
) {
7267
parent::__construct($appName, $request);
7368

7469
$this->themingDefaults = $themingDefaults;
7570
$this->iconBuilder = $iconBuilder;
7671
$this->imageManager = $imageManager;
7772
$this->fileAccessHelper = $fileAccessHelper;
73+
$this->appManager = $appManager;
7874
}
7975

8076
/**
@@ -92,6 +88,11 @@ public function __construct(
9288
* 404: Themed icon not found
9389
*/
9490
public function getThemedIcon(string $app, string $image): Response {
91+
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
92+
$app = 'core';
93+
$image = 'favicon.png';
94+
}
95+
9596
$color = $this->themingDefaults->getColorPrimary();
9697
try {
9798
$iconFileName = $this->imageManager->getCachedImage('icon-' . $app . '-' . $color . str_replace('/', '_', $image));
@@ -121,6 +122,10 @@ public function getThemedIcon(string $app, string $image): Response {
121122
* 404: Favicon not found
122123
*/
123124
public function getFavicon(string $app = 'core'): Response {
125+
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
126+
$app = 'core';
127+
}
128+
124129
$response = null;
125130
$iconFile = null;
126131
try {
@@ -163,6 +168,10 @@ public function getFavicon(string $app = 'core'): Response {
163168
* 404: Touch icon not found
164169
*/
165170
public function getTouchIcon(string $app = 'core'): Response {
171+
if ($app !== 'core' && !$this->appManager->isEnabledForUser($app)) {
172+
$app = 'core';
173+
}
174+
166175
$response = null;
167176
try {
168177
$iconFile = $this->imageManager->getImage('favicon');

‎apps/theming/lib/Controller/ThemingController.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
445445
/**
446446
* @NoCSRFRequired
447447
* @PublicPage
448+
* @BruteForceProtection(action=manifest)
448449
*
449450
* Get the manifest for an app
450451
*
@@ -454,14 +455,20 @@ public function getThemeStylesheet(string $themeId, bool $plain = false, bool $w
454455
*
455456
* 200: Manifest returned
456457
*/
457-
public function getManifest(string $app) {
458+
public function getManifest(string $app): JSONResponse {
458459
$cacheBusterValue = $this->config->getAppValue('theming', 'cachebuster', '0');
459460
if ($app === 'core' || $app === 'settings') {
460461
$name = $this->themingDefaults->getName();
461462
$shortName = $this->themingDefaults->getName();
462463
$startUrl = $this->urlGenerator->getBaseUrl();
463464
$description = $this->themingDefaults->getSlogan();
464465
} else {
466+
if (!$this->appManager->isEnabledForUser($app)) {
467+
$response = new JSONResponse([], Http::STATUS_NOT_FOUND);
468+
$response->throttle(['action' => 'manifest', 'app' => $app]);
469+
return $response;
470+
}
471+
465472
$info = $this->appManager->getAppInfo($app, false, $this->l10n->getLanguageCode());
466473
$name = $info['name'] . ' - ' . $this->themingDefaults->getName();
467474
$shortName = $info['name'];

‎apps/theming/tests/Controller/IconControllerTest.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use OCA\Theming\IconBuilder;
3434
use OCA\Theming\ImageManager;
3535
use OCA\Theming\ThemingDefaults;
36+
use OCP\App\IAppManager;
3637
use OCP\AppFramework\Http;
3738
use OCP\AppFramework\Http\DataDisplayResponse;
3839
use OCP\AppFramework\Http\FileDisplayResponse;
@@ -57,6 +58,8 @@ class IconControllerTest extends TestCase {
5758
private $iconBuilder;
5859
/** @var FileAccessHelper|\PHPUnit\Framework\MockObject\MockObject */
5960
private $fileAccessHelper;
61+
/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
62+
private $appManager;
6063
/** @var ImageManager */
6164
private $imageManager;
6265

@@ -66,6 +69,7 @@ protected function setUp(): void {
6669
$this->iconBuilder = $this->createMock(IconBuilder::class);
6770
$this->imageManager = $this->createMock(ImageManager::class);
6871
$this->fileAccessHelper = $this->createMock(FileAccessHelper::class);
72+
$this->appManager = $this->createMock(IAppManager::class);
6973

7074
$this->timeFactory = $this->createMock(ITimeFactory::class);
7175
$this->timeFactory->expects($this->any())
@@ -80,7 +84,8 @@ protected function setUp(): void {
8084
$this->themingDefaults,
8185
$this->iconBuilder,
8286
$this->imageManager,
83-
$this->fileAccessHelper
87+
$this->fileAccessHelper,
88+
$this->appManager,
8489
);
8590

8691
parent::setUp();

‎core/templates/layout.guest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
<link rel="icon" href="<?php print_unescaped(image_path('core', 'favicon.ico')); /* IE11+ supports png */ ?>">
2121
<link rel="apple-touch-icon" href="<?php print_unescaped(image_path('core', 'favicon-touch.png')); ?>">
2222
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path('core', 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
23-
<link rel="manifest" href="<?php print_unescaped(image_path('core', 'manifest.json')); ?>">
23+
<link rel="manifest" href="<?php print_unescaped(image_path('core', 'manifest.json')); ?>" crossorigin="use-credentials">
2424
<?php emit_css_loading_tags($_); ?>
2525
<?php emit_script_loading_tags($_); ?>
2626
<?php print_unescaped($_['headers']); ?>

‎core/templates/layout.public.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
<link rel="apple-touch-icon" href="<?php print_unescaped(image_path($_['appid'], 'favicon-touch.png')); ?>">
2222
<link rel="apple-touch-icon-precomposed" href="<?php print_unescaped(image_path($_['appid'], 'favicon-touch.png')); ?>">
2323
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path($_['appid'], 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
24-
<link rel="manifest" href="<?php print_unescaped(image_path($_['appid'], 'manifest.json')); ?>">
24+
<link rel="manifest" href="<?php print_unescaped(image_path($_['appid'], 'manifest.json')); ?>" crossorigin="use-credentials">
2525
<?php emit_css_loading_tags($_); ?>
2626
<?php emit_script_loading_tags($_); ?>
2727
<?php print_unescaped($_['headers']); ?>

‎core/templates/layout.user.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
<link rel="apple-touch-icon" href="<?php print_unescaped(image_path($_['appid'], 'favicon-touch.png')); ?>">
3838
<link rel="apple-touch-icon-precomposed" href="<?php print_unescaped(image_path($_['appid'], 'favicon-touch.png')); ?>">
3939
<link rel="mask-icon" sizes="any" href="<?php print_unescaped(image_path($_['appid'], 'favicon-mask.svg')); ?>" color="<?php p($theme->getColorPrimary()); ?>">
40-
<link rel="manifest" href="<?php print_unescaped(image_path($_['appid'], 'manifest.json')); ?>">
40+
<link rel="manifest" href="<?php print_unescaped(image_path($_['appid'], 'manifest.json')); ?>" crossorigin="use-credentials">
4141
<?php emit_css_loading_tags($_); ?>
4242
<?php emit_script_loading_tags($_); ?>
4343
<?php print_unescaped($_['headers']); ?>

0 commit comments

Comments
 (0)