On 7/20/20 10:48 PM, Laine Stump wrote:
(I am *NOT* advocating that we apply this patch. Just providing it for
informational purposes, since we had previously discussed this
possibility on the list)
Since it's impossible to determine whether xmlNodeContent has returned
a NULL due to OOM, or due to badly formed / evil XML, this patch open
codes virXMLNodeContentString to get the content string directly from
the node.
This turns out to not be so easy as it seemed at first glance when it
was suggested - the "content" member of the element node itself does
not contain the content string for the node. The content string that
we want can be found (at least for our uses of libxml) by looking for
a child node of the element node - if that child node is of type
XML_TEXT_NODE, then the content member of *that* node is the string
we're looking for. If there is no child node, then the element has no
content, so we return "". Likewise, if the child node is type
XML_TEXT_NODE but has no content, we also return "". In all other
cases, we log an error and return because this is some case that
hasn't been encountered in our test cases, so either someone is
sending bad XML, or our assumptions about the layout of the XML node
object list are incorrect.
Note that while calling virXMLNodeContentString() would return NULL
from an OOM situation, this new code will exit the process on OOM
(since it is calling glib for memory allocation).
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---
src/util/virxml.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/src/util/virxml.c b/src/util/virxml.c
index 5315d4ff6f..b2298d74c8 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -538,7 +538,17 @@ virXMLPropStringLimit(xmlNodePtr node,
char *
virXMLNodeContentString(xmlNodePtr node)
{
- char *ret = (char *)xmlNodeGetContent(node);
+ /* We specifically avoid using virXMLNodeContentString() here, because
+ * when NULL is returned, it is difficult/impossible to
+ * distinguish between 1) OOM, 2) NULL content, 3) some other error.
+ */
s/virXMLNodeContentString/xmlNodeGetContent/
This patch makes sense to me. I'll leave it up to you whether you merge
it or not.
Michal
