fix(compose): allow required and alternative variables in network names

Accept the `?`, `:?`, `+` and `:+` interpolation operators in Compose
network names, such as `${CONTAINER_NAME?error}`. The validation added
in v4.4.0 only knew `-` and `:-`, so these valid files stopped deploying.

The text after `?` is an error message and is not used as the default of
the created environment variable. Unsafe operands are still rejected.

Fixes #12126

bootstrap/helpers/parsers.php

@@ -196,18 +196,38 @@ function validateComposeArrayVolumeSource(string $source): void
 }
 
 /**
- * Splits a top-level network `name:` that is one whole Compose variable (`${VAR}`, `${VAR:-default}`
- * or `${VAR-default}`), such as an external network that differs per server.
+ * Matches one braced Compose variable: `${VAR}`, `${VAR:-default}`, `${VAR-default}`,
+ * `${VAR:?error}`, `${VAR?error}`, `${VAR:+value}` or `${VAR+value}`.
+ */
+const COMPOSE_NETWORK_VARIABLE_PATTERN = '\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?([-?+])([^}]*))?\}';
+
+/**
+ * Splits a top-level network `name:` that is one whole Compose variable (see
+ * COMPOSE_NETWORK_VARIABLE_PATTERN), such as an external network that differs per server. Only the
+ * `-` operators give a default; the text after `?` is an error message and after `+` a value that
+ * Compose uses only when the variable is set.
  *
  * @return array{variable: string, default: ?string}|null
  */
 function composeNetworkNameVariable(string $name): ?array
 {
-    if (preg_match('/\A\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}\z/', $name, $matches) !== 1) {
+    if (preg_match('/\A'.COMPOSE_NETWORK_VARIABLE_PATTERN.'\z/', $name, $matches) !== 1) {
         return null;
     }
 
-    return ['variable' => $matches[1], 'default' => $matches[2] ?? null];
+    return ['variable' => $matches[1], 'default' => ($matches[2] ?? '') === '-' ? $matches[3] : null];
+}
+
+/**
+ * Tells if the text after the operator of a Compose variable is safe. A default or alternative value
+ * becomes part of the network name; an error message only shows in the Compose error, so it can
+ * also contain spaces and simple punctuation.
+ */
+function isSafeComposeNetworkVariableOperand(string $operator, string $operand): bool
+{
+    $pattern = $operator === '?' ? '/\A[A-Za-z0-9 _.,:!-]*\z/' : '/\A[A-Za-z0-9_.-]*\z/';
+
+    return preg_match($pattern, $operand) === 1;
 }
 
 /**
@@ -254,20 +274,19 @@ function validateComposeNetworkNameField(string $name): void
 }
 
 /**
- * Allows $VAR, ${VAR}, ${VAR:-default} and ${VAR-default} with safe defaults in a network value that Compose interpolates.
+ * Allows $VAR and the braced variables of COMPOSE_NETWORK_VARIABLE_PATTERN with safe operands in a network value that Compose interpolates.
  */
 function validateComposeNetworkNameWithVariables(string $name, string $context, ?string $serviceName = null): void
 {
     $variable = composeNetworkNameVariable($name);
-    if ($variable !== null) {
-        $isValid = $variable['default'] === null || ValidationPatterns::isValidDockerNetwork($variable['default']);
+    if ($variable !== null && $variable['default'] !== null) {
+        $isValid = ValidationPatterns::isValidDockerNetwork($variable['default']);
     } else {
         $hasSafeDefaults = true;
         $withoutVariables = preg_replace_callback(
-            '/\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}|\$[A-Za-z_][A-Za-z0-9_]*/',
+            '/'.COMPOSE_NETWORK_VARIABLE_PATTERN.'|\$[A-Za-z_][A-Za-z0-9_]*/',
             function (array $matches) use (&$hasSafeDefaults): string {
-                $default = $matches[2] ?? '';
-                if ($default !== '' && preg_match('/\A[A-Za-z0-9_.-]+\z/', $default) !== 1) {
+                if (! isSafeComposeNetworkVariableOperand($matches[2] ?? '', $matches[3] ?? '')) {
                     $hasSafeDefaults = false;
                 }
 

tests/Feature/ApplicationNetworkNameVariableTest.php

@@ -170,6 +170,28 @@ YAML);
     expect($application->environment_variables()->where('key', 'SHARED_NETWORK')->exists())->toBeFalse();
 });
 
+test('required network name variables are created without their error message as value', function () {
+    $application = makeComposeApplicationWithNetworks(<<<'YAML'
+  shared:
+    external: true
+    name: ${SHARED_NETWORK?error}
+  other:
+    name: ${OTHER_NETWORK:+other-network}
+  plain:
+    name: plain-network
+YAML);
+
+    $application->parse();
+
+    $variables = $application->environment_variables()->get()->keyBy('key');
+
+    expect($variables->has('SHARED_NETWORK'))->toBeTrue()
+        ->and((string) $variables->get('SHARED_NETWORK')->value)->toBe('')
+        ->and($variables->has('OTHER_NETWORK'))->toBeTrue()
+        ->and((string) $variables->get('OTHER_NETWORK')->value)->toBe('')
+        ->and($application->fresh()->docker_compose)->toContain('${SHARED_NETWORK?error}');
+});
+
 test('the deployment .env of a compose application contains the network name variable', function () {
     $application = makeComposeApplicationWithNetworks(<<<'YAML'
   shared:

tests/Unit/DockerComposeInjectionCompatibilityTest.php

@@ -241,8 +241,18 @@ test('service network list entries accept the same variables as network names',
     'plain variable' => ['$NET'],
     'variable mixed with text' => ['${COMPOSE_PROJECT_NAME}_default'],
     'text with a variable default' => ['app-${ENV:-prod}'],
+    'required variable' => ['${NET?error}'],
+    'required non-empty variable with a message' => ['${NET:?NET must be set}'],
+    'variable with an alternative value' => ['${NET:+proxy}'],
+    'text with a required variable' => ['app-${ENV?error}'],
 ]);
 
+test('a network name field can be a required variable', function () {
+    $compose = "services:\n  web:\n    image: nginx\n    networks:\n      - example\nnetworks:\n  example:\n    name: '\${CONTAINER_NAME?error}'\n";
+
+    expect(fn () => validateDockerComposeForInjection($compose))->not->toThrow(Exception::class);
+});
+
 test('unsafe service network list entries with variables are rejected', function (string $network) {
     $compose = "services:\n  web:\n    image: nginx\n    networks:\n      - '{$network}'\n";
 
@@ -254,6 +264,9 @@ test('unsafe service network list entries with variables are rejected', function
     'default with command substitution' => ['${NET:-$(id)}'],
     'default with a space' => ['${NET:-x y}'],
     'default with backticks' => ['${NET:-`id`}'],
+    'error message with command substitution' => ['${NET?$(id)}'],
+    'error message with a separator' => ['${NET:?x;id}'],
+    'alternative value with a space' => ['${NET:+x y}'],
 ]);
 
 test('a service network key with a variable is rejected because Compose does not interpolate keys', function () {

tests/Unit/PreSaveValidationTest.php

@@ -288,6 +288,8 @@ YAML;
     'variable' => ['${SHARED_NETWORK}'],
     'variable with a default' => ['${SHARED_NETWORK:-traefik_public}'],
     'variable with an unset-only default' => ['${SHARED_NETWORK-traefik-public.1}'],
+    'required variable' => ['${SHARED_NETWORK?error}'],
+    'required non-empty variable' => ['${SHARED_NETWORK:?missing}'],
 ]);
 
 test('validateDockerComposeForInjection still blocks unsafe compose network names with variables', function (string $name, string $where) {
@@ -304,7 +306,7 @@ test('validateDockerComposeForInjection still blocks unsafe compose network name
     'text around a variable with command substitution' => ['prefix_${NET}$(id)', 'name'],
     'nested variable' => ['${NET:-${OTHER}}', 'name'],
     'invalid variable name' => ['${1NET}', 'name'],
-    'required-variable form' => ['${NET:?missing}', 'name'],
+    'required variable with command substitution in the message' => ['${NET:?$(id)}', 'name'],
     'newline' => ["\${NET}\nid", 'name'],
     'variable as network key' => ['${NET}', 'key'],
 ]);