[libvirt] [PATCH] xml: omit domain name from comment if it contains double hyphen

We put a comment containing "virsh edit <domain_name>" at the start of the XML. W3C recommendation forbids the use of "--" in comments [1] and libvirt can't parse it either. This patch omits the domain name if it contains a double hyphen. [1] http://www.w3.org/TR/REC-xml/#sec-comments --- src/util/xml.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/xml.c b/src/util/xml.c index 39bc111..f3dc256 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -807,12 +807,16 @@ or other application using the libvirt API.\n\ if (safewrite(fd, cmd, len) != len) return -1; - if (safewrite(fd, " ", 1) != 1) - return -1; + /* Omit the domain name if it contains a double hyphen + * because they aren't allowed in XML comments */ + if (!strstr(name, "--")) { + if (safewrite(fd, " ", 1) != 1) + return -1; - len = strlen(name); - if (safewrite(fd, name, len) != len) - return -1; + len = strlen(name); + if (safewrite(fd, name, len) != len) + return -1; + } len = strlen(epilogue); if (safewrite(fd, epilogue, len) != len) -- 1.7.8.6

On 23.10.2012 14:16, Ján Tomko wrote:
We put a comment containing "virsh edit <domain_name>" at the start of the XML. W3C recommendation forbids the use of "--" in comments [1] and libvirt can't parse it either. This patch omits the domain name if it contains a double hyphen.
[1] http://www.w3.org/TR/REC-xml/#sec-comments --- src/util/xml.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/util/xml.c b/src/util/xml.c index 39bc111..f3dc256 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -807,12 +807,16 @@ or other application using the libvirt API.\n\ if (safewrite(fd, cmd, len) != len) return -1;
- if (safewrite(fd, " ", 1) != 1) - return -1; + /* Omit the domain name if it contains a double hyphen + * because they aren't allowed in XML comments */ + if (!strstr(name, "--")) { + if (safewrite(fd, " ", 1) != 1) + return -1;
- len = strlen(name); - if (safewrite(fd, name, len) != len) - return -1; + len = strlen(name); + if (safewrite(fd, name, len) != len) + return -1; + }
len = strlen(epilogue); if (safewrite(fd, epilogue, len) != len)
ACKed & pushed. Michal

On Tue, Oct 23, 2012 at 02:16:44PM +0200, Ján Tomko wrote:
We put a comment containing "virsh edit <domain_name>" at the start of the XML. W3C recommendation forbids the use of "--" in comments [1] and libvirt can't parse it either. This patch omits the domain name if it contains a double hyphen.
I'd really rather that we properly escaped the data rather than just dropping it. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 10/23/2012 09:31 AM, Daniel P. Berrange wrote:
On Tue, Oct 23, 2012 at 02:16:44PM +0200, Ján Tomko wrote:
We put a comment containing "virsh edit <domain_name>" at the start of the XML. W3C recommendation forbids the use of "--" in comments [1] and libvirt can't parse it either. This patch omits the domain name if it contains a double hyphen.
I'd really rather that we properly escaped the data rather than just dropping it.
Since the whole point of that comment is intended to give the user something they can paste into their shell, we could just escape it by doing: s/--/-''-/. And for that to work, we'd also need to shell-escape any other metacharacters in the domain name, so that the entire line is something that can easily be copied and pasted. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Oct 23, 2012 at 09:59:42AM -0600, Eric Blake wrote:
On 10/23/2012 09:31 AM, Daniel P. Berrange wrote:
On Tue, Oct 23, 2012 at 02:16:44PM +0200, Ján Tomko wrote:
We put a comment containing "virsh edit <domain_name>" at the start of the XML. W3C recommendation forbids the use of "--" in comments [1] and libvirt can't parse it either. This patch omits the domain name if it contains a double hyphen.
I'd really rather that we properly escaped the data rather than just dropping it.
Since the whole point of that comment is intended to give the user something they can paste into their shell, we could just escape it by doing: s/--/-''-/. And for that to work, we'd also need to shell-escape any other metacharacters in the domain name, so that the entire line is something that can easily be copied and pasted.
Or use CDATA eg <!-- <![CDATA ....any text ]]> --> Of course you still arguably need to escape any occurance of ']]>' in the domain name, but that is not really likely compared to '--' Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Ján Tomko
-
Michal Privoznik