Re: [PATCH v1 proxmox-backup] www: percent-encode maintenance mode message to allow commas

[Robert Obkircher <r.obkircher@proxmox.com>]

On 3/27/26 10:18, Dominik Csapak wrote:
> just my 2 cents:
>
> we should probably just adapt the frontend parser for propertystrings
> to also handle quoting. one idea was to use the rust crate + wasm
> to do that (so we have a consistent parser across frontend+backed)
> not sure it worth the effort to integrate wasm just for this single
> parser 
Just to be clear, rust currently doesn't quote [1] correctly either:

|letmode=MaintenanceMode{ ty:MaintenanceType::ReadOnly,
message:Some("abc,def".to_string()), };
lets=proxmox_schema::property_string::PropertyString::new(mode)
.to_property_string() .unwrap();
assert_eq!(s,"read-only,message=abc,def");
letd=MaintenanceMode::deserialize(proxmox_schema::de::SchemaDeserializer::new(
&s, &MaintenanceMode::API_SCHEMA, )) .expect("from
to_property_string");// panics|


This is documented here[1].

[1]
https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox-schema/src/schema.rs;h=47ee94dfa05a4bb38cb9f7772ffed72576408353;hb=HEAD#l1766

>
> we can ofc extend the javascript parser to do that. I have some intial
> js implementation of that lying around (from 2022, and only for
> pve-manager though). 
Note that parseMaintenanceMode is a custom parser that doesn't use
parsePropertyString.

>
> i could send that for pbs if it's desired
>
> if neither is an option, we could ofc go with url encoding/decoding,
> but the downside is that every (api)client has to do it themselves
> (think pdm for example) 
>
>
> On 3/26/26 2:48 PM, Robert Obkircher wrote:
>> Commas and equal signs caused problems because the maintenance mode
>> message is stored in a property string.
>>
>> With a comma and no quotes (e.g. 'a,b'), the backend failed to update
>> datastore.cfg because 'read-only,message=a,b' couldn't be re-parsed.
>> Adding a quote triggered the correct escape logic in the backend, but
>> then the frontend displayed the mode and message incorrectly. It also
>> cut off everything after the first equal sign and silently stripped
>> backslashes.
>>
>> Percent encoding was chosen because MaintenanceMode::check already
>> decoded the message. Previously, this potentially caused the error
>> message to differ from what was displayed in the web UI.
>>
>> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
>> ---
>> I'm not sure if this is a good idea or if we should simply forbid
>> those
>> characters.
>>
>> I also tried changing ElementSerializer::serialize_str to quote like
>> ElementSerializeSeq, but that wouldn't be sufficient because the
>> parser
>> in the frontend would still split by comma and parse something like
>> 'read-only,message="a,b"' as type 'b"' and message '"a'.
>>
>>   www/Utils.js                     | 6 +++++-
>>   www/window/MaintenanceOptions.js | 5 ++---
>>   2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/www/Utils.js b/www/Utils.js
>> index fc9a5916..5e1ee0c6 100644
>> --- a/www/Utils.js
>> +++ b/www/Utils.js
>> @@ -800,7 +800,11 @@ Ext.define('PBS.Utils', {
>>               ([m, msg], pair) => {
>>                   const [key, value] = pair.split('=');
>>                   if (key === 'message') {
>> -                    return [m, value.replace(/^"(.*)"$/,
>> '$1').replace(/\\"/g, '"')];
>> +                    try {
>> +                        return [m, decodeURIComponent(value)];
>> +                    } catch {
>> +                        return [m, value];
>> +                    }
>>                   } else {
>>                       return [value ?? key, msg];
>>                   }
>> diff --git a/www/window/MaintenanceOptions.js
>> b/www/window/MaintenanceOptions.js
>> index 9a735e5e..e9740843 100644
>> --- a/www/window/MaintenanceOptions.js
>> +++ b/www/window/MaintenanceOptions.js
>> @@ -39,9 +39,8 @@ Ext.define('PBS.window.MaintenanceOptions', {
>>               if (values.delete === 'maintenance-type') {
>>                   values.delete = 'maintenance-mode';
>>               } else if (values['maintenance-type']) {
>> -                const message = (values['maintenance-msg'] ?? '')
>> -                    .replaceAll('\\', '')
>> -                    .replaceAll('"', '\\"');
>> +                // property string values can't contain symbols
>> like commas and equal signs
>> +                const message =
>> encodeURIComponent(values['maintenance-msg'] ?? '');
>>                   const maybe_message = values['maintenance-msg'] ?
>> `,message="${message}"` : '';
>>                   values['maintenance-mode'] =
>> `type=${values['maintenance-type']}${maybe_message}`;
>>               }
>