Clean up dubious error handling in wellformed_xml().
authorTom Lane <[email protected]>
Fri, 16 Dec 2022 16:10:36 +0000 (11:10 -0500)
committerTom Lane <[email protected]>
Fri, 16 Dec 2022 16:10:40 +0000 (11:10 -0500)
This ancient bit of code was summarily trapping any ereport longjmp
whatsoever and assuming that it must represent an invalid-XML report.
It's not really appropriate to handle OOM-like situations that way:
maybe the input is valid or maybe not, but we couldn't find out.
And it'd be a seriously bad idea to ignore, say, a query cancel
error that way.  (Perhaps that can't happen because there is no
CHECK_FOR_INTERRUPTS anywhere within xml_parse, but even if that's
true today it's obviously a very fragile assumption.)

But in the wake of the previous commit, we can drop the PG_TRY
here altogether, and use the soft error mechanism to catch only
the kinds of errors that are legitimate to treat as invalid-XML.

(This is our first use of the soft error mechanism for something
not directly related to a datatype input function.  It won't be
the last.)

xml_is_document can be converted in the same way.  That one is
not actively broken, because it was checking specifically for
ERRCODE_INVALID_XML_DOCUMENT rather than trapping everything;
but the code is still shorter and probably faster this way.

Discussion: https://postgr.es/m/3564577.1671142683@sss.pgh.pa.us

src/backend/utils/adt/xml.c

index 4b9877ee0b769eca8dc68793ccfe1153174b4117..1928fcc2f04babcbc7867a01a1a9168fd252587f 100644 (file)
@@ -81,6 +81,7 @@
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "nodes/execnodes.h"
+#include "nodes/miscnodes.h"
 #include "nodes/nodeFuncs.h"
 #include "utils/array.h"
 #include "utils/builtins.h"
@@ -894,41 +895,18 @@ bool
 xml_is_document(xmltype *arg)
 {
 #ifdef USE_LIBXML
-       bool            result;
-       volatile xmlDocPtr doc = NULL;
-       MemoryContext ccxt = CurrentMemoryContext;
-
-       /* We want to catch ereport(INVALID_XML_DOCUMENT) and return false */
-       PG_TRY();
-       {
-               doc = xml_parse((text *) arg, XMLOPTION_DOCUMENT, true,
-                                               GetDatabaseEncoding(), NULL);
-               result = true;
-       }
-       PG_CATCH();
-       {
-               ErrorData  *errdata;
-               MemoryContext ecxt;
-
-               ecxt = MemoryContextSwitchTo(ccxt);
-               errdata = CopyErrorData();
-               if (errdata->sqlerrcode == ERRCODE_INVALID_XML_DOCUMENT)
-               {
-                       FlushErrorState();
-                       result = false;
-               }
-               else
-               {
-                       MemoryContextSwitchTo(ecxt);
-                       PG_RE_THROW();
-               }
-       }
-       PG_END_TRY();
+       xmlDocPtr       doc;
+       ErrorSaveContext escontext = {T_ErrorSaveContext};
 
+       /*
+        * We'll report "true" if no soft error is reported by xml_parse().
+        */
+       doc = xml_parse((text *) arg, XMLOPTION_DOCUMENT, true,
+                                       GetDatabaseEncoding(), (Node *) &escontext);
        if (doc)
                xmlFreeDoc(doc);
 
-       return result;
+       return !escontext.error_occurred;
 #else                                                  /* not USE_LIBXML */
        NO_XML_SUPPORT();
        return false;
@@ -4320,26 +4298,18 @@ xpath_exists(PG_FUNCTION_ARGS)
 static bool
 wellformed_xml(text *data, XmlOptionType xmloption_arg)
 {
-       bool            result;
-       volatile xmlDocPtr doc = NULL;
-
-       /* We want to catch any exceptions and return false */
-       PG_TRY();
-       {
-               doc = xml_parse(data, xmloption_arg, true, GetDatabaseEncoding(), NULL);
-               result = true;
-       }
-       PG_CATCH();
-       {
-               FlushErrorState();
-               result = false;
-       }
-       PG_END_TRY();
+       xmlDocPtr       doc;
+       ErrorSaveContext escontext = {T_ErrorSaveContext};
 
+       /*
+        * We'll report "true" if no soft error is reported by xml_parse().
+        */
+       doc = xml_parse(data, xmloption_arg, true,
+                                       GetDatabaseEncoding(), (Node *) &escontext);
        if (doc)
                xmlFreeDoc(doc);
 
-       return result;
+       return !escontext.error_occurred;
 }
 #endif