Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1uN2bA-00AMyi-0n for pgsql-bugs@arkaria.postgresql.org; Thu, 05 Jun 2025 04:53:04 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1uN2b7-003lkA-NV for pgsql-bugs@arkaria.postgresql.org; Thu, 05 Jun 2025 04:53:02 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1uN2b6-003lk2-On for pgsql-bugs@lists.postgresql.org; Thu, 05 Jun 2025 04:53:02 +0000 Received: from fhigh-a8-smtp.messagingengine.com ([103.168.172.159]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1uN2b4-000INZ-1F for pgsql-bugs@lists.postgresql.org; Thu, 05 Jun 2025 04:53:00 +0000 Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfhigh.phl.internal (Postfix) with ESMTP id 29D30114026F; Thu, 5 Jun 2025 00:52:57 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-09.internal (MEProxy); Thu, 05 Jun 2025 00:52:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paquier.xyz; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1749099177; x=1749185577; bh=SdkG9HZKFT cF+EDwZWZ6ZCJZvIVkluEjqrjh8SaYRLo=; b=L0jWFLp+0/lj4UWuSLYph4vTpj v8wGrcF4b5H1r9hN3Dezd4AHteWoaoxXV41XGQ6bn1nQri9txBtj80qq5bA4McA/ cz3ELbprAPkK4jXziY9uQcFLBg5ecsRcFwrKbyDNCcU/EhBT6CWMyKUDcIiDEbA6 +Q2K4EvZzHjbdOz5Yhg3VxIym6lF+q3x0FouFfMvikEuV0xtl56OhIgciOsPwOCU lioJMVvwd2Xye/V0I9IYcZcGzD9Trnz9uhzaUp/yxBK7OvNwWGKfItF8ONY45U5N zljWTRsZacvc5gErWC1aRsbZIvWKZx3B3EgX1xn9GoenttDrDf3/xdYDTBKw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1749099177; x=1749185577; bh=SdkG9HZKFTcF+EDwZWZ6ZCJZvIVkluEjqrj h8SaYRLo=; b=B9dafBnizutVkOZOXQl79YNJ9RyyrRSzid0ePRq25pjMVa6eDOR UKXK0AmImbDXKjVlQZLawXvuM4zgY3g0xijTqe2/C95TSVba1NQ9P7A4f+uMmHzy rhUPA3lS7j8FKfGGm16CyS9+9uSl4bXXKz8jg2T3D1r/Uyvc5ydaXTb+4amB4piA 5o6A0wIVfIJISckPhsSPY/9ThvyShQXMeSaTCcoXZZhj/dgYZT+c15Fpqalieof7 H5x7nd71849uaw7N2/umBZI5XMI+tOc0LKoyxVlV1p2LXj5nkGZd0By/ZjeBsi2Q yvmdkdo2hAeIyJE1SsI1dOHfmw+doGhONmA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtddugdefudehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucgfrhhlucfvnfffucdlfe ehmdenucfjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefo ihgthhgrvghlucfrrghquhhivghruceomhhitghhrggvlhesphgrqhhuihgvrhdrgiihii eqnecuggftrfgrthhtvghrnhepgeffjeevgfevuddvjedtvddtieejheduueelvddufedt gfefjedvkeevkeeivddvnecuffhomhgrihhnpehpohhsthhgrhgvshhqlhdrohhrghenuc evlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmihgthhgr vghlsehprghquhhivghrrdighiiipdhnsggprhgtphhtthhopeefpdhmohguvgepshhmth hpohhuthdprhgtphhtthhopehtghhlsehsshhsrdhpghhhrdhprgdruhhspdhrtghpthht ohepmhgrrhgrlhhishhtkeeisehmrghilhdrrhhupdhrtghpthhtohepphhgshhqlhdqsg hughhssehlihhsthhsrdhpohhsthhgrhgvshhqlhdrohhrgh X-ME-Proxy: Feedback-ID: i0fe9450f:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 5 Jun 2025 00:52:54 -0400 (EDT) Date: Thu, 5 Jun 2025 13:52:46 +0900 From: Michael Paquier To: Tom Lane Cc: maralist86@mail.ru, pgsql-bugs@lists.postgresql.org Subject: Re: BUG #18943: Return value of a function 'xmlBufferCreate' is dereferenced at xpath.c:177 without checking for NUL Message-ID: References: <18943-2f2a04ab03904598@postgresql.org> <861593.1748970933@sss.pgh.pa.us> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="jdrhqCFVr2yckIe9" Content-Disposition: inline In-Reply-To: <861593.1748970933@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --jdrhqCFVr2yckIe9 Content-Type: multipart/mixed; boundary="hb6OaVVHNm7Gosu2" Content-Disposition: inline --hb6OaVVHNm7Gosu2 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 03, 2025 at 01:15:33PM -0400, Tom Lane wrote: > Thanks for taking this on --- contrib/xml2 is really a mess so far as > error handling goes. Your patch looks like an improvement, although > I do have one concern: the routines in xml.c that use an xmlerrcxt > seem to check xmlerrcxt->err_occurred pretty frequently, eg xmlStrdup > is used like this: >=20 > doc->encoding =3D xmlStrdup((const xmlChar *) "UTF-8"); > if (doc->encoding =3D=3D NULL || xmlerrcxt->err_occurred) > xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, > "could not allocate XML document"); >=20 > Not sure if that's needed here. Yes, I was also wondering a bit about this part a couple of days ago while looking at the module's code and xml.c. Looking at cacd42d62cb2 and its thread (particularly [1]), I think that we need to bite the bullet or we are going to miss some error context. PgXmlErrorContext can remain private in xml.c as we can reuse pg_xml_error_occurred() to do the job for out-of-core code. =20 > There's more that could be looked at, if you feel like it: Well, now that you mention these things, I do feel like it while I have my hands on this area of the code. > xml_encode_special_chars seems to need a PG_TRY block to avoid > possibly leaking "tt". I also wonder if it's really not possible > for xmlEncodeSpecialChars to fail and return NULL. (xmltext() > doesn't believe that either, but maybe it's wrong too.) Oh, missed this call. xmlEncodeSpecialChars() relies on xmlEscapeText(), which can return NULL on allocation failures based on a check in the upstream code (first argument of the routine is not used, only second is). So xmltext() and xml_encode_special_chars() are both wrong; we need a try/catch block for the edge case where cstring_to_text_with_len() or cstring_to_text() could fail an allocation or we would leak what could have been allocated by libxml. > The usage of cleanup_workspace seems quite a mess: one caller > uses a PG_TRY block to ensure it's called, but the rest don't. > I also find it confusing that pgxml_xpath returns a value that is > also referenced in the workspace and cleanup_workspace is responsible > for freeing. I wonder if there's a better way to do that. Yes, that's not optimal. This comes down to the fact that we need workspace->res to not be free'd by libxml until the result of these SQL functions is generated, and even with my previous patch we could leak it if one of the allocations done for the function results fail. I've been wondering about a few approaches, like adding the error context to the workspace, but that did not feel right as we require the error context before the try/catch block, and the workspace and its internals allocated by libxml need to be fully handled in the scope of the try/catch. So I've finished with the attached, where the workspace and its cleanup routine gain volatile declarations, keeping pg_xml_done() outside the workspace logic. This is a rather mechanical change if done this way with the try/catch blocks moved one level higher, but as long as we need to hold on the result inside the workspace, I'm feeling that this is a cleaner approach in the long-run for xml2, because it's impossible to miss what's in the scope of the catch cleanup with the cleanup policy enforced in the definition of cleanup_workspace(). > In the end of xslt_process, I wonder why >=20 > if (resstr) > xmlFree((xmlChar *) resstr); >=20 > isn't done before the pg_xml_done call. Good point. Fixed. All that is rather unlikely going to be a problem in practice, so I don't really feel a strong reason to backpatch any of that. v18 would be OK, but we could just also wait for v19 based on how these are unlikely going to be problems in the field. Or it's worth worrying about a backpatch of the xml.c code paths which are in the core backend? The xmltext() case is isolated, so this part is not invasive. [1]: https://www.postgresql.org/message-id/23388.1311118974%40sss.pgh.pa.us -- Michael --hb6OaVVHNm7Gosu2 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v2-0001-xml2-Fix-error-handling-in-corner-cases.patch" Content-Transfer-Encoding: quoted-printable =46rom aab636ac1f757b5d3b57cf71ff8597142da79e08 Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Thu, 5 Jun 2025 13:50:21 +0900 Subject: [PATCH v2] xml2: Fix error handling in corner cases --- src/backend/utils/adt/xml.c | 32 ++- contrib/xml2/xpath.c | 413 ++++++++++++++++++++++++------------ contrib/xml2/xslt_proc.c | 26 ++- 3 files changed, 316 insertions(+), 155 deletions(-) diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index a4150bff2eae..828ba64118b0 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -529,14 +529,36 @@ xmltext(PG_FUNCTION_ARGS) #ifdef USE_LIBXML text *arg =3D PG_GETARG_TEXT_PP(0); text *result; - xmlChar *xmlbuf =3D NULL; + volatile xmlChar *xmlbuf =3D NULL; + PgXmlErrorContext *xmlerrcxt; =20 - xmlbuf =3D xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg)); + /* Otherwise, we gotta spin up some error handling. */ + xmlerrcxt =3D pg_xml_init(PG_XML_STRICTNESS_ALL); =20 - Assert(xmlbuf); + PG_TRY(); + { + xmlbuf =3D xmlEncodeSpecialChars(NULL, xml_text2xmlChar(arg)); + + if (xmlbuf =3D=3D NULL || xmlerrcxt->err_occurred) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate xmlChar"); + + result =3D cstring_to_text_with_len((const char *) xmlbuf, + xmlStrlen((const xmlChar *) xmlbuf)); + } + PG_CATCH(); + { + if (xmlbuf) + xmlFree((xmlChar *) xmlbuf); + + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); + + xmlFree((xmlChar *) xmlbuf); + pg_xml_done(xmlerrcxt, false); =20 - result =3D cstring_to_text_with_len((const char *) xmlbuf, xmlStrlen(xmlb= uf)); - xmlFree(xmlbuf); PG_RETURN_XML_P(result); #else NO_XML_SUPPORT(); diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c index 23d3f332dbaa..13c4409cf888 100644 --- a/contrib/xml2/xpath.c +++ b/contrib/xml2/xpath.c @@ -51,10 +51,10 @@ static text *pgxml_result_to_text(xmlXPathObjectPtr res= , xmlChar *toptag, =20 static xmlChar *pgxml_texttoxmlchar(text *textstring); =20 -static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath, - xpath_workspace *workspace); +static xpath_workspace *pgxml_xpath(text *document, xmlChar *xpath, + PgXmlErrorContext *xmlerrcxt); =20 -static void cleanup_workspace(xpath_workspace *workspace); +static void cleanup_workspace(volatile xpath_workspace *workspace); =20 =20 /* @@ -89,18 +89,40 @@ xml_encode_special_chars(PG_FUNCTION_ARGS) { text *tin =3D PG_GETARG_TEXT_PP(0); text *tout; - xmlChar *ts, - *tt; + volatile xmlChar *tt =3D NULL; + PgXmlErrorContext *xmlerrcxt; =20 - ts =3D pgxml_texttoxmlchar(tin); + xmlerrcxt =3D pg_xml_init(PG_XML_STRICTNESS_ALL); =20 - tt =3D xmlEncodeSpecialChars(NULL, ts); + PG_TRY(); + { + xmlChar *ts; =20 - pfree(ts); + ts =3D pgxml_texttoxmlchar(tin); =20 - tout =3D cstring_to_text((char *) tt); + tt =3D xmlEncodeSpecialChars(NULL, ts); + if (tt =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate xmlChar"); + pfree(ts); =20 - xmlFree(tt); + tout =3D cstring_to_text((char *) tt); + } + PG_CATCH(); + { + if (tt !=3D NULL) + xmlFree((xmlChar *) tt); + + pg_xml_done(xmlerrcxt, true); + + PG_RE_THROW(); + } + PG_END_TRY(); + + if (tt !=3D NULL) + xmlFree((xmlChar *) tt); + + pg_xml_done(xmlerrcxt, false); =20 PG_RETURN_TEXT_P(tout); } @@ -122,62 +144,90 @@ pgxmlNodeSetToText(xmlNodeSetPtr nodeset, xmlChar *septagname, xmlChar *plainsep) { - xmlBufferPtr buf; + volatile xmlBufferPtr buf =3D NULL; xmlChar *result; int i; + PgXmlErrorContext *xmlerrcxt; =20 - buf =3D xmlBufferCreate(); + /* spin some error handling */ + xmlerrcxt =3D pg_xml_init(PG_XML_STRICTNESS_ALL); =20 - if ((toptagname !=3D NULL) && (xmlStrlen(toptagname) > 0)) + PG_TRY(); { - xmlBufferWriteChar(buf, "<"); - xmlBufferWriteCHAR(buf, toptagname); - xmlBufferWriteChar(buf, ">"); - } - if (nodeset !=3D NULL) - { - for (i =3D 0; i < nodeset->nodeNr; i++) + buf =3D xmlBufferCreate(); + + if (buf =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate xmlBuffer"); + + if ((toptagname !=3D NULL) && (xmlStrlen(toptagname) > 0)) { - if (plainsep !=3D NULL) + xmlBufferWriteChar(buf, "<"); + xmlBufferWriteCHAR(buf, toptagname); + xmlBufferWriteChar(buf, ">"); + } + if (nodeset !=3D NULL) + { + for (i =3D 0; i < nodeset->nodeNr; i++) { - xmlBufferWriteCHAR(buf, - xmlXPathCastNodeToString(nodeset->nodeTab[i])); - - /* If this isn't the last entry, write the plain sep. */ - if (i < (nodeset->nodeNr) - 1) - xmlBufferWriteChar(buf, (char *) plainsep); - } - else - { - if ((septagname !=3D NULL) && (xmlStrlen(septagname) > 0)) + if (plainsep !=3D NULL) { - xmlBufferWriteChar(buf, "<"); - xmlBufferWriteCHAR(buf, septagname); - xmlBufferWriteChar(buf, ">"); + xmlBufferWriteCHAR(buf, + xmlXPathCastNodeToString(nodeset->nodeTab[i])); + + /* If this isn't the last entry, write the plain sep. */ + if (i < (nodeset->nodeNr) - 1) + xmlBufferWriteChar(buf, (char *) plainsep); } - xmlNodeDump(buf, - nodeset->nodeTab[i]->doc, - nodeset->nodeTab[i], - 1, 0); - - if ((septagname !=3D NULL) && (xmlStrlen(septagname) > 0)) + else { - xmlBufferWriteChar(buf, ""); + if ((septagname !=3D NULL) && (xmlStrlen(septagname) > 0)) + { + xmlBufferWriteChar(buf, "<"); + xmlBufferWriteCHAR(buf, septagname); + xmlBufferWriteChar(buf, ">"); + } + xmlNodeDump(buf, + nodeset->nodeTab[i]->doc, + nodeset->nodeTab[i], + 1, 0); + + if ((septagname !=3D NULL) && (xmlStrlen(septagname) > 0)) + { + xmlBufferWriteChar(buf, ""); + } } } } - } =20 - if ((toptagname !=3D NULL) && (xmlStrlen(toptagname) > 0)) - { - xmlBufferWriteChar(buf, ""); + if ((toptagname !=3D NULL) && (xmlStrlen(toptagname) > 0)) + { + xmlBufferWriteChar(buf, ""); + } + + result =3D xmlStrdup(buf->content); + if (result =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); } - result =3D xmlStrdup(buf->content); + PG_CATCH(); + { + if (buf) + xmlBufferFree(buf); + + pg_xml_done(xmlerrcxt, true); + + PG_RE_THROW(); + } + PG_END_TRY(); + xmlBufferFree(buf); + pg_xml_done(xmlerrcxt, false); + return result; } =20 @@ -208,16 +258,29 @@ xpath_nodeset(PG_FUNCTION_ARGS) xmlChar *septag =3D pgxml_texttoxmlchar(PG_GETARG_TEXT_PP(3)); xmlChar *xpath; text *xpres; - xmlXPathObjectPtr res; - xpath_workspace workspace; + volatile xpath_workspace *workspace; + PgXmlErrorContext *xmlerrcxt; =20 xpath =3D pgxml_texttoxmlchar(xpathsupp); + xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); =20 - res =3D pgxml_xpath(document, xpath, &workspace); + PG_TRY(); + { + workspace =3D pgxml_xpath(document, xpath, xmlerrcxt); + xpres =3D pgxml_result_to_text(workspace->res, toptag, septag, NULL); + } + PG_CATCH(); + { + if (workspace) + cleanup_workspace(workspace); =20 - xpres =3D pgxml_result_to_text(res, toptag, septag, NULL); + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); =20 - cleanup_workspace(&workspace); + cleanup_workspace(workspace); + pg_xml_done(xmlerrcxt, false); =20 pfree(xpath); =20 @@ -240,16 +303,29 @@ xpath_list(PG_FUNCTION_ARGS) xmlChar *plainsep =3D pgxml_texttoxmlchar(PG_GETARG_TEXT_PP(2)); xmlChar *xpath; text *xpres; - xmlXPathObjectPtr res; - xpath_workspace workspace; + volatile xpath_workspace *workspace; + PgXmlErrorContext *xmlerrcxt; =20 xpath =3D pgxml_texttoxmlchar(xpathsupp); + xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); =20 - res =3D pgxml_xpath(document, xpath, &workspace); + PG_TRY(); + { + workspace =3D pgxml_xpath(document, xpath, xmlerrcxt); + xpres =3D pgxml_result_to_text(workspace->res, NULL, NULL, plainsep); + } + PG_CATCH(); + { + if (workspace) + cleanup_workspace(workspace); =20 - xpres =3D pgxml_result_to_text(res, NULL, NULL, plainsep); + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); =20 - cleanup_workspace(&workspace); + cleanup_workspace(workspace); + pg_xml_done(xmlerrcxt, false); =20 pfree(xpath); =20 @@ -269,8 +345,8 @@ xpath_string(PG_FUNCTION_ARGS) xmlChar *xpath; int32 pathsize; text *xpres; - xmlXPathObjectPtr res; - xpath_workspace workspace; + volatile xpath_workspace *workspace; + PgXmlErrorContext *xmlerrcxt; =20 pathsize =3D VARSIZE_ANY_EXHDR(xpathsupp); =20 @@ -286,11 +362,24 @@ xpath_string(PG_FUNCTION_ARGS) xpath[pathsize + 7] =3D ')'; xpath[pathsize + 8] =3D '\0'; =20 - res =3D pgxml_xpath(document, xpath, &workspace); + xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); =20 - xpres =3D pgxml_result_to_text(res, NULL, NULL, NULL); + PG_TRY(); + { + workspace =3D pgxml_xpath(document, xpath, xmlerrcxt); + xpres =3D pgxml_result_to_text(workspace->res, NULL, NULL, NULL); + } + PG_CATCH(); + { + if (workspace) + cleanup_workspace(workspace); =20 - cleanup_workspace(&workspace); + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); + + cleanup_workspace(workspace); =20 pfree(xpath); =20 @@ -308,24 +397,38 @@ xpath_number(PG_FUNCTION_ARGS) text *document =3D PG_GETARG_TEXT_PP(0); text *xpathsupp =3D PG_GETARG_TEXT_PP(1); /* XPath expression */ xmlChar *xpath; - float4 fRes; - xmlXPathObjectPtr res; - xpath_workspace workspace; + float4 fRes =3D 0.0; + bool isNull =3D false; + volatile xpath_workspace *workspace =3D NULL; + PgXmlErrorContext *xmlerrcxt; =20 xpath =3D pgxml_texttoxmlchar(xpathsupp); + xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); =20 - res =3D pgxml_xpath(document, xpath, &workspace); + PG_TRY(); + { + workspace =3D pgxml_xpath(document, xpath, xmlerrcxt); + pfree(xpath); =20 - pfree(xpath); + if (workspace->res =3D=3D NULL) + isNull =3D true; + else + fRes =3D xmlXPathCastToNumber(workspace->res); + } + PG_CATCH(); + { + if (workspace) + cleanup_workspace(workspace); =20 - if (res =3D=3D NULL) - PG_RETURN_NULL(); + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); =20 - fRes =3D xmlXPathCastToNumber(res); + cleanup_workspace(workspace); + pg_xml_done(xmlerrcxt, false); =20 - cleanup_workspace(&workspace); - - if (xmlXPathIsNaN(fRes)) + if (isNull || xmlXPathIsNaN(fRes)) PG_RETURN_NULL(); =20 PG_RETURN_FLOAT4(fRes); @@ -341,21 +444,34 @@ xpath_bool(PG_FUNCTION_ARGS) text *xpathsupp =3D PG_GETARG_TEXT_PP(1); /* XPath expression */ xmlChar *xpath; int bRes; - xmlXPathObjectPtr res; - xpath_workspace workspace; + volatile xpath_workspace *workspace =3D NULL; + PgXmlErrorContext *xmlerrcxt; =20 xpath =3D pgxml_texttoxmlchar(xpathsupp); + xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); =20 - res =3D pgxml_xpath(document, xpath, &workspace); + PG_TRY(); + { + workspace =3D pgxml_xpath(document, xpath, xmlerrcxt); + pfree(xpath); =20 - pfree(xpath); + if (workspace->res =3D=3D NULL) + bRes =3D 0; + else + bRes =3D xmlXPathCastToBoolean(workspace->res); + } + PG_CATCH(); + { + if (workspace) + cleanup_workspace(workspace); =20 - if (res =3D=3D NULL) - PG_RETURN_BOOL(false); + pg_xml_done(xmlerrcxt, true); + PG_RE_THROW(); + } + PG_END_TRY(); =20 - bRes =3D xmlXPathCastToBoolean(res); - - cleanup_workspace(&workspace); + cleanup_workspace(workspace); + pg_xml_done(xmlerrcxt, false); =20 PG_RETURN_BOOL(bRes); } @@ -364,62 +480,44 @@ xpath_bool(PG_FUNCTION_ARGS) =20 /* Core function to evaluate XPath query */ =20 -static xmlXPathObjectPtr -pgxml_xpath(text *document, xmlChar *xpath, xpath_workspace *workspace) +static xpath_workspace * +pgxml_xpath(text *document, xmlChar *xpath, PgXmlErrorContext *xmlerrcxt) { int32 docsize =3D VARSIZE_ANY_EXHDR(document); - PgXmlErrorContext *xmlerrcxt; xmlXPathCompExprPtr comppath; + xpath_workspace *workspace =3D (xpath_workspace *) + palloc0(sizeof(xpath_workspace)); =20 workspace->doctree =3D NULL; workspace->ctxt =3D NULL; workspace->res =3D NULL; =20 - xmlerrcxt =3D pgxml_parser_init(PG_XML_STRICTNESS_LEGACY); - - PG_TRY(); + workspace->doctree =3D xmlReadMemory((char *) VARDATA_ANY(document), + docsize, NULL, NULL, + XML_PARSE_NOENT); + if (workspace->doctree !=3D NULL) { - workspace->doctree =3D xmlReadMemory((char *) VARDATA_ANY(document), - docsize, NULL, NULL, - XML_PARSE_NOENT); - if (workspace->doctree !=3D NULL) - { - workspace->ctxt =3D xmlXPathNewContext(workspace->doctree); - workspace->ctxt->node =3D xmlDocGetRootElement(workspace->doctree); + workspace->ctxt =3D xmlXPathNewContext(workspace->doctree); + workspace->ctxt->node =3D xmlDocGetRootElement(workspace->doctree); =20 - /* compile the path */ - comppath =3D xmlXPathCtxtCompile(workspace->ctxt, xpath); - if (comppath =3D=3D NULL) - xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, + /* compile the path */ + comppath =3D xmlXPathCtxtCompile(workspace->ctxt, xpath); + if (comppath =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, "XPath Syntax Error"); =20 - /* Now evaluate the path expression. */ - workspace->res =3D xmlXPathCompiledEval(comppath, workspace->ctxt); + /* Now evaluate the path expression. */ + workspace->res =3D xmlXPathCompiledEval(comppath, workspace->ctxt); =20 - xmlXPathFreeCompExpr(comppath); - } + xmlXPathFreeCompExpr(comppath); } - PG_CATCH(); - { - cleanup_workspace(workspace); =20 - pg_xml_done(xmlerrcxt, true); - - PG_RE_THROW(); - } - PG_END_TRY(); - - if (workspace->res =3D=3D NULL) - cleanup_workspace(workspace); - - pg_xml_done(xmlerrcxt, false); - - return workspace->res; + return workspace; } =20 /* Clean up after processing the result of pgxml_xpath() */ static void -cleanup_workspace(xpath_workspace *workspace) +cleanup_workspace(volatile xpath_workspace *workspace) { if (workspace->res) xmlXPathFreeObject(workspace->res); @@ -438,34 +536,59 @@ pgxml_result_to_text(xmlXPathObjectPtr res, xmlChar *septag, xmlChar *plainsep) { - xmlChar *xpresstr; + volatile xmlChar *xpresstr =3D NULL; + PgXmlErrorContext *xmlerrcxt; text *xpres; =20 if (res =3D=3D NULL) return NULL; =20 - switch (res->type) + /* spin some error handling */ + xmlerrcxt =3D pg_xml_init(PG_XML_STRICTNESS_ALL); + + PG_TRY(); { - case XPATH_NODESET: - xpresstr =3D pgxmlNodeSetToText(res->nodesetval, - toptag, - septag, plainsep); - break; + switch (res->type) + { + case XPATH_NODESET: + xpresstr =3D pgxmlNodeSetToText(res->nodesetval, + toptag, + septag, plainsep); + break; =20 - case XPATH_STRING: - xpresstr =3D xmlStrdup(res->stringval); - break; + case XPATH_STRING: + xpresstr =3D xmlStrdup(res->stringval); + if (xpresstr =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); + break; =20 - default: - elog(NOTICE, "unsupported XQuery result: %d", res->type); - xpresstr =3D xmlStrdup((const xmlChar *) ""); + default: + elog(NOTICE, "unsupported XQuery result: %d", res->type); + xpresstr =3D xmlStrdup((const xmlChar *) ""); + if (xpresstr =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); + } + + /* Now convert this result back to text */ + xpres =3D cstring_to_text((char *) xpresstr); } + PG_CATCH(); + { + if (xpresstr !=3D NULL) + xmlFree((xmlChar *) xpresstr); =20 - /* Now convert this result back to text */ - xpres =3D cstring_to_text((char *) xpresstr); + pg_xml_done(xmlerrcxt, true); + + PG_RE_THROW(); + } + PG_END_TRY(); =20 /* Free various storage */ - xmlFree(xpresstr); + xmlFree((xmlChar *) xpresstr); + + pg_xml_done(xmlerrcxt, false); =20 return xpres; } @@ -652,7 +775,7 @@ xpath_table(PG_FUNCTION_ARGS) =20 /* compile the path */ comppath =3D xmlXPathCtxtCompile(ctxt, xpaths[j]); - if (comppath =3D=3D NULL) + if (comppath =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, "XPath Syntax Error"); @@ -671,6 +794,10 @@ xpath_table(PG_FUNCTION_ARGS) rownr < res->nodesetval->nodeNr) { resstr =3D xmlXPathCastNodeToString(res->nodesetval->nodeTab[row= nr]); + if (resstr =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, + ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); had_values =3D true; } else @@ -680,11 +807,19 @@ xpath_table(PG_FUNCTION_ARGS) =20 case XPATH_STRING: resstr =3D xmlStrdup(res->stringval); + if (resstr =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, + ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); break; =20 default: elog(NOTICE, "unsupported XQuery result: %d", res->type); resstr =3D xmlStrdup((const xmlChar *) ""); + if (resstr =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) + xml_ereport(xmlerrcxt, + ERROR, ERRCODE_OUT_OF_MEMORY, + "could not allocate result"); } =20 /* diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c index b720d89f754a..c8e7dd45ed5b 100644 --- a/contrib/xml2/xslt_proc.c +++ b/contrib/xml2/xslt_proc.c @@ -58,7 +58,7 @@ xslt_process(PG_FUNCTION_ARGS) volatile xsltSecurityPrefsPtr xslt_sec_prefs =3D NULL; volatile xsltTransformContextPtr xslt_ctxt =3D NULL; volatile int resstat =3D -1; - xmlChar *resstr =3D NULL; + volatile xmlChar *resstr =3D NULL; int reslen =3D 0; =20 if (fcinfo->nargs =3D=3D 3) @@ -86,7 +86,7 @@ xslt_process(PG_FUNCTION_ARGS) VARSIZE_ANY_EXHDR(doct), NULL, NULL, XML_PARSE_NOENT); =20 - if (doctree =3D=3D NULL) + if (doctree =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT, "error parsing XML document"); =20 @@ -95,14 +95,14 @@ xslt_process(PG_FUNCTION_ARGS) VARSIZE_ANY_EXHDR(ssheet), NULL, NULL, XML_PARSE_NOENT); =20 - if (ssdoc =3D=3D NULL) + if (ssdoc =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT, "error parsing stylesheet as XML document"); =20 /* After this call we need not free ssdoc separately */ stylesheet =3D xsltParseStylesheetDoc(ssdoc); =20 - if (stylesheet =3D=3D NULL) + if (stylesheet =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, "failed to parse stylesheet"); =20 @@ -137,11 +137,15 @@ xslt_process(PG_FUNCTION_ARGS) restree =3D xsltApplyStylesheetUser(stylesheet, doctree, params, NULL, NULL, xslt_ctxt); =20 - if (restree =3D=3D NULL) + if (restree =3D=3D NULL || pg_xml_error_occurred(xmlerrcxt)) xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY, "failed to apply stylesheet"); =20 - resstat =3D xsltSaveResultToString(&resstr, &reslen, restree, stylesheet= ); + resstat =3D xsltSaveResultToString((xmlChar **) &resstr, &reslen, + restree, stylesheet); + + if (resstat >=3D 0) + result =3D cstring_to_text_with_len((char *) resstr, reslen); } PG_CATCH(); { @@ -155,6 +159,8 @@ xslt_process(PG_FUNCTION_ARGS) xsltFreeStylesheet(stylesheet); if (doctree !=3D NULL) xmlFreeDoc(doctree); + if (resstr !=3D NULL) + xmlFree((xmlChar *) resstr); xsltCleanupGlobals(); =20 pg_xml_done(xmlerrcxt, true); @@ -170,17 +176,15 @@ xslt_process(PG_FUNCTION_ARGS) xmlFreeDoc(doctree); xsltCleanupGlobals(); =20 + if (resstr) + xmlFree((xmlChar *) resstr); + pg_xml_done(xmlerrcxt, false); =20 /* XXX this is pretty dubious, really ought to throw error instead */ if (resstat < 0) PG_RETURN_NULL(); =20 - result =3D cstring_to_text_with_len((char *) resstr, reslen); - - if (resstr) - xmlFree(resstr); - PG_RETURN_TEXT_P(result); #else /* !USE_LIBXSLT */ =20 --=20 2.49.0 --hb6OaVVHNm7Gosu2-- --jdrhqCFVr2yckIe9 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmhBIp4ACgkQnvQgOdby QH24Zg//aYHcERjZ6LKg+YABbiN9h2OtN6wzecvH/pTVOR8bwUio9HLuqO5K8FNU QWIbTPueEW9yS/HoymiQ9pjl0tm4QWxEOw7Zj2WBn4aIeNyO53Ox/ZExPrvT+ITU lcv6yY9Uj9xGq4uZREDt+R94T58IRQ5GO7jjd4WFNYYkUQNYBG7xvSCLVeFr76AK alRwT/cSwPgO4hHIlx5+HMQBlZSy5JNhrPccJgXete2do0ntQKByoe3WJohfuCUh FhUCRhwjlnOzIQOVIutzUY7YdHikeH/xesrEEO2yiGGPwtywEmlOFpiqxLhXEMTv aA8PXc1WnqkAIzW3omwaKIszapdXIRpQsoyBfH3E5JMhkpuVpB+oN4/2U9MrHL1s smFf1SrIC1zrT0knlyBoG7DR7j5S617vum4Z/twIlFj5FXdxn58rydKBDVwX8Hrj rC1fJ147EpHjzVqKtoPPoSbjgzk5OH/QA+TGnVRcDJ/Sja5kKRzp16/DAMPv6LFz 2kgxZJLEmUKcyqFA5ahfrSGe1uWKJKZz69FzOXcmDkwNewkDMZ9c3Tvfmg3aTTEj ZCHgnCz2//Ga/fpCjQ2XSiddD14glv7+QGMEwUiL3za2B5iuQs6pHOKUQnqTzdsO YxrylW4ToBrvLK70eqTngIrxDpjEIN2XoFE0WrbGWbSCTrXSlI0= =/91n -----END PGP SIGNATURE----- --jdrhqCFVr2yckIe9--