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@redhat.com> --- 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