[libvirt] [PATCH] schema: Allow spaces in paths
Since ages filesystems allowed to have space characters in filenames and even directory names. In fact, on all major filesystems out there you can have whatever character you like except NULL. There's no reason why we should forbid users to not have spaces in their filenames. Moreover, if we do that only on RNG schema level while our XML parser/formatter crunches that happily. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/schemas/basictypes.rng | 8 ++--- .../domainschemadata/domain-disk-source-space.xml | 36 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 tests/domainschemadata/domain-disk-source-space.xml diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 474ad77..f2b7930 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -247,25 +247,25 @@ <define name="filePath"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/% ]+</param> </data> </define> <define name="dirPath"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/% ]+</param> </data> </define> <define name="absFilePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,:]+</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,: ]+</param> </data> </define> <define name="absDirPath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/% ]*</param> </data> </define> diff --git a/tests/domainschemadata/domain-disk-source-space.xml b/tests/domainschemadata/domain-disk-source-space.xml new file mode 100644 index 0000000..553b6c7 --- /dev/null +++ b/tests/domainschemadata/domain-disk-source-space.xml @@ -0,0 +1,36 @@ +<domain type='kvm'> + <name>dummy</name> + <uuid>aa86471a-e67b-41b1-8d7d-2dc37c2ac5ec</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + </features> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/a/path/with some space in it.iso'/> + <target dev='hda' bus='ide'/> + <readonly/> + <boot order='1'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + </devices> +</domain> -- 2.8.4
On 08/11/2016 12:02 PM, Michal Privoznik wrote:
Since ages filesystems allowed to have space characters in filenames and even directory names. In fact, on all major filesystems out there you can have whatever character you like except NULL. There's no reason why we should forbid users to not have spaces in their filenames. Moreover, if we do that only on RNG schema level while our XML parser/formatter crunches that happily.
There's a fedora bug about this particular issue: https://bugzilla.redhat.com/show_bug.cgi?id=1353296 But for example this range still rejects other valid characters as well, like unicode á . So maybe rather than a whitelist, we go the opposite way and make this a minimal blacklist, or drop the validation entirely. Unless there's some designated way to handle regex validation for unicode... - Cole
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/schemas/basictypes.rng | 8 ++--- .../domainschemadata/domain-disk-source-space.xml | 36 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 tests/domainschemadata/domain-disk-source-space.xml
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 474ad77..f2b7930 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -247,25 +247,25 @@
<define name="filePath"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/% ]+</param> </data> </define>
<define name="dirPath"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">[a-zA-Z0-9_\.\+\-\\&"'<>/% ]+</param> </data> </define>
<define name="absFilePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,:]+</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,: ]+</param> </data> </define>
<define name="absDirPath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/% ]*</param> </data> </define>
diff --git a/tests/domainschemadata/domain-disk-source-space.xml b/tests/domainschemadata/domain-disk-source-space.xml new file mode 100644 index 0000000..553b6c7 --- /dev/null +++ b/tests/domainschemadata/domain-disk-source-space.xml @@ -0,0 +1,36 @@ +<domain type='kvm'> + <name>dummy</name> + <uuid>aa86471a-e67b-41b1-8d7d-2dc37c2ac5ec</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.5'>hvm</type> + </os> + <features> + <acpi/> + <apic/> + </features> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source file='/a/path/with some space in it.iso'/> + <target dev='hda' bus='ide'/> + <readonly/> + <boot order='1'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='pci' index='0' model='pci-root'/> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + </devices> +</domain>
On 11.08.2016 19:43, Cole Robinson wrote:
On 08/11/2016 12:02 PM, Michal Privoznik wrote:
Since ages filesystems allowed to have space characters in filenames and even directory names. In fact, on all major filesystems out there you can have whatever character you like except NULL. There's no reason why we should forbid users to not have spaces in their filenames. Moreover, if we do that only on RNG schema level while our XML parser/formatter crunches that happily.
There's a fedora bug about this particular issue:
Ah, thank you for pointing that out. I often forget about those.
But for example this range still rejects other valid characters as well, like unicode á . So maybe rather than a whitelist, we go the opposite way and make this a minimal blacklist, or drop the validation entirely. Unless there's some designated way to handle regex validation for unicode...
Yeah, I guess we can drop the validation completely. I see couple of reasons to do that: a) all modern filesystems allow users to have whatever character they want in the file name (except NULL) [1] b) we must not think about disk sources as UNIX style paths. I mean, there are some hypervisors (like ESX) where disk source is just a name that is translated by hypervisor then into a path that it understands. c) it's okay if we have wider schema but narrower parser. I mean, if schema allows something that is later reject by parser. Will post v2. Thank you. Michal 1: https://en.wikipedia.org/wiki/Comparison_of_file_systems#Limits
participants (2)
-
Cole Robinson -
Michal Privoznik