[pve-devel] [PATCH storage v6 12/12] plugin: file_size_info: don't ignore base path with whitespace

[Thomas Lamprecht]

Am 18.11.24 um 08:42 schrieb Dominik Csapak:
> On 11/17/24 16:16, Thomas Lamprecht wrote:
>> Am 15.11.24 um 16:17 schrieb Dominik Csapak:
>>> if the base image (parent) of an image contains whitespace in it's path
>>> (e.g. a space), the current untainting would not match and it would seem
>>> there was no parent.
>>
>> do we really want all spaces like newline too? Those sometimes can cause odd
>> things when printing to CLI or the like, so maybe just add space explicitly?
>>
>> Like with: /^([ \S]+)$/
>>
> 
> mhmm i agree that there might be some characters that can make problem.
> 
> in that case I'd rather just 'die' if we encounter a base image with problematic characters,
> instead of treating it as having no parent?

yeah, that's the nicer approach in general, we just need to be somewhat certain
that it cannot happen on existing systems causing some bad regression during
a release, if we can imagine how this can break such systems then maybe just
warn now and change that to a die for PVE 9?

That said, from top of my head it doesn't seem very like that this can easily
happen, so if you think so too then fine by me to die now already.

> 
> I can't exactly remember the context of this patch, but we now disallow
> base images for imported volumes altogether, so not sure if it is still necessary
> to allow such paths for parents
> (file based storages can't have a space in the path, and neither can have volume ids
> created with our api)

yeah, this probably doesn't matters much anymore, but being explicit about the
error here would still be better and possible save some dev/support from some
debugging headache in the future.