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)) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id E832A780F8 for ; Thu, 29 Apr 2021 14:13:57 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id D3E881BE60 for ; Thu, 29 Apr 2021 14:13:27 +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 UTF8SMTPS id 9D9401BE52 for ; Thu, 29 Apr 2021 14:13:26 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 72A13464CB; Thu, 29 Apr 2021 14:13:26 +0200 (CEST) Message-ID: <9e0cf6d2-c583-fd21-4507-321632840644@proxmox.com> Date: Thu, 29 Apr 2021 14:13:24 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Thunderbird/88.0 Content-Language: en-US To: Proxmox VE development discussion , Lorenz Stechauner References: <20210428141346.240896-1-l.stechauner@proxmox.com> From: Dominik Csapak In-Reply-To: <20210428141346.240896-1-l.stechauner@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.050 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far 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. [me.storage, sencha.com] Subject: Re: [pve-devel] [PATCH manager 1/1] fix #1710: add retrieve from url button for storage 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: Thu, 29 Apr 2021 12:13:58 -0000 one high level comment here: we try to avoid a long 'initComponent' in favor of declaring classes schematically with extjs mvvm (model, view, viewmodel), similar to mvc (model view controller) for the edit window this would mean it looking like this: Ext.define(.... ... controller: { xclass: 'Ext.app.ViewController', fooChange: ... }, items: [ { xtype: 'foo', property1: 'bar', listener: { change: 'fooChange', }, }, ... ], }); this makes it easier to separate layout/hierarchy and the actual behaviours (i.e. some events or actions) more comments inline On 4/28/21 16:13, Lorenz Stechauner wrote: > Add PVE.storage.Retrieve window and PVE.form.hashAlgorithmSelector. > Users are now able to download/retrieve any .iso/... file onto their > storages and verify file integrity with checksums. > > Signed-off-by: Lorenz Stechauner > --- > www/manager6/Makefile | 1 + > www/manager6/form/HashAlgorithmSelector.js | 21 +++ > www/manager6/storage/ContentView.js | 161 +++++++++++++++++++++ > 3 files changed, 183 insertions(+) > create mode 100644 www/manager6/form/HashAlgorithmSelector.js > > diff --git a/www/manager6/Makefile b/www/manager6/Makefile > index afed3283..8e6557d8 100644 > --- a/www/manager6/Makefile > +++ b/www/manager6/Makefile > @@ -38,6 +38,7 @@ JSSRC= \ > form/GlobalSearchField.js \ > form/GroupSelector.js \ > form/GuestIDSelector.js \ > + form/HashAlgorithmSelector.js \ > form/HotplugFeatureSelector.js \ > form/IPProtocolSelector.js \ > form/IPRefSelector.js \ > diff --git a/www/manager6/form/HashAlgorithmSelector.js b/www/manager6/form/HashAlgorithmSelector.js > new file mode 100644 > index 00000000..4a72cc08 > --- /dev/null > +++ b/www/manager6/form/HashAlgorithmSelector.js > @@ -0,0 +1,21 @@ > +Ext.define('PVE.form.hashAlgorithmSelector', { > + extend: 'Proxmox.form.KVComboBox', > + alias: ['widget.pveHashAlgorithmSelector'], > + comboItems: [], > + hasNoneOption: false, > + initComponent: function() { > + var me = this; > + me.comboItems = [ > + ['md5', 'MD5'], > + ['sha1', 'SHA-1'], > + ['sha224', 'SHA-224'], > + ['sha256', 'SHA-256'], > + ['sha384', 'SHA-384'], > + ['sha512', 'SHA-512'], > + ]; > + if (me.hasNoneOption) { > + me.comboItems.unshift(['none', 'None']); > + } why do you make this optional, the only user always adds it afaics? or are there more potential users in the future? in that case, the user may want to configure the whole list (e.g. no md5, only none/sha256, etc.) > + this.callParent(); > + }, > +}); > diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js > index dd6df4b1..7187ebbe 100644 > --- a/www/manager6/storage/ContentView.js > +++ b/www/manager6/storage/ContentView.js > @@ -191,6 +191,153 @@ Ext.define('PVE.storage.Upload', { > }, > }); > > +Ext.define('PVE.storage.Retrieve', { > + extend: 'Proxmox.window.Edit', > + alias: 'widget.pveStorageRetrieve', > + > + resizable: false, this can be omitted here, it's already set to false in the edit window > + > + modal: true, same here > + > + isCreate: true, > + > + showTaskViewer: true, > + upidFieldName: 'upid', this is only necessary because the api call does two things yeah? if we change to a 'get_url_info' + 'retrive' model, this is not necessary at all > + > + initComponent: function() { > + var me = this; > + > + if (!me.nodename) { > + throw "no node name specified"; > + } > + if (!me.storage) { > + throw "no storage ID specified"; > + } > + > + me.url = `/nodes/${me.nodename}/storage/${me.storage}/retrieve`; > + me.method = 'POST'; > + > + let defaultContent = me.contents[0] || ''; > + > + let urlField = Ext.create('Ext.form.field.Text', { > + name: 'url', > + allowBlank: false, > + fieldLabel: gettext('URL'), > + }); > + > + let fileNameField = Ext.create('Ext.form.field.Text', { > + name: 'filename', > + allowBlank: false, > + fieldLabel: gettext('File name'), > + }); > + > + let fileSizeField = Ext.create('Ext.form.field.Text', { > + name: 'size', > + disabled: true, > + fieldLabel: gettext('File size'), > + emptyText: gettext('unknown'), > + }); > + > + let checksumField = Ext.create('Ext.form.field.Text', { > + name: 'checksum', > + fieldLabel: gettext('Checksum'), > + allowBlank: true, > + disabled: true, > + emptyText: gettext('none'), > + }); > + > + let checksumAlgField = Ext.create('PVE.form.hashAlgorithmSelector', { > + name: 'checksumalg', > + fieldLabel: gettext('Hash algorithm'), > + allowBlank: true, > + hasNoneOption: true, > + value: 'none', > + }); > + > + let inputPanel = Ext.create('Proxmox.panel.InputPanel', { > + method: 'POST', > + waitMsgTarget: true, > + border: false, > + columnT: [ > + urlField, > + fileNameField, > + ], > + column1: [ > + { > + xtype: 'pveContentTypeSelector', > + cts: me.contents, > + fieldLabel: gettext('Content'), > + name: 'content', > + value: defaultContent, > + allowBlank: false, > + }, > + ], > + column2: [ > + fileSizeField, > + ], > + advancedColumn1: [ > + checksumField, > + checksumAlgField, > + ], > + advancedColumn2: [ > + { > + xtype: 'checkbox', > + name: 'insecure', > + fieldLabel: gettext('Trust invalid certificates'), > + labelWidth: 150, > + }, this btw. does not work for the filename+size, so i cannot download a file from a url with an untrusted cert (via gui) since the field is never valid > + ], > + }); > + > + urlField.on('change', function() { > + urlField.setValidation("Waiting for response..."); > + urlField.validate(); > + fileSizeField.setValue(""); > + Proxmox.Utils.API2Request({ > + url: me.url, > + method: 'POST', > + params: { > + metaonly: 1, > + url: me.getValues()['url'], > + content: me.getValues()['content'], > + }, > + failure: function(res, opt) { > + urlField.setValidation(res.result.message); > + urlField.validate(); > + fileSizeField.setValue(""); > + }, > + success: function(res, opt) { > + urlField.setValidation(); > + urlField.validate(); > + > + let data = res.result.data; > + fileNameField.setValue(data.filename); > + fileSizeField.setValue(Proxmox.Utils.format_size(data.size)); > + }, > + }); > + }); this needs some buffering as it is now, it makes a request on every key stroke i.e. when i type in: http://www.google.com it makes 21(!) requests e.g. textfield has a 'checkChangeBuffer' property https://docs.sencha.com/extjs/6.0.1/classic/Ext.form.field.Text.html#cfg-checkChangeBuffer or the buffer property of the listener: https://docs.sencha.com/extjs/6.0.1/classic/Ext.util.Observable.html#method-addListener also it would probably be good if the window gets a load mask when the request gets done so the user has a feedback if its taking long > + > + checksumAlgField.on('change', function() { > + if (this.getValue() === 'none') { > + checksumField.setDisabled(true); > + checksumField.setValue(""); > + checksumField.allowBlank = true; > + } else { > + checksumField.setDisabled(false); > + checksumField.allowBlank = false; > + } > + }); > + > + Ext.apply(me, { > + title: gettext('Retrieve from URL'), > + items: inputPanel, > + submitText: gettext('Download'), > + }); > + > + me.callParent(); > + }, > +}); > + > Ext.define('PVE.storage.ContentView', { > extend: 'Ext.grid.GridPanel', > > @@ -262,6 +409,19 @@ Ext.define('PVE.storage.ContentView', { > }, > }); > > + let retrieveButton = Ext.create('Proxmox.button.Button', { > + text: gettext('Retrieve from URL'), > + handler: function() { > + let win = Ext.create('PVE.storage.Retrieve', { > + nodename: nodename, > + storage: storage, > + contents: [content], > + }); > + win.show(); > + win.on('destroy', reload); this can be simplified to Ext.create('PVE.storage.Retrieve', { nodename, storage, contents: [content], autoShow: true, listeners: { destroy: () => reload(), }, }); caution, a listener (.on(), changes the scope of the function) > + }, > + }); > + > let removeButton = Ext.create('Proxmox.button.StdRemoveButton', { > selModel: sm, > delay: 5, > @@ -276,6 +436,7 @@ Ext.define('PVE.storage.ContentView', { > } > if (me.useUploadButton) { > me.tbar.push(uploadButton); > + me.tbar.push(retrieveButton); > } > if (!me.useCustomRemoveButton) { > me.tbar.push(removeButton); >