diff --git a/lib/Service/ExAppEnvVarsHelper.php b/lib/Service/ExAppEnvVarsHelper.php
new file mode 100644
index 000000000..385acc32d
--- /dev/null
+++ b/lib/Service/ExAppEnvVarsHelper.php
@@ -0,0 +1,72 @@
+` or ``) becomes an empty array instead of an empty string.
+ * Left as-is, such a value passes the "drop variables with an empty value" filter and is later
+ * stringified to the literal `Array` in the container environment.
+ *
+ * The helper produces a canonical NAME => {name, displayName, description, default, value} map with
+ * every field a string, applies caller overrides (occ `--env`, UI deploy options, stored deploy
+ * options on update), and drops variables whose final value is empty.
+ */
+class ExAppEnvVarsHelper {
+ /**
+ * @param array $variables raw `environment-variables.variable` entries: a list, or a single entry as produced by SimpleXML for one `` element
+ * @param array $overrides deploy-option overrides, NAME => value or NAME => ['value' => value]; overrides for undeclared names are ignored
+ * @return array normalized NAME-keyed map, entries with an empty final value removed
+ * @throws InvalidArgumentException on the first malformed variable; message identifies the entry and field
+ */
+ public static function normalizeAndValidate(array $variables, array $overrides): array {
+ if (!array_is_list($variables)) {
+ $variables = [$variables];
+ }
+ $envVars = [];
+ foreach ($variables as $index => $variable) {
+ if (!is_array($variable)) {
+ throw new InvalidArgumentException(sprintf('variable #%d: entry must be an object, got %s', $index, get_debug_type($variable)));
+ }
+ $name = $variable['name'] ?? null;
+ if (!is_string($name) || trim($name) === '') {
+ throw new InvalidArgumentException(sprintf("variable #%d: 'name' must be a non-empty string, got %s", $index, get_debug_type($name)));
+ }
+ $default = self::toString($variable['default'] ?? '');
+ $envVars[$name] = [
+ 'name' => $name,
+ 'displayName' => self::toString($variable['display-name'] ?? ''),
+ 'description' => self::toString($variable['description'] ?? ''),
+ 'default' => $default,
+ 'value' => $default,
+ ];
+ }
+ foreach ($overrides as $name => $value) {
+ if (array_key_exists($name, $envVars)) {
+ $envVars[$name]['value'] = self::toString($value['value'] ?? $value ?? '');
+ }
+ }
+ return array_filter($envVars, static function (array $envVar) {
+ return $envVar['value'] !== '';
+ });
+ }
+
+ /**
+ * An empty XML element arrives as [] after the simplexml/json roundtrip: treat any
+ * non-scalar as an empty string so the empty-value filter applies to every input shape.
+ */
+ private static function toString(mixed $value): string {
+ return is_scalar($value) ? (string)$value : '';
+ }
+}
diff --git a/lib/Service/ExAppService.php b/lib/Service/ExAppService.php
index a0a538241..d39fab8c4 100644
--- a/lib/Service/ExAppService.php
+++ b/lib/Service/ExAppService.php
@@ -261,7 +261,7 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
# fill 'id' if it is missing(this field was called `appid` in previous versions in json)
$appInfo['id'] = $appInfo['id'] ?? $appId;
# during manual install JSON can have all values at root level
- foreach (['docker-install', 'translations_folder', 'routes', 'k8s-service-roles'] as $key) {
+ foreach (['docker-install', 'translations_folder', 'routes', 'k8s-service-roles', 'environment-variables'] as $key) {
if (isset($appInfo[$key])) {
$appInfo['external-app'][$key] = $appInfo[$key];
unset($appInfo[$key]);
@@ -291,34 +291,6 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
$appInfo['external-app']['routes'] = [$appInfo['external-app']['routes']['route']];
}
}
- // Advanced deploy options
- if (isset($appInfo['external-app']['environment-variables']['variable'])) {
- $envVars = [];
- if (!isset($appInfo['external-app']['environment-variables']['variable'][0])) {
- $appInfo['external-app']['environment-variables']['variable'] = [$appInfo['external-app']['environment-variables']['variable']];
- }
- foreach ($appInfo['external-app']['environment-variables']['variable'] as $envVar) {
- $envVars[$envVar['name']] = [
- 'name' => $envVar['name'],
- 'displayName' => $envVar['display-name'] ?? '',
- 'description' => $envVar['description'] ?? '',
- 'default' => $envVar['default'] ?? '',
- 'value' => $envVar['default'] ?? '',
- ];
- }
- if (isset($deployOptions['environment_variables']) && count(array_keys($deployOptions['environment_variables'])) > 0) {
- // override with given deploy options values
- foreach ($deployOptions['environment_variables'] as $key => $value) {
- if (array_key_exists($key, $envVars)) {
- $envVars[$key]['value'] = $value['value'] ?? $value ?? '';
- }
- }
- }
- $envVars = array_filter($envVars, function ($envVar) {
- return $envVar['value'] !== '';
- });
- $appInfo['external-app']['environment-variables'] = $envVars;
- }
if (isset($appInfo['external-app']['k8s-service-roles']['role'])) {
$roles = $appInfo['external-app']['k8s-service-roles']['role'];
if (!isset($roles[0])) {
@@ -344,6 +316,21 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
}
}
}
+ // Advanced deploy options; runs for both the XML and the JSON path so the
+ // environment-variables contract of the returned appInfo is input-format independent
+ if (isset($appInfo['external-app']['environment-variables']['variable'])) {
+ $variables = $appInfo['external-app']['environment-variables']['variable'];
+ if (!is_array($variables)) {
+ return ['error' => sprintf("ExApp '%s' has invalid environment variable definition. 'variable' must be an object or a list of objects, got %s", $appId, get_debug_type($variables))];
+ }
+ try {
+ $appInfo['external-app']['environment-variables'] = ExAppEnvVarsHelper::normalizeAndValidate(
+ $variables, $deployOptions['environment_variables'] ?? []
+ );
+ } catch (InvalidArgumentException $e) {
+ return ['error' => sprintf("ExApp '%s' has invalid environment variable definition. %s", $appId, $e->getMessage())];
+ }
+ }
if (isset($appInfo['external-app']['routes'])) {
if (!is_array($appInfo['external-app']['routes'])) {
return ['error' => sprintf("ExApp '%s' has invalid route definition. 'routes' must be a list of route objects, got %s", $appId, get_debug_type($appInfo['external-app']['routes']))];
diff --git a/tests/php/Service/ExAppEnvVarsHelperTest.php b/tests/php/Service/ExAppEnvVarsHelperTest.php
new file mode 100644
index 000000000..150d1fc18
--- /dev/null
+++ b/tests/php/Service/ExAppEnvVarsHelperTest.php
@@ -0,0 +1,145 @@
+
+ * element must not survive as an empty array and end up as the literal string `Array` in
+ * the container environment. The input is produced by the same simplexml/json roundtrip
+ * getAppInfo uses, so the [] shape is real, not hand-crafted.
+ */
+ #[DataProvider('emptyDefaultXmlProvider')]
+ public function testEmptyDefaultElementFromRealXmlIsDropped(string $xml): void {
+ $parsed = json_decode(json_encode((array)simplexml_load_string($xml)), true);
+ $variables = $parsed['environment-variables']['variable'];
+
+ // lock in the SimpleXML behavior the bug depends on: empty element parses to []
+ self::assertSame([], $variables['default']);
+
+ self::assertSame([], ExAppEnvVarsHelper::normalizeAndValidate($variables, []));
+ }
+
+ public static function emptyDefaultXmlProvider(): array {
+ return [
+ '' => [
+ ''
+ . 'EMPTY_ELEMEmptyd'
+ . '',
+ ],
+ '' => [
+ ''
+ . 'EMPTY_ELEMEmptyd'
+ . '',
+ ],
+ ];
+ }
+
+ #[DataProvider('validVariablesProvider')]
+ public function testNormalizeAndValidate(array $variables, array $overrides, array $expected): void {
+ self::assertSame($expected, ExAppEnvVarsHelper::normalizeAndValidate($variables, $overrides));
+ }
+
+ public static function validVariablesProvider(): array {
+ return [
+ 'single arrives as one object, not a list' => [
+ ['name' => 'A', 'display-name' => 'Var A', 'description' => 'desc', 'default' => 'x'],
+ [],
+ ['A' => ['name' => 'A', 'displayName' => 'Var A', 'description' => 'desc', 'default' => 'x', 'value' => 'x']],
+ ],
+ 'variable without is dropped' => [
+ [['name' => 'A', 'display-name' => 'Var A', 'description' => 'desc']],
+ [],
+ [],
+ ],
+ 'empty-element default ([]) is dropped, sibling with a value survives' => [
+ [
+ ['name' => 'EMPTY_ELEM', 'display-name' => 'Empty', 'description' => 'd', 'default' => []],
+ ['name' => 'KEPT', 'display-name' => 'Kept', 'description' => 'd', 'default' => 'v'],
+ ],
+ [],
+ ['KEPT' => ['name' => 'KEPT', 'displayName' => 'Kept', 'description' => 'd', 'default' => 'v', 'value' => 'v']],
+ ],
+ 'empty-element display-name and description become empty strings' => [
+ [['name' => 'A', 'display-name' => [], 'description' => [], 'default' => 'x']],
+ [],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'x']],
+ ],
+ 'override replaces the default value' => [
+ [['name' => 'A', 'default' => 'x']],
+ ['A' => 'y'],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+ ],
+ 'override with empty value drops the variable' => [
+ [['name' => 'A', 'default' => 'x']],
+ ['A' => ''],
+ [],
+ ],
+ 'override in stored deploy-options shape' => [
+ [['name' => 'A', 'default' => 'x']],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+ ],
+ 'stored pre-fix deploy option with [] value is dropped, not deployed as Array' => [
+ [['name' => 'A', 'default' => []]],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => [], 'value' => []]],
+ [],
+ ],
+ 'override for an undeclared variable is ignored' => [
+ [['name' => 'A', 'default' => 'x']],
+ ['B' => 'y'],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'x']],
+ ],
+ 'no variables declared' => [[], ['A' => 'y'], []],
+ 'numeric override is canonicalized to string' => [
+ [['name' => 'A', 'default' => 'x']],
+ ['A' => 123],
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => '123']],
+ ],
+ ];
+ }
+
+ #[DataProvider('invalidVariablesProvider')]
+ public function testNormalizeAndValidateRejects(array $variables, string $expectedMessageFragment): void {
+ try {
+ ExAppEnvVarsHelper::normalizeAndValidate($variables, []);
+ self::fail('Expected InvalidArgumentException, none thrown');
+ } catch (InvalidArgumentException $e) {
+ self::assertStringContainsString($expectedMessageFragment, $e->getMessage());
+ }
+ }
+
+ public static function invalidVariablesProvider(): array {
+ return [
+ 'entry is not an array' => [
+ ['not-an-object'],
+ 'variable #0: entry must be an object',
+ ],
+ 'missing name' => [
+ [['display-name' => 'X', 'default' => 'v']],
+ "variable #0: 'name' must be a non-empty string",
+ ],
+ 'empty element parses to an array' => [
+ [['name' => [], 'default' => 'v']],
+ "variable #0: 'name' must be a non-empty string",
+ ],
+ 'whitespace-only name' => [
+ [['name' => ' ', 'default' => 'v']],
+ "variable #0: 'name' must be a non-empty string",
+ ],
+ ];
+ }
+}
diff --git a/tests/php/Service/ExAppServiceGetAppInfoTest.php b/tests/php/Service/ExAppServiceGetAppInfoTest.php
new file mode 100644
index 000000000..8e052df8c
--- /dev/null
+++ b/tests/php/Service/ExAppServiceGetAppInfoTest.php
@@ -0,0 +1,224 @@
+ (parsed to [] by the simplexml/json roundtrip) or an
+ * unnormalized JSON definition never reaches the deploy actions.
+ */
+class ExAppServiceGetAppInfoTest extends TestCase {
+ private ExAppService $service;
+ private string $infoXmlPath = '';
+
+ protected function setUp(): void {
+ parent::setUp();
+
+ $cacheFactory = $this->createMock(ICacheFactory::class);
+ $cacheFactory->method('isAvailable')->willReturn(false);
+
+ $this->service = new ExAppService(
+ $this->createMock(LoggerInterface::class),
+ $cacheFactory,
+ $this->createMock(IUserManager::class),
+ $this->createMock(ExAppFetcher::class),
+ $this->createMock(ExAppArchiveFetcher::class),
+ $this->createMock(ExAppMapper::class),
+ $this->createMock(TopMenuService::class),
+ $this->createMock(InitialStateService::class),
+ $this->createMock(ScriptsService::class),
+ $this->createMock(StylesService::class),
+ $this->createMock(FilesActionsMenuService::class),
+ $this->createMock(TaskProcessingService::class),
+ $this->createMock(TalkBotsService::class),
+ $this->createMock(SettingsService::class),
+ $this->createMock(ExAppOccService::class),
+ $this->createMock(ExAppDeployOptionsService::class),
+ $this->createMock(ExAppSetupCheckService::class),
+ $this->createMock(IConfig::class),
+ );
+ }
+
+ protected function tearDown(): void {
+ if ($this->infoXmlPath !== '' && file_exists($this->infoXmlPath)) {
+ unlink($this->infoXmlPath);
+ }
+ parent::tearDown();
+ }
+
+ private function writeInfoXml(string $environmentVariables): string {
+ $this->infoXmlPath = tempnam(sys_get_temp_dir(), 'appapi-test-info-');
+ file_put_contents($this->infoXmlPath, <<
+
+ test_app
+
+
+$environmentVariables
+
+
+
+XML);
+ return $this->infoXmlPath;
+ }
+
+ public function testEmptyDefaultElementIsDroppedFromInfoXml(): void {
+ $infoXml = $this->writeInfoXml(<<
+ EMPTY_ELEM
+ Empty
+ d
+
+
+
+ NO_DEFAULT
+ No default
+
+
+ KEPT
+ Kept
+ v
+
+XML);
+
+ $appInfo = $this->service->getAppInfo('test_app', $infoXml, null);
+
+ self::assertArrayNotHasKey('error', $appInfo);
+ self::assertSame(
+ ['KEPT' => ['name' => 'KEPT', 'displayName' => 'Kept', 'description' => '', 'default' => 'v', 'value' => 'v']],
+ $appInfo['external-app']['environment-variables']
+ );
+ }
+
+ public function testDeployOptionsOverrideDeclaredVariables(): void {
+ $infoXml = $this->writeInfoXml(<<
+ A
+ x
+
+
+ B
+ y
+
+XML);
+
+ $appInfo = $this->service->getAppInfo('test_app', $infoXml, null, [
+ 'environment_variables' => ['A' => 'overridden', 'B' => ''],
+ ]);
+
+ self::assertArrayNotHasKey('error', $appInfo);
+ self::assertSame(
+ ['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'overridden']],
+ $appInfo['external-app']['environment-variables']
+ );
+ }
+
+ public function testVariableWithEmptyNameElementReturnsError(): void {
+ $infoXml = $this->writeInfoXml(<<
+
+ v
+
+XML);
+
+ $appInfo = $this->service->getAppInfo('test_app', $infoXml, null);
+
+ self::assertArrayHasKey('error', $appInfo);
+ self::assertStringContainsString('invalid environment variable definition', $appInfo['error']);
+ }
+
+ public function testJsonInfoEnvironmentVariablesAreNormalized(): void {
+ $jsonInfo = json_encode([
+ 'id' => 'test_app',
+ 'external-app' => [
+ 'environment-variables' => [
+ 'variable' => [
+ ['name' => 'EMPTY', 'display-name' => 'Empty', 'default' => ''],
+ ['name' => 'KEPT', 'display-name' => 'Kept', 'default' => 'v'],
+ ['name' => 'TYPED', 'default' => 5],
+ ],
+ ],
+ ],
+ ]);
+
+ $appInfo = $this->service->getAppInfo('test_app', null, $jsonInfo, [
+ 'environment_variables' => ['KEPT' => 'overridden'],
+ ]);
+
+ self::assertArrayNotHasKey('error', $appInfo);
+ self::assertSame(
+ [
+ 'KEPT' => ['name' => 'KEPT', 'displayName' => 'Kept', 'description' => '', 'default' => 'v', 'value' => 'overridden'],
+ 'TYPED' => ['name' => 'TYPED', 'displayName' => '', 'description' => '', 'default' => '5', 'value' => '5'],
+ ],
+ $appInfo['external-app']['environment-variables']
+ );
+ }
+
+ public function testJsonInfoRootLevelEnvironmentVariablesAreNormalized(): void {
+ $jsonInfo = json_encode([
+ 'id' => 'test_app',
+ 'environment-variables' => [
+ 'variable' => [
+ ['name' => 'EMPTY', 'default' => ''],
+ ['name' => 'KEPT', 'default' => 'v'],
+ ],
+ ],
+ ]);
+
+ $appInfo = $this->service->getAppInfo('test_app', null, $jsonInfo);
+
+ self::assertArrayNotHasKey('error', $appInfo);
+ self::assertSame(
+ ['KEPT' => ['name' => 'KEPT', 'displayName' => '', 'description' => '', 'default' => 'v', 'value' => 'v']],
+ $appInfo['external-app']['environment-variables']
+ );
+ }
+
+ public function testJsonInfoVariableWithEmptyNameReturnsError(): void {
+ $jsonInfo = json_encode([
+ 'id' => 'test_app',
+ 'external-app' => [
+ 'environment-variables' => [
+ 'variable' => [
+ ['name' => '', 'default' => 'v'],
+ ],
+ ],
+ ],
+ ]);
+
+ $appInfo = $this->service->getAppInfo('test_app', null, $jsonInfo);
+
+ self::assertArrayHasKey('error', $appInfo);
+ self::assertStringContainsString('invalid environment variable definition', $appInfo['error']);
+ }
+}