all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Friedrich Weber <f.weber@proxmox.com>
Subject: [pve-devel] applied: [PATCH widget-toolkit v4] window: edit: avoid sharing custom config objects between subclasses
Date: Wed, 10 Apr 2024 10:36:03 +0200	[thread overview]
Message-ID: <2a954f29-3117-4c4d-89c3-67ea0925e2d1@proxmox.com> (raw)
In-Reply-To: <20240409081611.46106-1-f.weber@proxmox.com>

Am 09/04/2024 um 10:16 schrieb Friedrich Weber:
> Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and
> `submitOptions` to two objects that, if not overwritten, are shared
> between all instances of subclasses. This bears the danger of
> modifying the shared object in a subclass instead of overwriting it,
> which affects all edit windows of the current session and can cause
> hard-to-catch GUI bugs.
> 
> One such bug is the following: Currently, the `PVE.pool.AddStorage`
> component inadvertently adds `poolid` to an `extraRequestParams`
> object that is shared between all instances of `Proxmox.window.Edit`.
> As a result, after adding a storage to a pool, opening any edit window
> will send a GET request with a superfluous `poolid` parameter and
> cause an error in the GUI:
> 
>> Parameter verification failed. (400)
>> poolid: property is not defined in schema and the schema does not
>> allow additional properties
> 
> This breaks all edit windows of the current session. A workaround is
> to reload the current browser session.
> 
> To avoid this class of bugs in the future, implement a constructor
> that makes copies of `extraRequestParams` and `submitOptions`. This
> ensures that any subclass instance modifies only its own copies, and
> modifications do not leak to other subclass instances.
> 
> Suggested-by: Stefan Sterz <s.sterz@proxmox.com>
> Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
> ---
> 
> Notes:
>     @Thomas, I've added a Suggested-by, feel free to remove/keep as you
>     prefer.
>     
>     Changes from v3:
>     - Fix broken pool edit window (thx sterzy!) by passing all arguments
>       to `callParent`. The `initConfig` call is obsolete as the constructor
>       of `Ext.Component` [1] calls `initConfig` already.
>     
>     Changes from v1+v2:
>     - As suggested by sterzy (thx!), avoid this class of bugs in a more
>       generic fashion by introducing a `Proxmox.window.Edit` constructor
>       that copies custom config objects
>     - Added full error message to commit message for better searchability
>     
>     v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062657.html
>     v2: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062561.html
>     v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html
>     
>     [1] https://docs.sencha.com/extjs/7.0.0/classic/src/Component.js.html#line2203
> 
>  src/window/Edit.js | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
>

applied, with Stefan's T-b, thanks!




      parent reply	other threads:[~2024-04-10  8:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-09  8:16 [pve-devel] " Friedrich Weber
2024-04-09  8:34 ` Stefan Sterz
2024-04-10  8:36 ` Thomas Lamprecht [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a954f29-3117-4c4d-89c3-67ea0925e2d1@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=f.weber@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal