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

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

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

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

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