[CF 5718] v3 - contrib/xml2: xslt_process() should report XSLT-related error details and accept xmltype

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/5718

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/D9PS2YM0OS8H.D27X2G7ZEQRS@b1-systems.de
Author(s): Robin Haberkorn

Author: Commitfest Bot

contrib/xml2/expected/xml2.out

@@ -261,3 +261,33 @@ $$<xsl:stylesheet version="1.0"
   </xsl:template>
 </xsl:stylesheet>$$);
 ERROR:  failed to apply stylesheet
+DETAIL:  runtime error: file SQL line 7 element output
+File write for 0wn3d.txt refused
+runtime error: file SQL line 7 element output
+xsltDocumentElem: write rights for 0wn3d.txt denied
+-- detecting missing stylesheet parameter
+SELECT xslt_process('<xml/>',
+$$<stylesheet version="1.0" xmlns="http://www.w3.org/1999/XSL/Transform">
+  <template match="/">
+    <value-of select="$n1"/>
+  </template>
+</stylesheet>$$)::xml;
+ERROR:  failed to apply stylesheet
+DETAIL:  runtime error: file SQL line 3 element value-of
+Variable 'n1' has not been declared.
+Undefined variable
+runtime error: file SQL line 3 element value-of
+XPath evaluation returned no result.
+-- xmltype and Array-based signature
+SELECT xslt_process(xmlelement(name xml),
+$$<stylesheet version="1.0" xmlns="http://www.w3.org/1999/XSL/Transform">
+  <template match="/">
+    <value-of select="$n1"/>
+  </template>
+</stylesheet>$$::xml, ARRAY['n1','"foo"']);
+ xslt_process 
+--------------
+ foo         +
+ 
+(1 row)
+

contrib/xml2/sql/xml2.sql

@@ -153,3 +153,19 @@ $$<xsl:stylesheet version="1.0"
     </sax:output>
   </xsl:template>
 </xsl:stylesheet>$$);
+
+-- detecting missing stylesheet parameter
+SELECT xslt_process('<xml/>',
+$$<stylesheet version="1.0" xmlns="http://www.w3.org/1999/XSL/Transform">
+  <template match="/">
+    <value-of select="$n1"/>
+  </template>
+</stylesheet>$$)::xml;
+
+-- xmltype and Array-based signature
+SELECT xslt_process(xmlelement(name xml),
+$$<stylesheet version="1.0" xmlns="http://www.w3.org/1999/XSL/Transform">
+  <template match="/">
+    <value-of select="$n1"/>
+  </template>
+</stylesheet>$$::xml, ARRAY['n1','"foo"']);

contrib/xml2/xml2--1.1.sql

@@ -71,3 +71,14 @@ CREATE FUNCTION xslt_process(text,text)
 RETURNS text
 AS 'MODULE_PATHNAME'
 LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE;
+
+CREATE FUNCTION xslt_process(xml,xml,text[])
+RETURNS xml
+AS 'MODULE_PATHNAME','xslt_process_xmltype'
+LANGUAGE C STRICT VOLATILE PARALLEL SAFE;
+
+-- the function checks for the correct argument count
+CREATE FUNCTION xslt_process(xml,xml)
+RETURNS xml
+AS 'MODULE_PATHNAME','xslt_process_xmltype'
+LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE;

contrib/xml2/xslt_proc.c

@@ -10,6 +10,9 @@
 #include "fmgr.h"
 #include "utils/builtins.h"
 #include "utils/xml.h"
+#include "utils/array.h"
+#include "utils/memutils.h"
+#include "mb/pg_wchar.h"
 
 #ifdef USE_LIBXSLT
 
@@ -35,9 +38,18 @@
 extern PgXmlErrorContext *pgxml_parser_init(PgXmlStrictness strictness);
 
 /* local defs */
+static xmltype *xslt_process_internal(xmltype *doct, xmltype *ssheet, const char **params);
 static const char **parse_params(text *paramstr);
 #endif							/* USE_LIBXSLT */
 
+/*
+ * FIXME: This cannot easily be exposed in xml.h.
+ * Perhaps there should be an xml-internal.h?
+ */
+xmlDocPtr	xml_parse(text *data, XmlOptionType xmloption_arg,
+					  bool preserve_whitespace, int encoding,
+					  XmlOptionType *parsed_xmloptiontype, xmlNodePtr *parsed_nodes,
+					  Node *escontext);
 
 PG_FUNCTION_INFO_V1(xslt_process);
 
@@ -48,9 +60,103 @@ xslt_process(PG_FUNCTION_ARGS)
 
 	text	   *doct = PG_GETARG_TEXT_PP(0);
 	text	   *ssheet = PG_GETARG_TEXT_PP(1);
