6:51 a.m.
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
8:33 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:43 a.m.
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.
11:15 a.m.
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
4:49 a.m.
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 :|
5415
days inactive
5435
days old
4 comments
3 participants
participants (3)
-
Dan Kenigsberg -
Daniel P. Berrange -
Daniel Veillard