[libvirt] [PATCH v3] Fixed URI parsing

[Eric Blake]

On 02/24/2012 11:09 AM, Martin Kletzander wrote:
> Function xmlParseURI does not remove square brackets around IPv6
> address when parsing. One of the solutions is making wrappers around
> functions working with xmlURI*. This assures that uri->server will be
> always properly assigned and it doesn't have to be changed when used
> on some new place in the code.
> For this purpose, functions virParseURI and virSaveURI were
> added. These function are wrappers around xmlParseURI and xmlSaveUri
> respectively.
> Also there is one new syntax check function to prohibit these functions
> anywhere else.
> 

> +++ b/src/libvirt_private.syms
> @@ -1430,6 +1430,11 @@ virTypedParameterArrayValidate;
>  virTypedParameterAssign;
> 
> 
> +# viruri.h
> +virURIParse;
> +virURIFormat;

Swap these two lines.

> +xmlURIPtr
> +virURIParse(const char *uri)
> +{
> +    xmlURIPtr ret = xmlParseURI(uri);
> +
> +    /* First check: does it even make sense to jump inside */
> +    if (ret != NULL &&
> +        ret->server != NULL &&
> +        ret->server[0] == '[') {
> +        size_t length = strlen(ret->server);
> +
> +        /* We want to modify the server string only if there are
> +         * square brackets on both ends and inside there is IPv6
> +         * address. Otherwise we could make a mistake by modifying
> +         * something else than IPv6 address. */

s/else than/other than an/

> +unsigned char *
> +virURIFormat(xmlURIPtr uri)
> +{
> +    char *tmpserver = NULL, *backupserver = uri->server;

NULL deref...

> +    unsigned char *ret;
> +
> +    /* First check: does it make sense to do anything */
> +    if (uri != NULL &&

...since you allow uri == NULL on input.  Reorder the assignment to
backupserver to come after you know uri is not NULL.

> +        uri->server != NULL &&
> +        strchr(uri->server, ':') != NULL) {
> +
> +        if (virAsprintf(&tmpserver, "[%s]", uri->server) == -1)

It's more idiomatic to use '< 0', not '== -1'.

> +++ b/src/util/viruri.h
> @@ -0,0 +1,18 @@
> +/*
> + * viruri.h: internal definitions used for URI parsing.

Needs a copyright header.

ACK with those nits fixed; I think we're close enough that you can push
without having to get a review on a v4.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120224/1b0bc5ea/attachment-0001.sig>