From c4939f121563f300b98b30837151753fc8255554 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 16 Dec 2022 11:10:36 -0500 Subject: [PATCH] Clean up dubious error handling in wellformed_xml(). 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 | 64 ++++++++++--------------------------- 1 file changed, 17 insertions(+), 47 deletions(-) diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index 4b9877ee0b..1928fcc2f0 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -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