NotificationsYou must be signed in to change notification settings
Fork6
Star31

Commitc4939f1

committed

Clean up dubious error handling in wellformed_xml().

This ancient bit of code was summarily trapping any ereport longjmpwhatsoever 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 cancelerror that way. (Perhaps that can't happen because there is noCHECK_FOR_INTERRUPTS anywhere within xml_parse, but even if that'strue today it's obviously a very fragile assumption.)But in the wake of the previous commit, we can drop the PG_TRYhere altogether, and use the soft error mechanism to catch onlythe kinds of errors that are legitimate to treat as invalid-XML.(This is our first use of the soft error mechanism for somethingnot directly related to a datatype input function. It won't bethe last.)xml_is_document can be converted in the same way. That one isnot actively broken, because it was checking specifically forERRCODE_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 parent37bef84 commitc4939f1Copy full SHA for c4939f1

File tree

1 file changed

+17

-47

lines changed

src/backend/utils/adt
- xml.c

1 file changed

+17

-47

lines changed

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

Lines changed: 17 additions & 47 deletions

Original file line number	Diff line number	Diff line change
`@@ -81,6 +81,7 @@`
`81`	`81`	`#include"mb/pg_wchar.h"`
`82`	`82`	`#include"miscadmin.h"`
`83`	`83`	`#include"nodes/execnodes.h"`
	`84`	`+#include"nodes/miscnodes.h"`
`84`	`85`	`#include"nodes/nodeFuncs.h"`
`85`	`86`	`#include"utils/array.h"`
`86`	`87`	`#include"utils/builtins.h"`
`@@ -894,41 +895,18 @@ bool`
`894`	`895`	`xml_is_document(xmltype*arg)`
`895`	`896`	`{`
`896`	`897`	`#ifdefUSE_LIBXML`
`897`		`-boolresult;`
`898`		`-volatilexmlDocPtrdoc=NULL;`
`899`		`-MemoryContextccxt=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`		`-MemoryContextecxt;`
`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`	`+xmlDocPtrdoc;`
	`899`	`+ErrorSaveContextescontext= {T_ErrorSaveContext};`
`927`	`900`
	`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);`
`928`	`906`	`if (doc)`
`929`	`907`	`xmlFreeDoc(doc);`
`930`	`908`
`931`		`-returnresult;`
	`909`	`+return!escontext.error_occurred;`
`932`	`910`	`#else/* not USE_LIBXML */`
`933`	`911`	`NO_XML_SUPPORT();`
`934`	`912`	`return false;`
`@@ -4320,26 +4298,18 @@ xpath_exists(PG_FUNCTION_ARGS)`
`4320`	`4298`	`staticbool`
`4321`	`4299`	`wellformed_xml(text*data,XmlOptionTypexmloption_arg)`
`4322`	`4300`	`{`
`4323`		`-boolresult;`
`4324`		`-volatilexmlDocPtrdoc=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`	`+xmlDocPtrdoc;`
	`4302`	`+ErrorSaveContextescontext= {T_ErrorSaveContext};`
`4338`	`4303`
	`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);`
`4339`	`4309`	`if (doc)`
`4340`	`4310`	`xmlFreeDoc(doc);`
`4341`	`4311`
`4342`		`-returnresult;`
	`4312`	`+return!escontext.error_occurred;`
`4343`	`4313`	`}`
`4344`	`4314`	`#endif`
`4345`	`4315`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitc4939f1

File tree

1 file changed

1 file changed

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

0 commit comments