On 04.12.2024 11:16, Thomas Lamprecht wrote:
Am 04.12.24 um 10:16 schrieb Gabriel Goller:
On 03.12.2024 18:24, Thomas Lamprecht wrote:
Am 03.12.24 um 16:29 schrieb Gabriel Goller:
The consent-text paramter is the base64-encoded content of the optional
consent-banner which can be displayed before login.
This can get quite big, would be good to set some relatively high limit here,
I think, as our pmxcfs files have a max size and not being able to change other
options due to this property being close to the pmxcfs file size limit would
not be ideal.
To keep the textareafield widget generic, I would add the maxLength
parameters to pbs and pve separately.
About the limit itself, the example banner in the bug report [0] has 500
characters and these legal texts can sometimes get quite long – so maybe
a limit of 1000 characters?
The one I copied over from some example link [0] comes out as 3733 B in base64,
but I did not encode the image it contained as base64 data-url, so might be
bigger if the text should spot some logo, or patriotic flag image like it was
in my example ^^
To get a real world example for how much bigger I encoded the image from [0]
now, and it comes out as ~ 350 KiB of base64, if converted to WebP format first
it gets down to ~ 32 KiB.
So maybe go for bigger limit, say 128 KiB, as that still gives quite a bit of
headroom to the pmxcfs per-file limit and allows (efficiently encoded) data-URL
images.
According to my conservative calculations that would then be a max input
length (in characters) of circa 24000.
Btw. your Ext.htmlEncode() breaks images with data URLs, our wrapper
around the Markdown parser we use already sanitizes problematic HTML, so an
additional HTML encode step might not be warranted here?
Yep, that's right we do the hmtlEncode in the Markdown.parse function
already so this is not needed. Nice catch!
[0]: https://businessportal.dla.mil/scon/sys-msg/index-ext.html
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=5463
btw. Did we ever talk about placing this into a dedicated, separate file?
(just out of interest, might be also an option to consider here if we did
not talked already about it)
Yep, my first version used a separate file – this was discussed here:
https://lore.proxmox.com/pbs-devel/fc22ec23-a300-488d-821f-bea128588...@proxmox.com/
Hmm, I remember now; with the future-proofing this format makes sense, but
we could have also split content and how it's acted on, e.g., use a separate
file for the notes and add the settings for a future enforcement or the like
to the datacenter.cfg once it happens, I could have thought about that variant
back then already, sorry. The reason I asked this is that for PDM we use a
separate file for the generic notes to avoid cluttering config files comments
with that potential bigger encoded text. But for PBS we already went this way,
and it's OK enough (with an explicit limit enforced by the API schema) so fine
by me to keep as is.
Yep, I'll include the limit in pbs as well.
Thanks for the review!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
