[libvirt] [PATCH] allow (only) surrounding whitespace in uuid
Please consider something along these lines. Without it, pretty-printed domxml is rejected due to the whitespace before uuid, and long long string of hexadecimal digits is accepted. --- src/util/uuid.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/util/uuid.c b/src/util/uuid.c index 002a64d..0f2ca96 100644 --- a/src/util/uuid.c +++ b/src/util/uuid.c @@ -145,9 +145,13 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { /* * do a liberal scan allowing '-' and ' ' anywhere between character - * pairs as long as there is 32 of them in the end. + * pairs, and surrounding whitespace, as long as there are exactly + * 32 hexadecimal digits the end. */ cur = uuidstr; + while (c_isspace(*cur)) + cur++; + for (i = 0;i < VIR_UUID_BUFLEN;) { uuid[i] = 0; if (*cur == 0) @@ -170,6 +174,12 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { cur++; } + while (*cur) { + if (!c_isspace(*cur)) + goto error; + cur++; + } + return 0; error: -- 1.6.5.2
On Tue, Jan 05, 2010 at 02:51:06PM +0200, Dan Kenigsberg wrote:
Please consider something along these lines. Without it, pretty-printed domxml is rejected due to the whitespace before uuid, and long long string of hexadecimal digits is accepted. --- src/util/uuid.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/util/uuid.c b/src/util/uuid.c index 002a64d..0f2ca96 100644 --- a/src/util/uuid.c +++ b/src/util/uuid.c @@ -145,9 +145,13 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) {
/* * do a liberal scan allowing '-' and ' ' anywhere between character - * pairs as long as there is 32 of them in the end. + * pairs, and surrounding whitespace, as long as there are exactly + * 32 hexadecimal digits the end. */ cur = uuidstr; + while (c_isspace(*cur)) + cur++; + for (i = 0;i < VIR_UUID_BUFLEN;) { uuid[i] = 0; if (*cur == 0) @@ -170,6 +174,12 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) { cur++; }
+ while (*cur) { + if (!c_isspace(*cur)) + goto error; + cur++; + } + return 0;
error:
Yes that looks fine to me and testing for NUL termination sound important, ACK, pushed, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Jan 21, 2010 at 03:33:21PM +0100, Daniel Veillard wrote:
On Tue, Jan 05, 2010 at 02:51:06PM +0200, Dan Kenigsberg wrote:
Please consider something along these lines. Without it, pretty-printed domxml is rejected due to the whitespace before uuid, and long long string of hexadecimal digits is accepted. ---
Yes that looks fine to me and testing for NUL termination sound important, ACK,
pushed, thanks !
Thank you. Though since I've sent that, I noticed that surrounding whitespace is a more general problem of xml text nodes, such as the <os><type> element. Should libvirt consider any surrounding whitespace as insignificant? What's usually done in the xml world? Dan.
On Sat, Jan 23, 2010 at 05:43:41PM +0200, Dan Kenigsberg wrote:
On Thu, Jan 21, 2010 at 03:33:21PM +0100, Daniel Veillard wrote:
On Tue, Jan 05, 2010 at 02:51:06PM +0200, Dan Kenigsberg wrote:
Please consider something along these lines. Without it, pretty-printed domxml is rejected due to the whitespace before uuid, and long long string of hexadecimal digits is accepted. ---
Yes that looks fine to me and testing for NUL termination sound important, ACK,
pushed, thanks !
Thank you. Though since I've sent that, I noticed that surrounding whitespace is a more general problem of xml text nodes, such as the <os><type> element.
Should libvirt consider any surrounding whitespace as insignificant? What's usually done in the xml world?
From an XML viewpoint, all whitespeces outside of markup constructs (for example spaces spearating attributes on an element) are considered significant and must be reported to the application, at least at the parser level [1]. Any treatment after that is considered application specific. The outcome of SGML experiene is that it's basically impossible to make heuristics on what might be spaces used for markup indentation vs. actual user content, so teh decision was to make everything significant by default. IMHO we should not try to cleanup spaces by default, e.g. in paths <device>/dev/foo </device> should lead to an error at runtime. OS type is a bit on the edge because "Linux" "linux" should probably be considered equivalent, so why not cleanup a leading or trailing space. Daniel [1] okay there are some subtle exceptions in attribute content if the attributes are declared of some types in the DTD but we don't use DTD in libvirt anyway -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Sat, Jan 23, 2010 at 05:43:41PM +0200, Dan Kenigsberg wrote:
On Thu, Jan 21, 2010 at 03:33:21PM +0100, Daniel Veillard wrote:
On Tue, Jan 05, 2010 at 02:51:06PM +0200, Dan Kenigsberg wrote:
Please consider something along these lines. Without it, pretty-printed domxml is rejected due to the whitespace before uuid, and long long string of hexadecimal digits is accepted. ---
Yes that looks fine to me and testing for NUL termination sound important, ACK,
pushed, thanks !
Thank you. Though since I've sent that, I noticed that surrounding whitespace is a more general problem of xml text nodes, such as the <os><type> element.
Should libvirt consider any surrounding whitespace as insignificant?
No, IMHO we should preserve all whitespace. I don't actually agree with the patch that DV just committed to trim whitespace in UUID since it introduces a one-off special case in the XML handling when none of the other elements are trimmed. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Dan Kenigsberg -
Daniel P. Berrange -
Daniel Veillard