[PATCH proxmox-backup] bin: debug: api: fix panic on parameters given more than once

[PATCH proxmox-backup] bin: debug: api: fix panic on parameters given more than once

[Jakob-Tobias Winter]

Array parameters, for example 'group-filter', are set by giving the
parameter once per value. The command line parser collects the values of
a repeated parameter in an array, while the 'api' subcommands expected a
single string for each parameter and panicked with "entered unreachable
code". So array parameters could only be passed with one value.

Hand each value over to the parameter verification separately. Array
parameters now accept more than one value, and other parameters that are
given more than once get rejected as duplicate instead of causing a
panic.

Signed-off-by: Jakob-Tobias Winter <tobias@stuttgart.de>
---
Reproducer:

  proxmox-backup-debug api create /config/tape-backup-job --id two \
      --store store1 --pool pool1 --drive drive1 \
      --group-filter type:vm --group-filter group:ct/200

Tested with debug builds of current master and of 4.2.8, calling the API
in-process (PROXMOX_DEBUG_API_CODE=1): the call panics without the patch
and creates the job with both filters with it. Calls without a repeated
parameter print the same as before. The HTTP path was not tested.

 src/bin/proxmox_backup_debug/api.rs | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/bin/proxmox_backup_debug/api.rs b/src/bin/proxmox_backup_debug/api.rs
index 61cd1b7db..39f8c2100 100644
--- a/src/bin/proxmox_backup_debug/api.rs
+++ b/src/bin/proxmox_backup_debug/api.rs
@@ -153,15 +153,21 @@ fn merge_parameters(
         .collect();
 
     if let Some(Value::Object(map)) = param {
-        param_list.extend(map.into_iter().map(|(k, v)| {
-            (
-                k,
-                match v {
-                    Value::String(s) => s,
-                    _ => unreachable!(), // we're in the CLI
-                },
-            )
-        }));
+        for (k, v) in map {
+            match v {
+                Value::String(s) => param_list.push((k, s)),
+                // a parameter given more than once is collected into an array
+                Value::Array(list) => {
+                    for item in list {
+                        match item {
+                            Value::String(s) => param_list.push((k.clone(), s)),
+                            _ => unreachable!(), // we're in the CLI
+                        }
+                    }
+                }
+                _ => unreachable!(), // we're in the CLI
+            }
+        }
     }
 
     let params = schema.parse_parameter_strings(&param_list, true)?;
-- 
2.53.0

Re: [PATCH proxmox-backup] bin: debug: api: fix panic on parameters given more than once

[Christian Ebner]

On 11/9/26 11:07 PM, Jakob-Tobias Winter wrote:
> Array parameters, for example 'group-filter', are set by giving the
> parameter once per value. The command line parser collects the values of
> a repeated parameter in an array, while the 'api' subcommands expected a
> single string for each parameter and panicked with "entered unreachable
> code". So array parameters could only be passed with one value.
> 
> Hand each value over to the parameter verification separately. Array
> parameters now accept more than one value, and other parameters that are
> given more than once get rejected as duplicate instead of causing a
> panic.
> 
> Signed-off-by: Jakob-Tobias Winter <tobias@stuttgart.de>
Thanks for the patch!

Only minor remarks:
Changes fix the panic, leave however the unreachable! macro in place. 
IMO it would make more sense to propagate these errors and show a 
meaningful error message in the CLI for the respective code paths. That 
should also cover param not being a Value::Object.

Further this would benefit from some regression tests.