[libvirt] [PATCH] Forbid new-line char in name of networks

Michal Privoznik mprivozn at redhat.com
Mon Oct 10 02:53:15 UTC 2016


On 08.10.2016 23:38, Sławek Kapłoński wrote:
> New line character in name of network is now forbidden because it
> mess virsh output and can be confusing for users.
> 
> Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=818064
> ---
>  src/conf/network_conf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index aa39776..f9c537f 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -2123,6 +2123,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>          goto error;
>      }
>  
> +    if (strchr(def->name, '\n')) {
> +        virReportError(
> +            VIR_ERR_XML_ERROR,
> +            _("name %s cannot contain new-line character"), def->name);
> +        goto error;
> +    }
> +
>      /* Extract network uuid */
>      tmp = virXPathString("string(./uuid[1])", ctxt);
>      if (!tmp) {
> 

Okay, this makes sense. We can put reasonable restrictions on object
names. However, I think while we are at this we can fix the same problem
in other areas of the code. For instance, storage pools and volumes
suffer from the same problem. Now, instead of copying this chunk all
over the places, what about doing this more wisely?

For instance:

char *c;
...
/* Parse name from XML */

if ((c = strpbrk(def->name, "/\n")) {
    /* Insert your favourite character here ^^ */
    virReportError(VIR_ERR_XML_DETAIL,
        _("invalid char in name: %c"), *c);
    return -1; /* goto cleanup */
}

This way, we can just add new character to the string when somebody
files a bug.
Moreover, if we'd wrap those couple of lines into a function, we could
just do:

if (!virXMLCheckName(def->name, "/\n"))
   return -1;

Let me know if you have any troubles writing this, or if you disagree.

Michal




More information about the libvir-list mailing list