Greetings,
Because of the memory bug in xmlCopyNode, this is a new patch with
different method. In this patch, instead of using xmlCopyNode to bring the
namespace back, we added the required namespaces to the node before
dumping the node to string, and cleaning it up afterwards (because the same
node could be dumped again in next xpath result).
Thanks,
Ali Akbar
2014-07-11 15:36 GMT+07:00 Ali Akbar <the.ap...@gmail.com>:
> Greetings,
>
> Attached modified patch to handle xmlCopyNode returning NULL. The patch is
> larger because xmlerrcxt must be passed to xml_xmlnodetoxmltype (xmlerrcxt
> is needed for calling xml_ereport).
>
> From libxml2 source, it turns out that the other cases that xmlCopyNode
> will return NULL will not happen. So in this patch i assume that the only
> case is memory exhaustion.
>
> But i found some bug in libxml2's code, because we call xmlCopyNode with 1
> as the second parameter, internally xmlCopyNode will call xmlStaticCopyNode
> recursively via xmlStaticCopyNodeList. And xmlStaticCopyNodeList doesn't
> check the return of xmlStaticCopyNode whether it's NULL. So if someday the
> recursion returns NULL (maybe because of memory exhaustion), it will
> SEGFAULT.
>
> Knowing this but in libxml2 code, is this patch is still acceptable?
>
> Thanks,
> Ali Akbar
>
--
Ali Akbar
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 422be69..f34c87e 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -141,9 +141,11 @@ static bool print_xml_decl(StringInfo buf, const xmlChar *version,
pg_enc encoding, int standalone);
static xmlDocPtr xml_parse(text *data, XmlOptionType xmloption_arg,
bool preserve_whitespace, int encoding);
-static text *xml_xmlnodetoxmltype(xmlNodePtr cur);
+static text *xml_xmlnodetoxmltype(xmlNodePtr cur,
+ PgXmlErrorContext *xmlerrcxt);
static int xml_xpathobjtoxmlarray(xmlXPathObjectPtr xpathobj,
- ArrayBuildState **astate);
+ ArrayBuildState **astate,
+ PgXmlErrorContext *xmlerrcxt);
#endif /* USE_LIBXML */
static StringInfo query_to_xml_internal(const char *query, char *tablename,
@@ -3590,27 +3592,109 @@ SPI_sql_row_to_xmlelement(int rownum, StringInfo result, char *tablename,
#ifdef USE_LIBXML
+/* check ns definition of node and its childrens. If any one of ns is
+ * not defined in node and it's children, but in the node's parent,
+ * copy the definition to node.
+ */
+static void
+xml_checkandcopyns(xmlNodePtr root,
+ PgXmlErrorContext *xmlerrcxt,
+ xmlNodePtr node,
+ xmlNsPtr lastns_before)
+{
+ xmlNsPtr ns = NULL;
+ xmlNsPtr cur_ns;
+ xmlNodePtr cur;
+
+ if (node->ns != NULL)
+ {
+ /* check in new nses */
+ cur_ns = lastns_before == NULL ? node->nsDef : lastns_before->next;
+ while (cur_ns != NULL)
+ {
+ if (cur_ns->href != NULL)
+ {
+ if (((cur_ns->prefix == NULL) && (node->ns->prefix == NULL)) ||
+ ((cur_ns->prefix != NULL) && (node->ns->prefix != NULL) &&
+ xmlStrEqual(cur_ns->prefix, node->ns->prefix)))
+ {
+ ns = cur_ns;
+ break;
+ }
+ }
+ cur_ns = cur_ns->next;
+ }
+ if (ns == NULL) /* not in new nses */
+ {
+ ns = xmlSearchNs(NULL, node->parent, node->ns->prefix);
+
+ if (ns != NULL) {
+ ns = xmlNewNs(root, ns->href, ns->prefix);
+
+ if (ns == NULL && xmlerrcxt->err_occurred) {
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlNs");
+ }
+ }
+ }
+ }
+ /* check and copy ns for children recursively */
+ cur = node->children;
+ while (cur != NULL) {
+ xml_checkandcopyns(root, xmlerrcxt, cur, lastns_before);
+ cur = cur->next;
+ }
+}
+
/*
* Convert XML node to text (dump subtree in case of element,
* return value otherwise)
*/
static text *
-xml_xmlnodetoxmltype(xmlNodePtr cur)
+xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt)
{
xmltype *result;
if (cur->type == XML_ELEMENT_NODE)
{
xmlBufferPtr buf;
+ xmlNsPtr lastns_before;
+ xmlNsPtr ns;
+ xmlNsPtr next;
buf = xmlBufferCreate();
+
PG_TRY();
{
+ lastns_before = cur->nsDef;
+ if (lastns_before != NULL) {
+ while (lastns_before->next != NULL) {
+ lastns_before = lastns_before->next;
+ }
+ }
+ xml_checkandcopyns(cur, xmlerrcxt, cur, lastns_before);
+
xmlNodeDump(buf, NULL, cur, 0, 1);
result = xmlBuffer_to_xmltype(buf);
+
+ /* delete and free new nses */
+ ns = lastns_before == NULL ? cur->nsDef : lastns_before->next;
+ while (ns != NULL) {
+ next = ns->next;
+ xmlFree(ns);
+ ns = next;
+ }
+ if (lastns_before == NULL) {
+ cur->nsDef = NULL;
+ } else {
+ lastns_before->next = NULL;
+ }
}
PG_CATCH();
{
+ /* new namespaces will be freed while free-ing the node, so we
+ * won't free it here
+ */
xmlBufferFree(buf);
PG_RE_THROW();
}
@@ -3656,7 +3740,8 @@ xml_xmlnodetoxmltype(xmlNodePtr cur)
*/
static int
xml_xpathobjtoxmlarray(xmlXPathObjectPtr xpathobj,
- ArrayBuildState **astate)
+ ArrayBuildState **astate,
+ PgXmlErrorContext *xmlerrcxt)
{
int result = 0;
Datum datum;
@@ -3678,7 +3763,8 @@ xml_xpathobjtoxmlarray(xmlXPathObjectPtr xpathobj,
for (i = 0; i < result; i++)
{
- datum = PointerGetDatum(xml_xmlnodetoxmltype(xpathobj->nodesetval->nodeTab[i]));
+ datum = PointerGetDatum(xml_xmlnodetoxmltype(xpathobj->nodesetval->nodeTab[i],
+ xmlerrcxt));
*astate = accumArrayResult(*astate, datum,
false, XMLOID,
CurrentMemoryContext);
@@ -3882,9 +3968,9 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
* Extract the results as requested.
*/
if (res_nitems != NULL)
- *res_nitems = xml_xpathobjtoxmlarray(xpathobj, astate);
+ *res_nitems = xml_xpathobjtoxmlarray(xpathobj, astate, xmlerrcxt);
else
- (void) xml_xpathobjtoxmlarray(xpathobj, astate);
+ (void) xml_xpathobjtoxmlarray(xpathobj, astate, xmlerrcxt);
}
PG_CATCH();
{
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 382f9df..866f299 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -584,6 +584,21 @@ SELECT xpath('//loc:piece/@id', '<local:data xmlns:local="http://127.0.0.1";><loc
{1,2}
(1 row)
+SELECT xpath('//loc:piece', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+ xpath
+------------------------------------------------------------------------------------------------------------------------------------------------
+ {"<local:piece xmlns:local=\"http://127.0.0.1\"; id=\"1\">number one</local:piece>","<local:piece xmlns:local=\"http://127.0.0.1\"; id=\"2\"/>"}
+(1 row)
+
+SELECT xpath('//loc:*', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+ xpath
+--------------------------------------------------------------------------------------------------------------------------------------------------------------
+ {"<local:data xmlns:local=\"http://127.0.0.1\";> +
+ <local:piece id=\"1\">number one</local:piece> +
+ <local:piece id=\"2\"/> +
+ </local:data>","<local:piece xmlns:local=\"http://127.0.0.1\"; id=\"1\">number one</local:piece>","<local:piece xmlns:local=\"http://127.0.0.1\"; id=\"2\"/>"}
+(1 row)
+
SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>');
xpath
-------------------------
diff --git a/src/test/regress/expected/xml_1.out b/src/test/regress/expected/xml_1.out
index a34d1f4..46aff5f 100644
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -498,6 +498,18 @@ LINE 1: SELECT xpath('//loc:piece/@id', '<local:data xmlns:local="ht...
^
DETAIL: This functionality requires the server to be built with libxml support.
HINT: You need to rebuild PostgreSQL using --with-libxml.
+SELECT xpath('//loc:piece', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+ERROR: unsupported XML feature
+LINE 1: SELECT xpath('//loc:piece', '<local:data xmlns:local="http:/...
+ ^
+DETAIL: This functionality requires the server to be built with libxml support.
+HINT: You need to rebuild PostgreSQL using --with-libxml.
+SELECT xpath('//loc:*', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+ERROR: unsupported XML feature
+LINE 1: SELECT xpath('//loc:*', '<local:data xmlns:local="http://127...
+ ^
+DETAIL: This functionality requires the server to be built with libxml support.
+HINT: You need to rebuild PostgreSQL using --with-libxml.
SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>');
ERROR: unsupported XML feature
LINE 1: SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>'...
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 90d4d67..f436f97 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -174,6 +174,8 @@ SELECT xpath(NULL, NULL) IS NULL FROM xmltest;
SELECT xpath('', '<!-- error -->');
SELECT xpath('//text()', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>');
SELECT xpath('//loc:piece/@id', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+SELECT xpath('//loc:piece', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
+SELECT xpath('//loc:*', '<local:data xmlns:local="http://127.0.0.1";><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>');
SELECT xpath('//text()', '<root><</root>');
SELECT xpath('//@value', '<root value="<"/>');
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