+	const char **params = NULL;
+	text	   *result;
+
+	if (fcinfo->nargs == 3)
+	{
+		text	   *paramstr = PG_GETARG_TEXT_PP(2);
+
+		params = parse_params(paramstr);
+	}
+
+	result = xslt_process_internal(doct, ssheet, params);
+
+	PG_RETURN_TEXT_P(result);
+
+#else							/* !USE_LIBXSLT */
+
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("xslt_process() is not available without libxslt")));
+	PG_RETURN_NULL();
+
+#endif							/* USE_LIBXSLT */
+}
+
+PG_FUNCTION_INFO_V1(xslt_process_xmltype);
+
+Datum
+xslt_process_xmltype(PG_FUNCTION_ARGS)
+{
+#ifdef USE_LIBXSLT
+
+	xmltype    *doct = PG_GETARG_XML_P(0);
+	xmltype    *ssheet = PG_GETARG_XML_P(1);
+	const char **params = NULL;
+	xmltype    *result;
+
+	/*
+	 * Parameters are key-value pairs. The values are XPath expressions, so
+	 * strings will have to be escaped with single or double quotes. Even
+	 * `xsltproc --stringparam` does nothing else than adding single or double
+	 * quotes and fails if the value contains both.
+	 */
+	if (fcinfo->nargs == 3)
+	{
+		ArrayType  *paramarray = PG_GETARG_ARRAYTYPE_P(2);
+		Datum	   *arr_datums;
+		bool	   *arr_nulls;
+		int			arr_count;
+		int			i,
+					j;
+
+		deconstruct_array_builtin(paramarray, TEXTOID, &arr_datums, &arr_nulls, &arr_count);
+
+		if ((arr_count % 2) != 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_ARRAY_ELEMENT_ERROR),
+					 errmsg("number of stylesheet parameters (%d) must be a multiple of 2",
+							arr_count)));
+
+		params = palloc_array(const char *, arr_count + 1);
+
+		for (i = 0, j = 0; i < arr_count; i++)
+		{
+			char	   *cstr;
+
+			if (arr_nulls[i])
+				continue;
+
+			cstr = TextDatumGetCString(arr_datums[i]);
+			params[j++] = (char *) pg_do_encoding_conversion((unsigned char *) cstr,
+															 strlen(cstr),
+															 GetDatabaseEncoding(),
+															 PG_UTF8);
+		}
+		params[j] = NULL;
+	}
+
+	result = xslt_process_internal(doct, ssheet, params);
+
+	PG_RETURN_XML_P(result);
+
+#else							/* !USE_LIBXSLT */
+
+	ereport(ERROR,
+			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+			 errmsg("xslt_process() is not available without libxslt")));
+	PG_RETURN_NULL();
+
+#endif							/* USE_LIBXSLT */
+}
+
+#ifdef USE_LIBXSLT
+
+static xmltype *
+xslt_process_internal(xmltype *doct, xmltype *ssheet, const char **params)
+{
 	text	   *result;
-	text	   *paramstr;
-	const char **params;
 	PgXmlErrorContext *xmlerrcxt;
 	volatile xsltStylesheetPtr stylesheet = NULL;
 	volatile xmlDocPtr doctree = NULL;
@@ -61,48 +167,48 @@ xslt_process(PG_FUNCTION_ARGS)
 	xmlChar    *resstr = NULL;
 	int			reslen = 0;
 
-	if (fcinfo->nargs == 3)
-	{
-		paramstr = PG_GETARG_TEXT_PP(2);
-		params = parse_params(paramstr);
-	}
-	else
-	{
-		/* No parameters */
-		params = (const char **) palloc(sizeof(char *));
-		params[0] = NULL;
-	}
+	/* the previous libxslt error context */
+	xmlGenericErrorFunc saved_errfunc;
+	void	   *saved_errcxt;
 
 	/* Setup parser */
-	xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_LEGACY);
+	xmlerrcxt = pgxml_parser_init(PG_XML_STRICTNESS_ALL);
+
+	/*
+	 * Save the previous libxslt error context.
+	 */
+	saved_errfunc = xsltGenericError;
+	saved_errcxt = xsltGenericErrorContext;
+	xsltSetGenericErrorFunc(xmlerrcxt, xml_generic_error_handler);
 
 	PG_TRY();
 	{
 		xmlDocPtr	ssdoc;
 		bool		xslt_sec_prefs_error;
 
-		/* Parse document */
-		doctree = xmlReadMemory((char *) VARDATA_ANY(doct),
-								VARSIZE_ANY_EXHDR(doct), NULL, NULL,
-								XML_PARSE_NOENT);
+		/*
+		 * Parse document. It's important to set an "URL", so libxslt includes
+		 * line numbers in error messages (cf. xsltPrintErrorContext()).
+		 */
+		doctree = xml_parse(doct, XMLOPTION_DOCUMENT, true,
+							GetDatabaseEncoding(), NULL, NULL, NULL);
 
-		if (doctree == NULL)
+		if (doctree == NULL || pg_xml_error_occurred(xmlerrcxt))
 			xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
 						"error parsing XML document");
 
 		/* Same for stylesheet */
