Skip to content

Commit 5134cca

Browse files
authored
Merge pull request #882 from nextcloud/fix/route-defaults-normalization
fix: normalize missing bruteforce_protection and headers_to_exclude on ExApp routes
2 parents f9445cb + 2e78425 commit 5134cca

5 files changed

Lines changed: 67 additions & 13 deletions

File tree

lib/Controller/ExAppProxyController.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
1616
use OCA\AppAPI\AppInfo\Application;
1717
use OCA\AppAPI\Db\ExApp;
18+
use OCA\AppAPI\Db\ExAppMapper;
1819
use OCA\AppAPI\Db\ExAppRouteAccessLevel;
1920
use OCA\AppAPI\ProxyResponse;
2021
use OCA\AppAPI\Service\AppAPIService;
@@ -261,9 +262,7 @@ private function prepareProxy(
261262
);
262263
return null;
263264
}
264-
$bruteforceProtection = isset($route['bruteforce_protection'])
265-
? json_decode($route['bruteforce_protection'], true)
266-
: [];
265+
$bruteforceProtection = ExAppMapper::parseJsonList($route['bruteforce_protection'] ?? null);
267266
if (!empty($bruteforceProtection)) {
268267
$delay = $this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), Application::APP_ID);
269268
}
@@ -350,8 +349,10 @@ private function passesExAppProxyRouteAccessLevelCheck(int $accessLevel): bool {
350349
}
351350

352351
private function buildHeadersWithExclude(array $route, array $headers): array {
353-
$headersToExclude = json_decode($route['headers_to_exclude'], true);
354-
$headersToExclude = array_map('strtolower', $headersToExclude);
352+
$headersToExclude = array_map(
353+
'strtolower',
354+
array_filter(ExAppMapper::parseJsonList($route['headers_to_exclude'] ?? null), 'is_string')
355+
);
355356

356357
if (!in_array('x-origin-ip', $headersToExclude)) {
357358
$headersToExclude[] = 'x-origin-ip';

lib/Db/ExAppMapper.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,18 @@ public function __construct(IDBConnection $db) {
2525
parent::__construct($db, 'ex_apps');
2626
}
2727

28+
/**
29+
* Decode a JSON-list column (`bruteforce_protection`, `headers_to_exclude`) into an array,
30+
* tolerating NULL / non-string / malformed values from legacy rows.
31+
*/
32+
public static function parseJsonList(mixed $raw): array {
33+
if (!is_string($raw)) {
34+
return [];
35+
}
36+
$decoded = json_decode($raw, true);
37+
return is_array($decoded) ? $decoded : [];
38+
}
39+
2840
/**
2941
* @throws Exception
3042
*

lib/Service/ExAppService.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,8 @@ public function getExApps(): array {
432432
public function registerExAppRoutes(ExApp $exApp, array $routes): ?ExApp {
433433
try {
434434
$this->exAppMapper->registerExAppRoutes($exApp, $routes);
435-
$exApp->setRoutes($routes);
436-
return $exApp;
437-
} catch (Exception $e) {
435+
return $this->exAppMapper->findByAppId($exApp->getAppid());
436+
} catch (Exception|MultipleObjectsReturnedException|DoesNotExistException $e) {
438437
$this->logger->error(sprintf('Error while registering ExApp %s routes: %s. Routes: %s', $exApp->getAppid(), $e->getMessage(), json_encode($routes)));
439438
return null;
440439
}

lib/Service/HarpService.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use GuzzleHttp\Exception\ClientException;
1414
use OCA\AppAPI\Db\DaemonConfig;
1515
use OCA\AppAPI\Db\ExApp;
16+
use OCA\AppAPI\Db\ExAppMapper;
1617
use OCA\AppAPI\DeployActions\ManualActions;
1718
use OCP\ICertificateManager;
1819
use OCP\IConfig;
@@ -116,14 +117,10 @@ public function getHarpExApp(ExApp $exApp): array {
116117
'host' => $this->getExAppHost($exApp),
117118
'port' => $exApp->getPort(),
118119
'routes' => array_map(function ($route) {
119-
$bruteforceList = json_decode($route['bruteforce_protection'], true);
120-
if (!$bruteforceList) {
121-
$bruteforceList = [];
122-
}
123120
return [
124121
'url' => $route['url'],
125122
'access_level' => $route['access_level'],
126-
'bruteforce_protection' => $bruteforceList,
123+
'bruteforce_protection' => ExAppMapper::parseJsonList($route['bruteforce_protection'] ?? null),
127124
];
128125
}, $exApp->getRoutes()),
129126
];

tests/php/Db/ExAppMapperTest.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\AppAPI\Tests\php\Db;
11+
12+
use OCA\AppAPI\Db\ExAppMapper;
13+
use PHPUnit\Framework\Attributes\DataProvider;
14+
use PHPUnit\Framework\TestCase;
15+
16+
class ExAppMapperTest extends TestCase {
17+
18+
/**
19+
* `parseJsonList` is used by the proxy controller and HaRP route serializer to read the
20+
* nullable `bruteforce_protection` / `headers_to_exclude` columns. The matrix below covers
21+
* every row shape we have seen or can construct: legacy NULLs, malformed JSON, non-string
22+
* inputs, and well-formed payloads.
23+
*/
24+
#[DataProvider('jsonListProvider')]
25+
public function testParseJsonList(mixed $raw, array $expected): void {
26+
self::assertSame($expected, ExAppMapper::parseJsonList($raw));
27+
}
28+
29+
public static function jsonListProvider(): array {
30+
return [
31+
'null' => [null, []],
32+
'empty string' => ['', []],
33+
'empty array string' => ['[]', []],
34+
'valid array of ints' => ['[401,403]', [401, 403]],
35+
'valid array of strings' => ['["Cookie","Authorization"]', ['Cookie', 'Authorization']],
36+
'invalid json' => ['{broken', []],
37+
'json literal "null"' => ['null', []],
38+
'json scalar string' => ['"foo"', []],
39+
'json scalar int' => ['42', []],
40+
'non-string input (array)' => [['Cookie'], []],
41+
'non-string input (int)' => [42, []],
42+
'non-string input (bool)' => [false, []],
43+
];
44+
}
45+
}

0 commit comments

Comments
 (0)