[pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Gabriel Goller]

Add a maxLength of 24000 to the consentBanner. This is the same limit as
in PVE, and while it makes sense there (file size limits in pmxcfs), it
acts more as an arbitrary stop-gap here.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/config/NodeOptionView.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
index c327356f7f24..966e6d719469 100644
--- a/www/config/NodeOptionView.js
+++ b/www/config/NodeOptionView.js
@@ -59,6 +59,9 @@ Ext.define('PBS.NodeOptionView', {
 	    name: 'consent-text',
 	    text: gettext('Consent Text'),
 	    deleteEmpty: true,
+	    fieldOpts: {
+		maxLength: 24000,
+	    },
 	    onlineHelp: 'consent_banner',
 	},
     ],
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Thomas Lamprecht]

Am 06.12.24 um 13:23 schrieb Gabriel Goller:
> Add a maxLength of 24000 to the consentBanner. This is the same limit as
> in PVE, and while it makes sense there (file size limits in pmxcfs), it
> acts more as an arbitrary stop-gap here.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  www/config/NodeOptionView.js | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
> index c327356f7f24..966e6d719469 100644
> --- a/www/config/NodeOptionView.js
> +++ b/www/config/NodeOptionView.js
> @@ -59,6 +59,9 @@ Ext.define('PBS.NodeOptionView', {
>  	    name: 'consent-text',
>  	    text: gettext('Consent Text'),
>  	    deleteEmpty: true,
> +	    fieldOpts: {
> +		maxLength: 24000,

But that's frontend only? So not really a limitation for anybody.
While it is great to have for UX, the real check should go into
the backend.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Gabriel Goller]

On 09.12.2024 12:29, Thomas Lamprecht wrote:
>Am 06.12.24 um 13:23 schrieb Gabriel Goller:
>> Add a maxLength of 24000 to the consentBanner. This is the same limit as
>> in PVE, and while it makes sense there (file size limits in pmxcfs), it
>> acts more as an arbitrary stop-gap here.
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>  www/config/NodeOptionView.js | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
>> index c327356f7f24..966e6d719469 100644
>> --- a/www/config/NodeOptionView.js
>> +++ b/www/config/NodeOptionView.js
>> @@ -59,6 +59,9 @@ Ext.define('PBS.NodeOptionView', {
>>  	    name: 'consent-text',
>>  	    text: gettext('Consent Text'),
>>  	    deleteEmpty: true,
>> +	    fieldOpts: {
>> +		maxLength: 24000,
>
>But that's frontend only? So not really a limitation for anybody.
>While it is great to have for UX, the real check should go into
>the backend.

Right, I'll add a limit to the api as well.
Note that we also have a request body size limit
(proxmox-rest-server/src/rest.rs:409), which I think is quite sensible,
so I'd set the frontend limit to the request body limit -1024 (for other
options to coexist) (so 63 * 1024) and I'll set the backend limit to
128kB that you suggested.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Gabriel Goller]

On 10.12.2024 11:25, Gabriel Goller wrote:
>On 09.12.2024 12:29, Thomas Lamprecht wrote:
>>Am 06.12.24 um 13:23 schrieb Gabriel Goller:
>>>Add a maxLength of 24000 to the consentBanner. This is the same limit as
>>>in PVE, and while it makes sense there (file size limits in pmxcfs), it
>>>acts more as an arbitrary stop-gap here.
>>>
>>>Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>>>---
>>> www/config/NodeOptionView.js | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>>diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
>>>index c327356f7f24..966e6d719469 100644
>>>--- a/www/config/NodeOptionView.js
>>>+++ b/www/config/NodeOptionView.js
>>>@@ -59,6 +59,9 @@ Ext.define('PBS.NodeOptionView', {
>>> 	    name: 'consent-text',
>>> 	    text: gettext('Consent Text'),
>>> 	    deleteEmpty: true,
>>>+	    fieldOpts: {
>>>+		maxLength: 24000,
>>
>>But that's frontend only? So not really a limitation for anybody.
>>While it is great to have for UX, the real check should go into
>>the backend.
>
>Right, I'll add a limit to the api as well.
>Note that we also have a request body size limit
>(proxmox-rest-server/src/rest.rs:409), which I think is quite sensible,
>so I'd set the frontend limit to the request body limit -1024 (for other
>options to coexist) (so 63 * 1024) and I'll set the backend limit to
>128kB that you suggested.

No I'm stupid that won't help anything.
I'll have to set the backend limit when updating to the 63*1024 as well.
I could set a limit when reading though, but that seems a bit harsh.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Gabriel Goller]

On 10.12.2024 11:27, Gabriel Goller wrote:
>On 10.12.2024 11:25, Gabriel Goller wrote:
>>On 09.12.2024 12:29, Thomas Lamprecht wrote:
>>>Am 06.12.24 um 13:23 schrieb Gabriel Goller:
>>>>Add a maxLength of 24000 to the consentBanner. This is the same limit as
>>>>in PVE, and while it makes sense there (file size limits in pmxcfs), it
>>>>acts more as an arbitrary stop-gap here.
>>>>
>>>>Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>>>>---
>>>>www/config/NodeOptionView.js | 3 +++
>>>>1 file changed, 3 insertions(+)
>>>>
>>>>diff --git a/www/config/NodeOptionView.js b/www/config/NodeOptionView.js
>>>>index c327356f7f24..966e6d719469 100644
>>>>--- a/www/config/NodeOptionView.js
>>>>+++ b/www/config/NodeOptionView.js
>>>>@@ -59,6 +59,9 @@ Ext.define('PBS.NodeOptionView', {
>>>>	    name: 'consent-text',
>>>>	    text: gettext('Consent Text'),
>>>>	    deleteEmpty: true,
>>>>+	    fieldOpts: {
>>>>+		maxLength: 24000,
>>>
>>>But that's frontend only? So not really a limitation for anybody.
>>>While it is great to have for UX, the real check should go into
>>>the backend.
>>
>>Right, I'll add a limit to the api as well.
>>Note that we also have a request body size limit
>>(proxmox-rest-server/src/rest.rs:409), which I think is quite sensible,
>>so I'd set the frontend limit to the request body limit -1024 (for other
>>options to coexist) (so 63 * 1024) and I'll set the backend limit to
>>128kB that you suggested.
>
>No I'm stupid that won't help anything.
>I'll have to set the backend limit when updating to the 63*1024 as well.
>I could set a limit when reading though, but that seems a bit harsh.

Actually the schema does this already. So when setting max_length in the
schema reading and writing above that length fails. Obviously when a
user manually inputs something longer, a few panels in the ui won't
work, but the error message there is quite understandable. Optionally I
could check the length in the validate method that is only checked when
saving new data. But that still allows the user to manually paste
something in the file.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] ui: add consent banner maxLength

[Gabriel Goller]

Sent a v2!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel