[Libvir] XML escaping patch

[Daniel P. Berrange]

On Fri, Jul 06, 2007 at 09:49:46AM -0400, Daniel Veillard wrote:
>   This is related to bug #206653
>     https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=206653
> basically we are very optimistic when generating the XML files, and
> sometimes this can break. The most common case is if some string inherited
> from the user input or some other config file embeds one of > < or & ,
> another one would be if the strings are containing character outside of 
> ACSII range and not encoded in UTF-8. We can at least cope with the
> easy case of escaping the 3 characters.
> This patch adds a simple buffer printing routing working with a simple
> string argument, and use it for the 2 cases where I think it's most likely
> to be needed i.e. cmdline and bootloader_args. There is a number of places
> where paths are used and the user might use weird character names, but since
> those cases can't be handled properly (you can't change that path or try
> to convert encoding on the fly since we can't guess reliably which one is
> used) I didn't tried to change those.
> This makes for a relatively simple patch which should IMHO cover most case
> where we may break while we really should not.

To be honest the whole way of building up XML documents through string
concatenation is rather disgusting. Isn't there some simple API that we
could use to build up based on logical elements, attributes & text...

> diff -u -p -r1.126 xend_internal.c
> --- src/xend_internal.c	5 Jul 2007 16:04:12 -0000	1.126
> +++ src/xend_internal.c	6 Jul 2007 13:31:34 -0000
> @@ -1337,7 +1337,7 @@ xend_parse_sexp_desc_os(virConnectPtr xe
>             virBufferVSprintf(buf, "    <root>%s</root>\n", tmp);
>          tmp = sexpr_node(node, "domain/image/linux/args");
>          if ((tmp != NULL) && (tmp[0] != 0))
> -           virBufferVSprintf(buf, "    <cmdline>%s</cmdline>\n", tmp);
> +           virBufferEscapeString(buf, "    <cmdline>%s</cmdline>\n", tmp);
>      }
>  
>      virBufferAdd(buf, "  </os>\n", 8);
> @@ -1423,7 +1423,7 @@ xend_parse_sexp_desc(virConnectPtr conn,
>          /*
>           * Only insert bootloader_args if there is also a bootloader param
>           */
> -        virBufferVSprintf(&buf, "  <bootloader_args>%s</bootloader_args>\n", tmp);
> +        virBufferEscapeString(&buf, "  <bootloader_args>%s</bootloader_args>\n", tmp);
>      }

So instead of that we could do

    dom =  xmlRootElement("domain")
    xmlAttributeInt(dom, "id", id);
    xmlAttributeString(dom, "type", "xen");

    os = xmlElement(dom, "os")
    xmlElementString(os, "bootloader", tmp)
  
    char *doc = xmlString(dom);
    xmlFree(dom);
    return doc;

Of course this is a much large change than the one you're suggesting. We
surely need to fixup much more than the two 'cmdline' and 'bootloader_args'
elements that you describe - practically any element / attribute which
takes a %s format string needs this fix.

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|