[libvirt] [PATCH 0/4] xen: parsing and sexpr escaping fixes

This series fixes some xen XM parsing and sexpr generation issues. The first two patches fix parsing /etc/xen files that use python style triple quotes (which libvirt will actually generate in certain situations). The last two patches fix generating xend sexpr with the reserved characters & \ or ' Cole Robinson (4): conf: Convert ParseString to use STRPREFIX conf: Fix parsing python style triple quotes xend: urlencode: Properly escape '&' xend: Escape reserved sexpr characters docs/schemas/domain.rng | 10 +- src/util/buf.c | 79 +++++++++++++++++++ src/util/buf.h | 1 + src/util/conf.c | 20 +++-- src/xen/xend_internal.c | 116 +++++++++++++++------------ tests/xmconfigdata/test-escape-paths.cfg | 2 +- tests/xmconfigdata/test-escape-paths.xml | 5 + tests/xml2sexprdata/xml2sexpr-escape.sexpr | 1 + tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++++++ tests/xml2sexprtest.c | 1 + 10 files changed, 193 insertions(+), 66 deletions(-) create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml -- 1.7.3.2

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/conf.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/util/conf.c b/src/util/conf.c index ba1a384..a31bbc4 100644 --- a/src/util/conf.c +++ b/src/util/conf.c @@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt) return NULL; } NEXT; - } else if ((ctxt->cur + 6 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) { + } else if ((ctxt->cur + 6 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) { + /* String starts with python-style triple quotes """ */ ctxt->cur += 3; base = ctxt->cur; - while ((ctxt->cur + 2 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) { - if (CUR == '\n') ctxt->line++; - NEXT; + + while ((ctxt->cur + 2 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) { + if (CUR == '\n') + ctxt->line++; + NEXT; } - if ((ctxt->cur[0] != '"') || (ctxt->cur[1] != '"') || - (ctxt->cur[2] != '"')) { + + if (!STRPREFIX(ctxt->cur, "\"\"\"")) { virConfError(ctxt, VIR_ERR_CONF_SYNTAX, _("unterminated string")); return(NULL); } -- 1.7.3.2

On 11/19/2010 09:15 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/conf.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c index ba1a384..a31bbc4 100644 --- a/src/util/conf.c +++ b/src/util/conf.c @@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt) return NULL; } NEXT; - } else if ((ctxt->cur + 6 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) { + } else if ((ctxt->cur + 6 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) { + /* String starts with python-style triple quotes """ */ ctxt->cur += 3; base = ctxt->cur; - while ((ctxt->cur + 2 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) {
How did the old code ever find the closing quotes? If there is anything except """ at ctxt->cur after the opening """, then the body of the while loop is never entered and NEXT is never called.
+ + while ((ctxt->cur + 2 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) {
Your rewrite faithfully matches the old code, which means its still looks buggy. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/19/2010 03:00 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/conf.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/util/conf.c b/src/util/conf.c index ba1a384..a31bbc4 100644 --- a/src/util/conf.c +++ b/src/util/conf.c @@ -394,17 +394,20 @@ virConfParseString(virConfParserCtxtPtr ctxt) return NULL; } NEXT; - } else if ((ctxt->cur + 6 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) { + } else if ((ctxt->cur + 6 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) { + /* String starts with python-style triple quotes """ */ ctxt->cur += 3; base = ctxt->cur; - while ((ctxt->cur + 2 < ctxt->end) && (ctxt->cur[0] == '"') && - (ctxt->cur[1] == '"') && (ctxt->cur[2] == '"')) {
How did the old code ever find the closing quotes? If there is anything except """ at ctxt->cur after the opening """, then the body of the while loop is never entered and NEXT is never called.
Yeah, it didn't work :) See patch 2 for that bug fix, I just didn't want to refactor and change functionality in 1 patch. - Cole
+ + while ((ctxt->cur + 2 < ctxt->end) && + (STRPREFIX(ctxt->cur, "\"\"\""))) {
Your rewrite faithfully matches the old code, which means its still looks buggy.

An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/schemas/domain.rng | 10 +++++----- src/util/conf.c | 3 ++- tests/xmconfigdata/test-escape-paths.cfg | 2 +- tests/xmconfigdata/test-escape-paths.xml | 5 +++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index bbbc846..870bea1 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -2028,27 +2028,27 @@ </define> <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="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> <define name="devicePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\+\-\\&"'<>/%]+</param> </data> </define> <define name="deviceName"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\-:/]+</param> + <param name="pattern">[a-zA-Z0-9_\.\-\\&"'<>:/]+</param> </data> </define> <define name="bridgeMode"> diff --git a/src/util/conf.c b/src/util/conf.c index a31bbc4..d9a7603 100644 --- a/src/util/conf.c +++ b/src/util/conf.c @@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt) ctxt->cur += 3; base = ctxt->cur; + /* Find the ending triple quotes */ while ((ctxt->cur + 2 < ctxt->end) && - (STRPREFIX(ctxt->cur, "\"\"\""))) { + !(STRPREFIX(ctxt->cur, "\"\"\""))) { if (CUR == '\n') ctxt->line++; NEXT; diff --git a/tests/xmconfigdata/test-escape-paths.cfg b/tests/xmconfigdata/test-escape-paths.cfg index f9f2cb8..e3e6db9 100644 --- a/tests/xmconfigdata/test-escape-paths.cfg +++ b/tests/xmconfigdata/test-escape-paths.cfg @@ -19,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r" ] +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r", """phy:/dev/HostVG/XenGuest'",hdb,w""" ] vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] parallel = "none" serial = "none" diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index dabf492..13e6e29 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -31,6 +31,11 @@ <target dev='hdc' bus='ide'/> <readonly/> </disk> + <disk type='block' device='disk'> + <driver name='phy'/> + <source dev='/dev/HostVG/XenGuest'"'/> + <target dev='hdb' bus='ide'/> + </disk> <interface type='bridge'> <mac address='00:16:3e:66:92:9c'/> <source bridge='xenbr1'/> -- 1.7.3.2

On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters.
- <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param>
So far, so good...
</data> </define> <define name="devicePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\+\-\\&"'<>/%]+</param>
but given that a devicePath can't have '.', should it really be allowed to have other characters like &, ", ', <, or >?
</data> </define> <define name="deviceName"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\-:/]+</param> + <param name="pattern">[a-zA-Z0-9_\.\-\\&"'<>:/]+</param>
Likewise for deviceName.
+++ b/src/util/conf.c @@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt) ctxt->cur += 3; base = ctxt->cur;
+ /* Find the ending triple quotes */ while ((ctxt->cur + 2 < ctxt->end) && - (STRPREFIX(ctxt->cur, "\"\"\""))) { + !(STRPREFIX(ctxt->cur, "\"\"\""))) {
Ah, the bug fix for patch 1. ACK to patch 1, then.
+++ b/tests/xmconfigdata/test-escape-paths.cfg @@ -19,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r" ] +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r", """phy:/dev/HostVG/XenGuest'",hdb,w""" ] vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] parallel = "none" serial = "none" diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index dabf492..13e6e29 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -31,6 +31,11 @@ <target dev='hdc' bus='ide'/> <readonly/> </disk> + <disk type='block' device='disk'> + <driver name='phy'/> + <source dev='/dev/HostVG/XenGuest'"'/>
Hmm; this really is a case of deviceName in the domain.rng schema. Are there really devices named with ' or " in the name? If so, then ACK to patch 2. If not, then it would be better to use <disk type='file'><source file='/.../XenGuest'"'/>, since that would pick up on absFilePath, which is more likely to match reality of having ' or " in the name. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/19/2010 03:38 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters.
- <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param>
So far, so good...
</data> </define> <define name="devicePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\+\-\\&"'<>/%]+</param>
but given that a devicePath can't have '.', should it really be allowed to have other characters like &, ", ', <, or >?
I didn't notice the lack of '.' but should probably also be added. From the XML point of view, a devicePath could really just be any old FS path. Looking again, deviceName/Path are used in very different ways in the schema, so this might need additional cleanup anyways. But anything that represents a path should probably allow all valid path characters, device or not.
</data> </define> <define name="deviceName"> <data type="string"> - <param name="pattern">[a-zA-Z0-9_\.\-:/]+</param> + <param name="pattern">[a-zA-Z0-9_\.\-\\&"'<>:/]+</param>
Likewise for deviceName.
+++ b/src/util/conf.c @@ -400,8 +400,9 @@ virConfParseString(virConfParserCtxtPtr ctxt) ctxt->cur += 3; base = ctxt->cur;
+ /* Find the ending triple quotes */ while ((ctxt->cur + 2 < ctxt->end) && - (STRPREFIX(ctxt->cur, "\"\"\""))) { + !(STRPREFIX(ctxt->cur, "\"\"\""))) {
Ah, the bug fix for patch 1. ACK to patch 1, then.
+++ b/tests/xmconfigdata/test-escape-paths.cfg @@ -19,7 +19,7 @@ vnc = 1 vncunused = 1 vnclisten = "127.0.0.1" vncpasswd = "123poi" -disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r" ] +disk = [ "phy:/dev/HostVG/XenGuest2,hda,w", "file:/root/boot.iso&test,hdc:cdrom,r", """phy:/dev/HostVG/XenGuest'",hdb,w""" ] vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,script=vif-bridge,model=e1000,type=ioemu" ] parallel = "none" serial = "none" diff --git a/tests/xmconfigdata/test-escape-paths.xml b/tests/xmconfigdata/test-escape-paths.xml index dabf492..13e6e29 100644 --- a/tests/xmconfigdata/test-escape-paths.xml +++ b/tests/xmconfigdata/test-escape-paths.xml @@ -31,6 +31,11 @@ <target dev='hdc' bus='ide'/> <readonly/> </disk> + <disk type='block' device='disk'> + <driver name='phy'/> + <source dev='/dev/HostVG/XenGuest'"'/>
Hmm; this really is a case of deviceName in the domain.rng schema. Are there really devices named with ' or " in the name?
Probably not, but someone could always create a symbolic link with whatever valid pathname they want.
If so, then ACK to patch 2. If not, then it would be better to use <disk type='file'><source file='/.../XenGuest'"'/>, since that would pick up on absFilePath, which is more likely to match reality of having ' or " in the name.
Having a whacky named file is probably more likely, but behind the scenes it's all the same code here that is being tested (xm disk block generation). The restrictions in RNG were fairly arbitrary anyways. Thanks, Cole

On 11/19/2010 02:51 PM, Cole Robinson wrote:
On 11/19/2010 03:38 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters.
- <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param>
So far, so good...
</data> </define> <define name="devicePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\+\-\\&"'<>/%]+</param>
but given that a devicePath can't have '.', should it really be allowed to have other characters like &, ", ', <, or >?
I didn't notice the lack of '.' but should probably also be added. From the XML point of view, a devicePath could really just be any old FS path.
If that's the case, then can we consolidate things rather than repeating the same pattern multiple times? That is, can the schema use _just_ filePath and absFilePath, rather than confusing things by adding devicePath? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/19/2010 04:56 PM, Eric Blake wrote:
On 11/19/2010 02:51 PM, Cole Robinson wrote:
On 11/19/2010 03:38 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
An incorrect check broke matching the closing set of quotes. Update tests to cover this case for XM config files, and update the domain schema to allow more path characters.
- <param name="pattern">/[a-zA-Z0-9_\.\+\-&/%]*</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]*</param>
So far, so good...
</data> </define> <define name="devicePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\+\-/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\+\-\\&"'<>/%]+</param>
but given that a devicePath can't have '.', should it really be allowed to have other characters like &, ", ', <, or >?
I didn't notice the lack of '.' but should probably also be added. From the XML point of view, a devicePath could really just be any old FS path.
If that's the case, then can we consolidate things rather than repeating the same pattern multiple times? That is, can the schema use _just_ filePath and absFilePath, rather than confusing things by adding devicePath?
Sounds good, I'll update the patch. Thanks, Cole

Since we send the sexpr to xend via HTTP, we need to properly escape '&' Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/xen/xend_internal.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index d6d66bd..3ccadde 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -710,6 +710,7 @@ urlencode(const char *string) switch (string[i]) { case ' ': case '\n': + case '&': snprintf(ptr, 4, "%%%02x", string[i]); ptr += 3; break; -- 1.7.3.2

On 11/19/2010 09:15 AM, Cole Robinson wrote:
Since we send the sexpr to xend via HTTP, we need to properly escape '&'
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/xen/xend_internal.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

If we don't escape ' or \ xend can't parse the generated sexpr. This might over apply the EscapeSexpr routine, but it shouldn't hurt. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/buf.c | 79 +++++++++++++++++++ src/util/buf.h | 1 + src/xen/xend_internal.c | 115 +++++++++++++++------------- tests/xml2sexprdata/xml2sexpr-escape.sexpr | 1 + tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++++++ tests/xml2sexprtest.c | 1 + 6 files changed, 169 insertions(+), 52 deletions(-) create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml diff --git a/src/util/buf.c b/src/util/buf.c index 553e2a0..90034ad 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -379,6 +379,85 @@ err: } /** + * virBufferEscapeSexpr: + * @buf: the buffer to dump + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which need to be escaped + * + * Do a formatted print with a single string to an sexpr buffer. The string + * is escaped to avoid generating a sexpr that xen will choke on. This + * doesn't fully escape the sexpr, just enough for our code to work. + */ +void +virBufferEscapeSexpr(const virBufferPtr buf, + const char *format, + const char *str) +{ + int size, count, len, grow_size; + char *escaped, *out; + const char *cur; + + if ((format == NULL) || (buf == NULL) || (str == NULL)) + return; + + if (buf->error) + return; + + len = strlen(str); + if (VIR_ALLOC_N(escaped, 2 * len + 1) < 0) { + virBufferNoMemory(buf); + return; + } + + cur = str; + out = escaped; + while (*cur != 0) { + switch (*cur) { + case '\\': + case '\'': + *out++ = '\\'; + /* fallthrough */ + default: + *out++ = *cur; + } + cur++; + } + *out = 0; + + if ((buf->use >= buf->size) && + virBufferGrow(buf, 100) < 0) { + goto err; + } + + size = buf->size - buf->use; + if ((count = snprintf(&buf->content[buf->use], size, + format, (char *)escaped)) < 0) { + buf->error = 1; + goto err; + } + + /* Grow buffer if necessary and retry */ + if (count >= size) { + buf->content[buf->use] = 0; + grow_size = (count + 1 > 1000) ? count + 1 : 1000; + if (virBufferGrow(buf, grow_size) < 0) { + goto err; + } + size = buf->size - buf->use; + + if ((count = snprintf(&buf->content[buf->use], size, + format, (char *)escaped)) < 0) { + buf->error = 1; + goto err; + } + } + buf->use += count; + +err: + VIR_FREE(escaped); +} + +/** * virBufferURIEncodeString: * @buf: the buffer to append to * @str: the string argument which will be URI-encoded diff --git a/src/util/buf.h b/src/util/buf.h index 6616898..54f4de5 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -45,6 +45,7 @@ void virBufferVSprintf(const virBufferPtr buf, const char *format, ...) void virBufferStrcat(const virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; void virBufferEscapeString(const virBufferPtr buf, const char *format, const char *str); +void virBufferEscapeSexpr(const virBufferPtr buf, const char *format, const char *str); void virBufferURIEncodeString (const virBufferPtr buf, const char *str); # define virBufferAddLit(buf_, literal_string_) \ diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 3ccadde..7a36fb0 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -522,6 +522,7 @@ xend_op_ext(virConnectPtr xend, const char *path, const char *key, va_list ap) } content = virBufferContentAndReset(&buf); + DEBUG("xend op: %s\n", content); ret = http2unix(xend_post(xend, path, content)); VIR_FREE(content); @@ -4605,8 +4606,6 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { goto error; } - DEBUG("Defining w/ sexpr: \n%s", sexpr); - ret = xend_op(conn, "", "op", "new", "config", sexpr, NULL); VIR_FREE(sexpr); if (ret != 0) { @@ -5297,11 +5296,12 @@ xenDaemonFormatSxprChr(virDomainChrDefPtr def, case VIR_DOMAIN_CHR_TYPE_FILE: case VIR_DOMAIN_CHR_TYPE_PIPE: - virBufferVSprintf(buf, "%s:%s", type, def->data.file.path); + virBufferVSprintf(buf, "%s:", type); + virBufferEscapeSexpr(buf, "%s", def->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_DEV: - virBufferVSprintf(buf, "%s", def->data.file.path); + virBufferEscapeSexpr(buf, "%s", def->data.file.path); break; case VIR_DOMAIN_CHR_TYPE_TCP: @@ -5322,8 +5322,9 @@ xenDaemonFormatSxprChr(virDomainChrDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferVSprintf(buf, "%s:%s%s", type, - def->data.nix.path, + virBufferVSprintf(buf, "%s:", type); + virBufferEscapeSexpr(buf, "%s", def->data.nix.path); + virBufferVSprintf(buf, "%s", def->data.nix.listen ? ",server,nowait" : ""); break; } @@ -5400,39 +5401,42 @@ xenDaemonFormatSxprDisk(virConnectPtr conn ATTRIBUTE_UNUSED, if (hvm) { /* Xend <= 3.0.2 wants a ioemu: prefix on devices for HVM */ - if (xendConfigVersion == 1) - virBufferVSprintf(buf, "(dev 'ioemu:%s')", def->dst); - else /* But newer does not */ - virBufferVSprintf(buf, "(dev '%s:%s')", def->dst, - def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? - "cdrom" : "disk"); + if (xendConfigVersion == 1) { + virBufferEscapeSexpr(buf, "(dev 'ioemu:%s')", def->dst); + } else { + /* But newer does not */ + virBufferEscapeSexpr(buf, "(dev '%s:", def->dst); + virBufferEscapeSexpr(buf, "%s')", + def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? + "cdrom" : "disk"); + } } else if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - virBufferVSprintf(buf, "(dev '%s:cdrom')", def->dst); + virBufferEscapeSexpr(buf, "(dev '%s:cdrom')", def->dst); } else { - virBufferVSprintf(buf, "(dev '%s')", def->dst); + virBufferEscapeSexpr(buf, "(dev '%s')", def->dst); } if (def->src) { if (def->driverName) { if (STREQ(def->driverName, "tap") || STREQ(def->driverName, "tap2")) { - virBufferVSprintf(buf, "(uname '%s:%s:%s')", - def->driverName, - def->driverType ? def->driverType : "aio", - def->src); + virBufferEscapeSexpr(buf, "(uname '%s:", def->driverName); + virBufferEscapeSexpr(buf, "%s:", + def->driverType ? def->driverType : "aio"); + virBufferEscapeSexpr(buf, "%s')", def->src); } else { - virBufferVSprintf(buf, "(uname '%s:%s')", - def->driverName, - def->src); + virBufferEscapeSexpr(buf, "(uname '%s:", def->driverName); + virBufferEscapeSexpr(buf, "%s')", def->src); } } else { if (def->type == VIR_DOMAIN_DISK_TYPE_FILE) { - virBufferVSprintf(buf, "(uname 'file:%s')", def->src); + virBufferEscapeSexpr(buf, "(uname 'file:%s')", def->src); } else if (def->type == VIR_DOMAIN_DISK_TYPE_BLOCK) { if (def->src[0] == '/') - virBufferVSprintf(buf, "(uname 'phy:%s')", def->src); + virBufferEscapeSexpr(buf, "(uname 'phy:%s')", def->src); else - virBufferVSprintf(buf, "(uname 'phy:/dev/%s')", def->src); + virBufferEscapeSexpr(buf, "(uname 'phy:/dev/%s')", + def->src); } else { virXendError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported disk type %s"), @@ -5501,13 +5505,13 @@ xenDaemonFormatSxprNet(virConnectPtr conn, switch (def->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: - virBufferVSprintf(buf, "(bridge '%s')", def->data.bridge.brname); + virBufferEscapeSexpr(buf, "(bridge '%s')", def->data.bridge.brname); if (def->data.bridge.script) script = def->data.bridge.script; - virBufferVSprintf(buf, "(script '%s')", script); + virBufferEscapeSexpr(buf, "(script '%s')", script); if (def->data.bridge.ipaddr != NULL) - virBufferVSprintf(buf, "(ip '%s')", def->data.bridge.ipaddr); + virBufferEscapeSexpr(buf, "(ip '%s')", def->data.bridge.ipaddr); break; case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -5530,17 +5534,18 @@ xenDaemonFormatSxprNet(virConnectPtr conn, def->data.network.name); return -1; } - virBufferVSprintf(buf, "(bridge '%s')", bridge); - virBufferVSprintf(buf, "(script '%s')", script); + virBufferEscapeSexpr(buf, "(bridge '%s')", bridge); + virBufferEscapeSexpr(buf, "(script '%s')", script); VIR_FREE(bridge); } break; case VIR_DOMAIN_NET_TYPE_ETHERNET: if (def->data.ethernet.script) - virBufferVSprintf(buf, "(script '%s')", def->data.ethernet.script); + virBufferEscapeSexpr(buf, "(script '%s')", + def->data.ethernet.script); if (def->data.ethernet.ipaddr != NULL) - virBufferVSprintf(buf, "(ip '%s')", def->data.ethernet.ipaddr); + virBufferEscapeSexpr(buf, "(ip '%s')", def->data.ethernet.ipaddr); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -5555,11 +5560,11 @@ xenDaemonFormatSxprNet(virConnectPtr conn, if (def->ifname != NULL && !STRPREFIX(def->ifname, "vif")) - virBufferVSprintf(buf, "(vifname '%s')", def->ifname); + virBufferEscapeSexpr(buf, "(vifname '%s')", def->ifname); if (!hvm) { if (def->model != NULL) - virBufferVSprintf(buf, "(model '%s')", def->model); + virBufferEscapeSexpr(buf, "(model '%s')", def->model); } else if (def->model == NULL) { /* @@ -5573,7 +5578,7 @@ xenDaemonFormatSxprNet(virConnectPtr conn, virBufferAddLit(buf, "(type netfront)"); } else { - virBufferVSprintf(buf, "(model '%s')", def->model); + virBufferEscapeSexpr(buf, "(model '%s')", def->model); virBufferAddLit(buf, "(type ioemu)"); } @@ -5680,7 +5685,8 @@ xenDaemonFormatSxprSound(virDomainDefPtr def, def->sounds[i]->model); return -1; } - virBufferVSprintf(buf, "%s%s", i ? "," : "", str); + virBufferVSprintf(buf, "%s", i ? "," : ""); + virBufferEscapeSexpr(buf, "%s", str); } if (virBufferError(buf)) { @@ -5737,10 +5743,13 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBuffer buf = VIR_BUFFER_INITIALIZER; char uuidstr[VIR_UUID_STRING_BUFLEN]; const char *tmp; + char *bufout; int hvm = 0, i; + DEBUG0("Formatting domain sexpr"); + virBufferAddLit(&buf, "(vm "); - virBufferVSprintf(&buf, "(name '%s')", def->name); + virBufferEscapeSexpr(&buf, "(name '%s')", def->name); virBufferVSprintf(&buf, "(memory %lu)(maxmem %lu)", def->mem.cur_balloon/1024, def->mem.max_balloon/1024); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); @@ -5753,7 +5762,7 @@ xenDaemonFormatSxpr(virConnectPtr conn, char *ranges = virDomainCpuSetFormat(def->cpumask, def->cpumasklen); if (ranges == NULL) goto error; - virBufferVSprintf(&buf, "(cpus '%s')", ranges); + virBufferEscapeSexpr(&buf, "(cpus '%s')", ranges); VIR_FREE(ranges); } @@ -5761,16 +5770,16 @@ xenDaemonFormatSxpr(virConnectPtr conn, virBufferVSprintf(&buf, "(uuid '%s')", uuidstr); if (def->description) - virBufferVSprintf(&buf, "(description '%s')", def->description); + virBufferEscapeSexpr(&buf, "(description '%s')", def->description); if (def->os.bootloader) { if (def->os.bootloader[0]) - virBufferVSprintf(&buf, "(bootloader '%s')", def->os.bootloader); + virBufferEscapeSexpr(&buf, "(bootloader '%s')", def->os.bootloader); else virBufferAddLit(&buf, "(bootloader)"); if (def->os.bootloaderArgs) - virBufferVSprintf(&buf, "(bootloader_args '%s')", def->os.bootloaderArgs); + virBufferEscapeSexpr(&buf, "(bootloader_args '%s')", def->os.bootloaderArgs); } if (!(tmp = virDomainLifecycleTypeToString(def->onPoweroff))) { @@ -5827,20 +5836,20 @@ xenDaemonFormatSxpr(virConnectPtr conn, } if (def->os.kernel) - virBufferVSprintf(&buf, "(kernel '%s')", def->os.kernel); + virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.kernel); if (def->os.initrd) - virBufferVSprintf(&buf, "(ramdisk '%s')", def->os.initrd); + virBufferEscapeSexpr(&buf, "(ramdisk '%s')", def->os.initrd); if (def->os.root) - virBufferVSprintf(&buf, "(root '%s')", def->os.root); + virBufferEscapeSexpr(&buf, "(root '%s')", def->os.root); if (def->os.cmdline) - virBufferVSprintf(&buf, "(args '%s')", def->os.cmdline); + virBufferEscapeSexpr(&buf, "(args '%s')", def->os.cmdline); if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST+1]; if (def->os.kernel) - virBufferVSprintf(&buf, "(loader '%s')", def->os.loader); + virBufferEscapeSexpr(&buf, "(loader '%s')", def->os.loader); else - virBufferVSprintf(&buf, "(kernel '%s')", def->os.loader); + virBufferEscapeSexpr(&buf, "(kernel '%s')", def->os.loader); virBufferVSprintf(&buf, "(vcpus %u)", def->maxvcpus); if (def->vcpus < def->maxvcpus) @@ -5883,14 +5892,14 @@ xenDaemonFormatSxpr(virConnectPtr conn, def->disks[i]->src == NULL) break; - virBufferVSprintf(&buf, "(cdrom '%s')", - def->disks[i]->src); + virBufferEscapeSexpr(&buf, "(cdrom '%s')", + def->disks[i]->src); break; case VIR_DOMAIN_DISK_DEVICE_FLOPPY: /* all xend versions define floppies here */ - virBufferVSprintf(&buf, "(%s '%s')", def->disks[i]->dst, - def->disks[i]->src); + virBufferEscapeSexpr(&buf, "(%s ", def->disks[i]->dst); + virBufferEscapeSexpr(&buf, "'%s')", def->disks[i]->src); break; default: @@ -5942,7 +5951,7 @@ xenDaemonFormatSxpr(virConnectPtr conn, /* get the device emulation model */ if (def->emulator && (hvm || xendConfigVersion >= 3)) - virBufferVSprintf(&buf, "(device_model '%s')", def->emulator); + virBufferEscapeSexpr(&buf, "(device_model '%s')", def->emulator); /* PV graphics for xen <= 3.0.4, or HVM graphics for xen <= 3.1.0 */ @@ -5986,7 +5995,9 @@ xenDaemonFormatSxpr(virConnectPtr conn, goto error; } - return virBufferContentAndReset(&buf); + bufout = virBufferContentAndReset(&buf); + DEBUG("Formatted sexpr: \n%s", bufout); + return bufout; error: virBufferFreeAndReset(&buf); diff --git a/tests/xml2sexprdata/xml2sexpr-escape.sexpr b/tests/xml2sexprdata/xml2sexpr-escape.sexpr new file mode 100644 index 0000000..efb0344 --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-escape.sexpr @@ -0,0 +1 @@ +(vm (name 'fvtest')(memory 420)(maxmem 420)(vcpus 2)(uuid '596a5d21-71f4-8fb2-e068-e2386a5c413e')(on_poweroff 'destroy')(on_reboot 'destroy')(on_crash 'destroy')(image (hvm (kernel '/var/lib/xen/vmlinuz.2Dn2YT')(ramdisk '/var/lib/xen/initrd.img.0u-Vhq')(args ' method=http://&""download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os ')(loader '/usr/lib/xen/boot/hvmloader')(vcpus 2)(boot c)(usb 1)(parallel none)(serial pty)(device_model '/usr/lib/xen/bin/qemu-dm')))(device (vbd (dev 'ioemu:xvda')(uname 'file:/root/\'\\some.img')(mode 'w')))) \ No newline at end of file diff --git a/tests/xml2sexprdata/xml2sexpr-escape.xml b/tests/xml2sexprdata/xml2sexpr-escape.xml new file mode 100644 index 0000000..73beb6b --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-escape.xml @@ -0,0 +1,24 @@ +<domain type='xen' id='15'> + <name>fvtest</name> + <uuid>596a5d2171f48fb2e068e2386a5c413e</uuid> + <os> + <type>hvm</type> + <kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel> + <initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd> + <cmdline> method=http://&""download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os </cmdline> + <loader>/usr/lib/xen/boot/hvmloader</loader> + </os> + <memory>430080</memory> + <vcpu>2</vcpu> + <on_poweroff>destroy</on_poweroff> + <on_reboot>destroy</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-dm</emulator> + <disk type='file' device='disk'> + <source file='/root/'\some.img'/> + <target dev='xvda'/> + </disk> + <console tty='/dev/pts/4'/> + </devices> +</domain> diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c index 9cf8d39..8a5d115 100644 --- a/tests/xml2sexprtest.c +++ b/tests/xml2sexprtest.c @@ -163,6 +163,7 @@ mymain(int argc, char **argv) DO_TEST("fv-net-netfront", "fv-net-netfront", "fvtest", 1); DO_TEST("boot-grub", "boot-grub", "fvtest", 1); + DO_TEST("escape", "escape", "fvtest", 1); virCapabilitiesFree(caps); -- 1.7.3.2

On 11/19/2010 09:15 AM, Cole Robinson wrote:
If we don't escape ' or \ xend can't parse the generated sexpr. This might over apply the EscapeSexpr routine, but it shouldn't hurt.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/buf.c | 79 +++++++++++++++++++ src/util/buf.h | 1 + src/xen/xend_internal.c | 115 +++++++++++++++------------- tests/xml2sexprdata/xml2sexpr-escape.sexpr | 1 + tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++++++ tests/xml2sexprtest.c | 1 + 6 files changed, 169 insertions(+), 52 deletions(-) create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml
diff --git a/src/util/buf.c b/src/util/buf.c index 553e2a0..90034ad 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -379,6 +379,85 @@ err: }
/** + * virBufferEscapeSexpr: + * @buf: the buffer to dump + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which need to be escaped + * + * Do a formatted print with a single string to an sexpr buffer. The string + * is escaped to avoid generating a sexpr that xen will choke on. This + * doesn't fully escape the sexpr, just enough for our code to work. + */ +void +virBufferEscapeSexpr(const virBufferPtr buf, + const char *format, + const char *str) +{ + int size, count, len, grow_size; + char *escaped, *out; + const char *cur; + + if ((format == NULL) || (buf == NULL) || (str == NULL)) + return; + + if (buf->error) + return; + + len = strlen(str);
Right here, is it worth adding: if (strcspn(str, "\\'") == len) { virBufferVSprintf(buf, format, str); return; }
+ if (VIR_ALLOC_N(escaped, 2 * len + 1) < 0) { + virBufferNoMemory(buf); + return; + }
so as to avoid the alloc in the common case of nothing to escape?
+ *out = 0; + + if ((buf->use >= buf->size) && + virBufferGrow(buf, 100) < 0) { + goto err; + }
That 100 looks wrong; shouldn't it instead be max(100,out-escaped)? For that matter, why do all this low level stuff; wouldn't it be easier after *out = 0 to just do: virBufferVSpring(buf, format, escaped); VIR_FREE(escaped); return; and leave all the resizing and snprintf stuff to the already written function?
diff --git a/src/util/buf.h b/src/util/buf.h index 6616898..54f4de5 100644 --- a/src/util/buf.h @@ -5400,39 +5401,42 @@ xenDaemonFormatSxprDisk(virConnectPtr conn ATTRIBUTE_UNUSED,
if (hvm) { /* Xend <= 3.0.2 wants a ioemu: prefix on devices for HVM */ - if (xendConfigVersion == 1) - virBufferVSprintf(buf, "(dev 'ioemu:%s')", def->dst); - else /* But newer does not */ - virBufferVSprintf(buf, "(dev '%s:%s')", def->dst, - def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? - "cdrom" : "disk"); + if (xendConfigVersion == 1) { + virBufferEscapeSexpr(buf, "(dev 'ioemu:%s')", def->dst); + } else { + /* But newer does not */ + virBufferEscapeSexpr(buf, "(dev '%s:", def->dst); + virBufferEscapeSexpr(buf, "%s')", + def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? + "cdrom" : "disk");
This can be virBufferVSprintf(buf, "%s')",...), given that the only two possible strings for %s don't need escaping.
@@ -5680,7 +5685,8 @@ xenDaemonFormatSxprSound(virDomainDefPtr def, def->sounds[i]->model); return -1; } - virBufferVSprintf(buf, "%s%s", i ? "," : "", str); + virBufferVSprintf(buf, "%s", i ? "," : "");
I'd write this as 'if (i) virBufferAddChar(buf, ',')' rather than a complicated VSprintf.
diff --git a/tests/xml2sexprdata/xml2sexpr-escape.xml b/tests/xml2sexprdata/xml2sexpr-escape.xml new file mode 100644 index 0000000..73beb6b --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-escape.xml @@ -0,0 +1,24 @@ +<domain type='xen' id='15'> + <name>fvtest</name> + <uuid>596a5d2171f48fb2e068e2386a5c413e</uuid> + <os> + <type>hvm</type> + <kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel> + <initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd> + <cmdline> method=http://&""download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os </cmdline>
Seems unusual to have & and " in a URL; but the point of this test was not so much a real configuration as a way to expose the sexpr escaping code path. Is there any better place to stick this where it won't be quite so confusing? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 11/19/2010 03:57 PM, Eric Blake wrote:
On 11/19/2010 09:15 AM, Cole Robinson wrote:
If we don't escape ' or \ xend can't parse the generated sexpr. This might over apply the EscapeSexpr routine, but it shouldn't hurt.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/buf.c | 79 +++++++++++++++++++ src/util/buf.h | 1 + src/xen/xend_internal.c | 115 +++++++++++++++------------- tests/xml2sexprdata/xml2sexpr-escape.sexpr | 1 + tests/xml2sexprdata/xml2sexpr-escape.xml | 24 ++++++ tests/xml2sexprtest.c | 1 + 6 files changed, 169 insertions(+), 52 deletions(-) create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.sexpr create mode 100644 tests/xml2sexprdata/xml2sexpr-escape.xml
diff --git a/src/util/buf.c b/src/util/buf.c index 553e2a0..90034ad 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -379,6 +379,85 @@ err: }
/** + * virBufferEscapeSexpr: + * @buf: the buffer to dump + * @format: a printf like format string but with only one %s parameter + * @str: the string argument which need to be escaped + * + * Do a formatted print with a single string to an sexpr buffer. The string + * is escaped to avoid generating a sexpr that xen will choke on. This + * doesn't fully escape the sexpr, just enough for our code to work. + */ +void +virBufferEscapeSexpr(const virBufferPtr buf, + const char *format, + const char *str) +{ + int size, count, len, grow_size; + char *escaped, *out; + const char *cur; + + if ((format == NULL) || (buf == NULL) || (str == NULL)) + return; + + if (buf->error) + return; + + len = strlen(str);
Right here, is it worth adding:
if (strcspn(str, "\\'") == len) { virBufferVSprintf(buf, format, str); return; }
+ if (VIR_ALLOC_N(escaped, 2 * len + 1) < 0) { + virBufferNoMemory(buf); + return; + }
so as to avoid the alloc in the common case of nothing to escape?
+ *out = 0; + + if ((buf->use >= buf->size) && + virBufferGrow(buf, 100) < 0) { + goto err; + }
That 100 looks wrong; shouldn't it instead be max(100,out-escaped)? For that matter, why do all this low level stuff; wouldn't it be easier after *out = 0 to just do:
virBufferVSpring(buf, format, escaped); VIR_FREE(escaped); return;
and leave all the resizing and snprintf stuff to the already written function?
This function is basically a copy of virBufferEscapeString. I'll add a patch to cleanup the original function before duplicating.
diff --git a/src/util/buf.h b/src/util/buf.h index 6616898..54f4de5 100644 --- a/src/util/buf.h @@ -5400,39 +5401,42 @@ xenDaemonFormatSxprDisk(virConnectPtr conn ATTRIBUTE_UNUSED,
if (hvm) { /* Xend <= 3.0.2 wants a ioemu: prefix on devices for HVM */ - if (xendConfigVersion == 1) - virBufferVSprintf(buf, "(dev 'ioemu:%s')", def->dst); - else /* But newer does not */ - virBufferVSprintf(buf, "(dev '%s:%s')", def->dst, - def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? - "cdrom" : "disk"); + if (xendConfigVersion == 1) { + virBufferEscapeSexpr(buf, "(dev 'ioemu:%s')", def->dst); + } else { + /* But newer does not */ + virBufferEscapeSexpr(buf, "(dev '%s:", def->dst); + virBufferEscapeSexpr(buf, "%s')", + def->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? + "cdrom" : "disk");
This can be virBufferVSprintf(buf, "%s')",...), given that the only two possible strings for %s don't need escaping.
@@ -5680,7 +5685,8 @@ xenDaemonFormatSxprSound(virDomainDefPtr def, def->sounds[i]->model); return -1; } - virBufferVSprintf(buf, "%s%s", i ? "," : "", str); + virBufferVSprintf(buf, "%s", i ? "," : "");
I'd write this as 'if (i) virBufferAddChar(buf, ',')' rather than a complicated VSprintf.
Okay, i'll fold in these changes.
diff --git a/tests/xml2sexprdata/xml2sexpr-escape.xml b/tests/xml2sexprdata/xml2sexpr-escape.xml new file mode 100644 index 0000000..73beb6b --- /dev/null +++ b/tests/xml2sexprdata/xml2sexpr-escape.xml @@ -0,0 +1,24 @@ +<domain type='xen' id='15'> + <name>fvtest</name> + <uuid>596a5d2171f48fb2e068e2386a5c413e</uuid> + <os> + <type>hvm</type> + <kernel>/var/lib/xen/vmlinuz.2Dn2YT</kernel> + <initrd>/var/lib/xen/initrd.img.0u-Vhq</initrd> + <cmdline> method=http://&""download.fedora.devel.redhat.com/pub/fedora/linux/core/test/5.91/x86_64/os </cmdline>
Seems unusual to have & and " in a URL; but the point of this test was not so much a real configuration as a way to expose the sexpr escaping code path. Is there any better place to stick this where it won't be quite so confusing?
Actually the BZ prompting this work was for a URL with & in it: https://bugzilla.redhat.com/show_bug.cgi?id=472437 I'll format the URL in a more sensible way though. Thanks, Cole
participants (2)
-
Cole Robinson
-
Eric Blake