-		ssdoc = xmlReadMemory((char *) VARDATA_ANY(ssheet),
-							  VARSIZE_ANY_EXHDR(ssheet), NULL, NULL,
-							  XML_PARSE_NOENT);
+		ssdoc = xml_parse(ssheet, XMLOPTION_DOCUMENT, true,
+						  GetDatabaseEncoding(), NULL, NULL, NULL);
 
-		if (ssdoc == NULL)
+		if (ssdoc == NULL || pg_xml_error_occurred(xmlerrcxt))
 			xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
 						"error parsing stylesheet as XML document");
 
 		/* After this call we need not free ssdoc separately */
 		stylesheet = xsltParseStylesheetDoc(ssdoc);
 
-		if (stylesheet == NULL)
+		if (stylesheet == NULL || pg_xml_error_occurred(xmlerrcxt))
 			xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
 						"failed to parse stylesheet");
 
@@ -137,11 +243,18 @@ xslt_process(PG_FUNCTION_ARGS)
 		restree = xsltApplyStylesheetUser(stylesheet, doctree, params,
 										  NULL, NULL, xslt_ctxt);
 
-		if (restree == NULL)
+		if (restree == NULL || pg_xml_error_occurred(xmlerrcxt))
 			xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
 						"failed to apply stylesheet");
 
+		/*
+		 * FIXME: There is probably a function to convert the string to the
+		 * correct database encoding.
+		 */
 		resstat = xsltSaveResultToString(&resstr, &reslen, restree, stylesheet);
+		if (resstat < 0 || pg_xml_error_occurred(xmlerrcxt))
+			xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
+						"failed to save result to string");
 	}
 	PG_CATCH();
 	{
@@ -157,6 +270,7 @@ xslt_process(PG_FUNCTION_ARGS)
 			xmlFreeDoc(doctree);
 		xsltCleanupGlobals();
 
+		xsltSetGenericErrorFunc(saved_errcxt, saved_errfunc);
 		pg_xml_done(xmlerrcxt, true);
 
 		PG_RE_THROW();
@@ -170,29 +284,17 @@ xslt_process(PG_FUNCTION_ARGS)
 	xmlFreeDoc(doctree);
 	xsltCleanupGlobals();
 
+	xsltSetGenericErrorFunc(saved_errcxt, saved_errfunc);
 	pg_xml_done(xmlerrcxt, false);
 
-	/* XXX this is pretty dubious, really ought to throw error instead */
-	if (resstat < 0)
-		PG_RETURN_NULL();
-
 	result = cstring_to_text_with_len((char *) resstr, reslen);
 
 	if (resstr)
 		xmlFree(resstr);
 
-	PG_RETURN_TEXT_P(result);
-#else							/* !USE_LIBXSLT */
-
-	ereport(ERROR,
-			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-			 errmsg("xslt_process() is not available without libxslt")));
-	PG_RETURN_NULL();
-#endif							/* USE_LIBXSLT */
+	return result;
 }
 
-#ifdef USE_LIBXSLT
-
 static const char **
 parse_params(text *paramstr)
 {

doc/src/sgml/xml2.sgml

@@ -408,22 +408,29 @@ ORDER BY doc_num, line_num;
   </indexterm>
 
 <synopsis>
-xslt_process(text document, text stylesheet, text paramlist) returns text
+xslt_process(xml document, xml stylesheet, text[] paramlist) returns xml
 </synopsis>
 
    <para>
     This function applies the XSL stylesheet to the document and returns
-    the transformed result. The <literal>paramlist</literal> is a list of parameter
-    assignments to be used in the transformation, specified in the form
-    <literal>a=1,b=2</literal>. Note that the
-    parameter parsing is very simple-minded: parameter values cannot
-    contain commas!
+    the transformed result. The <literal>paramlist</literal> is an array of parameter
+    assignments to be used in the transformation, specified in pairs of
+    key and value strings (e.g. <literal>ARRAY['a','1', 'b','2']</literal>).
+    The length of the array must be even.
+    Note that the values are still interpreted as XPath expressions, so string values need to
+    be quoted in single or double quotes (e.g. <literal>ARRAY['a','"string"']</literal>).
    </para>
 
    <para>
     There is also a two-parameter version of <function>xslt_process</function> which
     does not pass any parameters to the transformation.
    </para>
+
+   <para>
+    <emphasis>Deprecated</emphasis> variants of <function>xslt_process</function> accepting
+    text arguments and parameters encoded into single text strings
+    (e.g. <literal>a=1,b=2</literal>) are also still available.
+   </para>
   </sect3>
  </sect2>