8000 Clean up dubious error handling in wellformed_xml(). · postgrespro/postgres@c4939f1 · GitHub
[go: up one dir, main page]
Skip to content

    • Commit c4939f1

    Browse files
    tglsfdctglsfdc
    committed
    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
    1 parent 37bef84 commit c4939f1

    File tree

    1 file changed

    +17
    -47
    lines changed
    • src/backend/utils/adt

    1 file changed

    +17
    -47
    lines changed

    ‎src/backend/utils/adt/xml.c

    Lines changed: 17 additions & 47 deletions
    Original file line numberDiff line numberDiff line change
    @@ -81,6 +81,7 @@
    8181
    #include "mb/pg_wchar.h"
    8282
    #include "miscadmin.h"
    8383
    #include "nodes/execnodes.h"
    84+
    #include "nodes/miscnodes.h"
    8485
    #include "nodes/nodeFuncs.h"
    8586
    #include "utils/array.h"
    8687
    #include "utils/builtins.h"
    @@ -894,41 +895,18 @@ bool
    894895
    xml_is_document(xmltype *arg)
    895896
    {
    896897
    #ifdef USE_LIBXML
    897-
    bool result;
    898-
    volatile xmlDocPtr doc = NULL;
    899-
    MemoryContext ccxt = CurrentMemoryContext;
    900-
    901-
    /* We want to catch ereport(INVALID_XML_DOCUMENT) and return false */
    902-
    PG_TRY();
    903-
    {
    904-
    doc = xml_parse((text *) arg, XMLOPTION_DOCUMENT, true,
    905-
    GetDatabaseEncoding(), NULL);
    906-
    result = true;
    907-
    }
    908-
    PG_CATCH();
    909-
    {
    910-
    ErrorData *errdata;
    911-
    MemoryContext ecxt;
    912-
    913-
    ecxt = MemoryContextSwitchTo(ccxt);
    914-
    errdata = CopyErrorData();
    915-
    if (errdata->sqlerrcode == ERRCODE_INVALID_XML_DOCUMENT)
    916-
    {
    917-
    FlushErrorState();
    918-
    result = false;
    919-
    }
    920-
    else
    921-
    {
    922-
    MemoryContextSwitchTo(ecxt);
    923-
    PG_RE_THROW();
    924-
    }
    925-
    }
    926-
    PG_END_TRY();
    898+
    xmlDocPtr doc;
    899+
    ErrorSaveContext escontext = {T_ErrorSaveContext};
    927900

    901+
    /*
    902+
    * We'll report "true" if no soft error is reported by xml_parse().
    903+
    */
    904+
    doc = xml_parse((text *) arg, XMLOPTION_DOCUMENT, true,
    905+
    GetDatabaseEncoding(), (Node *) &escontext);
    928906
    if (doc)
    929907
    xmlFreeDoc(doc);
    930908

    931-
    return result;
    909+
    return !escontext.error_occurred;
    932910
    #else /* not USE_LIBXML */
    933911
    NO_XML_SUPPORT();
    934912
    return false;
    @@ -4320,26 +4298,18 @@ xpath_exists(PG_FUNCTION_ARGS)
    43204298
    static bool
    43214299
    wellformed_xml(text *data, XmlOptionType xmloption_arg)
    43224300
    {
    4323-
    bool result;
    4324-
    volatile xmlDocPtr doc = NULL;
    4325-
    4326-
    /* We want to catch any exceptions and return false */
    4327-
    PG_TRY();
    4328-
    {
    4329-
    doc = xml_parse(data, xmloption_arg, true, GetDatabaseEncoding(), NULL);
    4330-
    result = true;
    4331-
    }
    4332-
    PG_CATCH();
    4333-
    {
    4334-
    FlushErrorState();
    4335-
    result = false;
    4336-
    }
    4337-
    PG_END_TRY();
    4301+
    xmlDocPtr doc;
    4302+
    ErrorSaveContext escontext = {T_ErrorSaveContext};
    43384303

    4304+
    /*
    4305+
    * We'll report "true" if no soft error is reported by xml_parse().
    4306+
    */
    4307+
    doc = xml_parse(data, xmloption_arg, true,
    4308+
    GetDatabaseEncoding(), (Node *) &escontext);
    43394309
    if (doc)
    43404310
    xmlFreeDoc(doc);
    43414311

    4342-
    return result;
    4312+
    return !escontext.error_occurred;
    43434313
    }
    43444314
    #endif
    43454315

    0 commit comments

    Comments
     (0)
    0