From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C383693164 for ; Mon, 8 Apr 2024 11:31:10 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AC4427545 for ; Mon, 8 Apr 2024 11:31:10 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 8 Apr 2024 11:31:06 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C048044556 for ; Mon, 8 Apr 2024 11:31:06 +0200 (CEST) From: Friedrich Weber To: pve-devel@lists.proxmox.com Date: Mon, 8 Apr 2024 11:30:19 +0200 Message-Id: <20240408093019.109137-1-f.weber@proxmox.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.070 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: [pve-devel] [PATCH widget-toolkit v3] window: edit: avoid sharing custom config objects between subclasses X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Apr 2024 09:31:10 -0000 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 Suggested-by: Thomas Lamprecht Signed-off-by: Friedrich Weber --- Notes: @Thomas, I've added a Suggested-by, feel free to remove/keep as you prefer. 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 v2: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062561.html v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html src/window/Edit.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/window/Edit.js b/src/window/Edit.js index d4a2b551..d5163dd7 100644 --- a/src/window/Edit.js +++ b/src/window/Edit.js @@ -69,6 +69,16 @@ Ext.define('Proxmox.window.Edit', { // onlineHelp of our first item, if set. onlineHelp: undefined, + constructor: function(conf) { + let me = this; + // make copies in order to prevent subclasses from accidentally writing + // to objects that are shared with other edit window subclasses + me.extraRequestParams = Object.assign({}, me.extraRequestParams); + me.submitOptions = Object.assign({}, me.submitOptions); + me.initConfig(conf); + me.callParent(); + }, + isValid: function() { let me = this; -- 2.39.2