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

[Dominik Csapak <d.csapak@proxmox.com>]

On 3/27/26 11:19 AM, Robert Obkircher wrote:
> 
> 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
> 

thanks for pointing that out, I thought we had proper quoting support
there (for some reason?) maybe only the parser and not the
serializer.... IMO we should fix that too?

>>
>> 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.
> 

sure but that has a big overlap so we could probably reuse
a proper parser here too...

>>
>> 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}`;
>>>                }
>>