Re: [PATCH] Strip control codes in virBufferEscapeString

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 30, 2015 at 07:06:45AM -0600, Eric Blake wrote:
> On 03/30/2015 05:02 AM, Ján Tomko wrote:
> > These cannot be represented in XML.
> 
> Yes they can, via entities.  DV would know for sure, but I think that
>  is the entity for the C byte '\1'.
> 

Only in XML 1.1.

For XML 1.0: http://www.w3.org/TR/xml/#dt-charref
  Well-formedness constraint: Legal Character

  Characters referred to using character references MUST match the
  production for Char.

Which is: http://www.w3.org/TR/xml/#NT-Char

  [2]     Char       ::=      #x9 | #xA | #xD | [#x20-#xD7FF] |
  [#xE000-#xFFFD] | [#x10000-#x10FFFF]    /* any Unicode character,
  excluding the surrogate blocks, FFFE, and FFFF. */

Both libvirt and virt-xml-validate choke on those entities
error: (domain_definition):2: xmlParseCharRef: invalid xmlChar value 1
  <name>f21&#x01;</name>

DV was the one who wrote the code to skip over control characters in
commit b36f453a581f27a4a43558978724a52df32045bb (v0.3.0~1)
  new function virBufferEscapeString() to format a string while
  escaping its content for XML, and apply it to a couple of
  obvious places, should fix bug #206653

It was the optimization in
commit 0af02cb2e8d8192958735880e135ab69beb437c5 (v0.8.6~57)
    buf: Simplify virBufferEscapeString
which broke this for strings that do have control codes,
but not escapable characters.

> > 
> > We have been stripping them, but only if the string had
> > characters that needed escaping: <>"'&
> > 
> > Extend the strcspn check to include control codes, and strip
> > them even if we don't do any escaping.
> 
> NACK.  Stripping control codes from a volume name represents the wrong
> name.  We need to escape the problematic bytes, rather than strip them.
> 
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1184131
> > https://bugzilla.redhat.com/show_bug.cgi?id=1066564
> > ---
> >  src/util/virbuffer.c | 14 +++++++++++---
> >  tests/virbuftest.c   | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 60 insertions(+), 3 deletions(-)
> 
> As there are real bugs that will be fixed once we use the correct
> entities, I'm looking forward to v2.

This fixes the real bug of libvirt generating unparsable XML, which breaks
creation of any VMs in virt-manager.

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]