diff --git a/UPGRADING b/UPGRADING index 54a2f0e8c33e2..f2f8d608953b1 100644 --- a/UPGRADING +++ b/UPGRADING @@ -31,6 +31,11 @@ PHP 8.4 UPGRADE NOTES for invalid modes. Previously invalid modes would have been interpreted as PHP_ROUND_HALF_UP. +- XSL: + . XSLTProcessor::setParameter() will now throw a ValueError when its arguments + contain null bytes. This never actually worked correctly in the first place, + which is why it throws an exception nowadays. + ======================================== 2. New Features ======================================== diff --git a/ext/xsl/tests/bug48221.phpt b/ext/xsl/tests/bug48221.phpt index 9b75c787022fe..64a4a9c5d77f1 100644 --- a/ext/xsl/tests/bug48221.phpt +++ b/ext/xsl/tests/bug48221.phpt @@ -9,8 +9,9 @@ $proc->importStylesheet($xsl); $proc->setParameter('', '', '"\''); $proc->transformToXml($dom); ?> ---EXPECTF-- -Warning: XSLTProcessor::transformToXml(): Cannot create XPath expression (string contains both quote and double-quotes) in %s on line %d +Done +--EXPECT-- +Done --CREDITS-- Christian Weiske, cweiske@php.net PHP Testfest Berlin 2009-05-09 diff --git a/ext/xsl/tests/bug64137.phpt b/ext/xsl/tests/bug64137.phpt new file mode 100644 index 0000000000000..96b02e34e503c --- /dev/null +++ b/ext/xsl/tests/bug64137.phpt @@ -0,0 +1,35 @@ +--TEST-- +Request #64137 (XSLTProcessor::setParameter() should allow both quotes to be used) +--EXTENSIONS-- +xsl +--FILE-- +loadXML(''); + + $xsl = new DOMDocument; + $xsl->loadXML(''); + + $xslt = new XSLTProcessor; + $xslt->importStylesheet($xsl); + $xslt->setParameter('', 'foo', $input); + + echo $xslt->transformToXml($xml), "\n"; +} + +test(""); +test("a'"); +test("a\""); +test("fo'o\"ba'r\"ba'z"); +test("\"\"\"fo'o\"ba'r\"ba'z\"\"\""); +test("'''\"\"\"fo'o\"ba'r\"ba'z\"\"\"'''"); + +?> +--EXPECT-- +a' +a" +fo'o"ba'r"ba'z +"""fo'o"ba'r"ba'z""" +'''"""fo'o"ba'r"ba'z"""''' diff --git a/ext/xsl/tests/setParameter_exceptions_test.phpt b/ext/xsl/tests/setParameter_exceptions_test.phpt new file mode 100644 index 0000000000000..57602fc1814cd --- /dev/null +++ b/ext/xsl/tests/setParameter_exceptions_test.phpt @@ -0,0 +1,102 @@ +--TEST-- +setParameter exceptions test +--EXTENSIONS-- +xsl +--FILE-- +loadXML(''); + + $xsl = new DOMDocument; + $xsl->loadXML(''); + + $xslt = new XSLTProcessor; + $xslt->importStylesheet($xsl); + $xslt->setParameter('', $options); + + echo $xslt->transformToXml($xml), "\n"; +} + +echo "--- Invalid key ---\n"; + +try { + test([ + 12345 => "foo" + ]); +} catch (TypeError $e) { + echo $e->getMessage(), "\n"; +} + +echo "--- Valid key and value, but special cases ---\n"; + +test([ + "foo" => null, +]); + +test([ + "foo" => true, +]); + +echo "--- Exception from Stringable should abort execution ---\n"; + +class MyStringable { + public function __toString(): string { + throw new Exception("exception!"); + } +} + +try { + test([ + "foo" => new MyStringable, + ]); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} + +echo "--- Exception from warning should abort execution ---\n"; + +set_error_handler(function($errno, $errstr) { + throw new Exception($errstr); +}, E_WARNING); + +try { + test([ + "foo" => [], + "foo2" => [], + ]); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} + +set_error_handler(null, E_WARNING); + +echo "--- Warning may continue execution ---\n"; + +try { + test([ + "foo" => [], + "foo2" => [], + ]); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECTF-- +--- Invalid key --- +XSLTProcessor::setParameter(): Argument #2 ($name) must contain only string keys +--- Valid key and value, but special cases --- + +1 +--- Exception from Stringable should abort execution --- +exception! +--- Exception from warning should abort execution --- +Array to string conversion +--- Warning may continue execution --- + +Warning: Array to string conversion in %s on line %d + +Warning: Array to string conversion in %s on line %d +Array diff --git a/ext/xsl/tests/setParameter_null_bytes.phpt b/ext/xsl/tests/setParameter_null_bytes.phpt new file mode 100644 index 0000000000000..f6efe0d114b77 --- /dev/null +++ b/ext/xsl/tests/setParameter_null_bytes.phpt @@ -0,0 +1,43 @@ +--TEST-- +setParameter() with null bytes +--EXTENSIONS-- +xsl +--FILE-- +setParameter("", "foo\0", "bar"); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} + +try { + $xslt->setParameter("", "foo", "bar\0"); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} + +try { + $xslt->setParameter("", [ + "foo\0" => "bar", + ]); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} + +try { + $xslt->setParameter("", [ + "foo" => "bar\0", + ]); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECT-- +XSLTProcessor::setParameter(): Argument #2 ($name) must not contain any null bytes +XSLTProcessor::setParameter(): Argument #3 ($value) must not contain any null bytes +XSLTProcessor::setParameter(): Argument #3 ($value) must not contain keys with any null bytes +XSLTProcessor::setParameter(): Argument #3 ($value) must not contain values with any null bytes diff --git a/ext/xsl/tests/xsltprocessor_setparameter-errorquote.phpt b/ext/xsl/tests/xsltprocessor_setparameter-errorquote.phpt index 637f0e5556c27..7bb0c8bf3b49f 100644 --- a/ext/xsl/tests/xsltprocessor_setparameter-errorquote.phpt +++ b/ext/xsl/tests/xsltprocessor_setparameter-errorquote.phpt @@ -11,8 +11,9 @@ $proc->importStylesheet($xsl); $proc->setParameter('', '', '"\''); $proc->transformToXml($dom); ?> ---EXPECTF-- -Warning: XSLTProcessor::transformToXml(): Cannot create XPath expression (string contains both quote and double-quotes) in %s on line %d +Done +--EXPECT-- +Done --CREDITS-- Christian Weiske, cweiske@php.net PHP Testfest Berlin 2009-05-09 diff --git a/ext/xsl/xsltprocessor.c b/ext/xsl/xsltprocessor.c index 70223458a54b1..fb607c40b1a16 100644 --- a/ext/xsl/xsltprocessor.c +++ b/ext/xsl/xsltprocessor.c @@ -21,75 +21,29 @@ #include "php.h" #include "php_xsl.h" +#include #include "ext/libxml/php_libxml.h" -/* {{{ php_xsl_xslt_string_to_xpathexpr() - Translates a string to a XPath Expression */ -static char *php_xsl_xslt_string_to_xpathexpr(const char *str) -{ - const xmlChar *string = (const xmlChar *)str; - - xmlChar *value; - int str_len; - - str_len = xmlStrlen(string) + 3; - if (xmlStrchr(string, '"')) { - if (xmlStrchr(string, '\'')) { - php_error_docref(NULL, E_WARNING, "Cannot create XPath expression (string contains both quote and double-quotes)"); - return NULL; - } - value = (xmlChar*) safe_emalloc (str_len, sizeof(xmlChar), 0); - snprintf((char*)value, str_len, "'%s'", string); - } else { - value = (xmlChar*) safe_emalloc (str_len, sizeof(xmlChar), 0); - snprintf((char *)value, str_len, "\"%s\"", string); - } - return (char *) value; -} -/* }}} */ - -/* {{{ php_xsl_xslt_make_params() - Translates a PHP array to a libxslt parameters array */ -static char **php_xsl_xslt_make_params(HashTable *parht, int xpath_params) +static zend_result php_xsl_xslt_apply_params(xsltTransformContextPtr ctxt, HashTable *params) { - - int parsize; - zval *value; - char *xpath_expr; zend_string *string_key; - char **params = NULL; - int i = 0; - - parsize = (2 * zend_hash_num_elements(parht) + 1) * sizeof(char *); - params = (char **)safe_emalloc((2 * zend_hash_num_elements(parht) + 1), sizeof(char *), 0); - memset((char *)params, 0, parsize); + zval *value; - ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(parht, string_key, value) { + ZEND_HASH_MAP_FOREACH_STR_KEY_VAL(params, string_key, value) { ZEND_ASSERT(string_key != NULL); - if (Z_TYPE_P(value) != IS_STRING) { - if (!try_convert_to_string(value)) { - efree(params); - return NULL; - } - } + /* Already a string because of setParameter() */ + ZEND_ASSERT(Z_TYPE_P(value) == IS_STRING); - if (!xpath_params) { - xpath_expr = php_xsl_xslt_string_to_xpathexpr(Z_STRVAL_P(value)); - } else { - xpath_expr = estrndup(Z_STRVAL_P(value), Z_STRLEN_P(value)); - } - if (xpath_expr) { - params[i++] = estrndup(ZSTR_VAL(string_key), ZSTR_LEN(string_key)); - params[i++] = xpath_expr; + int result = xsltQuoteOneUserParam(ctxt, (const xmlChar *) ZSTR_VAL(string_key), (const xmlChar *) Z_STRVAL_P(value)); + if (result < 0) { + php_error_docref(NULL, E_WARNING, "Could not apply parameter \"%s\"", ZSTR_VAL(string_key)); + return FAILURE; } } ZEND_HASH_FOREACH_END(); - params[i++] = NULL; - - return params; + return SUCCESS; } -/* }}} */ static void xsl_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, int type) /* {{{ */ { @@ -402,8 +356,6 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl xmlNodePtr node = NULL; xsltTransformContextPtr ctxt; php_libxml_node_object *object; - char **params = NULL; - int clone; zval *doXInclude, rv; zend_string *member; FILE *f; @@ -440,10 +392,6 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl f = NULL; } - if (intern->parameter) { - params = php_xsl_xslt_make_params(intern->parameter, 0); - } - intern->doc = emalloc(sizeof(php_libxml_node_object)); memset(intern->doc, 0, sizeof(php_libxml_node_object)); @@ -459,6 +407,13 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl ctxt = xsltNewTransformContext(style, doc); ctxt->_private = (void *) intern; + if (intern->parameter) { + zend_result status = php_xsl_xslt_apply_params(ctxt, intern->parameter); + if (UNEXPECTED(status != SUCCESS) && EG(exception)) { + goto out; + } + } + member = ZSTR_INIT_LITERAL("doXInclude", 0); doXInclude = zend_std_read_property(Z_OBJ_P(id), member, BP_VAR_IS, NULL, &rv); if (Z_TYPE_P(doXInclude) != IS_NULL) { @@ -506,8 +461,10 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl if (secPrefsError == 1) { php_error_docref(NULL, E_WARNING, "Can't set libxslt security properties, not doing transformation for security reasons"); } else { - newdocp = xsltApplyStylesheetUser(style, doc, (const char**) params, NULL, f, ctxt); + newdocp = xsltApplyStylesheetUser(style, doc, /* params (handled manually) */ NULL, /* output */ NULL, f, ctxt); } + +out: if (f) { fclose(f); } @@ -527,14 +484,6 @@ static xmlDocPtr php_xsl_apply_stylesheet(zval *id, xsl_object *intern, xsltStyl efree(intern->doc); intern->doc = NULL; - if (params) { - clone = 0; - while(params[clone]) { - efree(params[clone++]); - } - efree(params); - } - return newdocp; } @@ -681,7 +630,7 @@ PHP_METHOD(XSLTProcessor, setParameter) Z_PARAM_STRING(namespace, namespace_len) Z_PARAM_ARRAY_HT_OR_STR(array_value, name) Z_PARAM_OPTIONAL - Z_PARAM_STR_OR_NULL(value) + Z_PARAM_PATH_STR_OR_NULL(value) ZEND_PARSE_PARAMETERS_END(); intern = Z_XSL_P(id); @@ -700,10 +649,23 @@ PHP_METHOD(XSLTProcessor, setParameter) zend_argument_type_error(2, "must contain only string keys"); RETURN_THROWS(); } + + if (UNEXPECTED(CHECK_NULL_PATH(ZSTR_VAL(string_key), ZSTR_LEN(string_key)))) { + zend_argument_value_error(3, "must not contain keys with any null bytes"); + RETURN_THROWS(); + } + str = zval_try_get_string(entry); if (UNEXPECTED(!str)) { RETURN_THROWS(); } + + if (UNEXPECTED(CHECK_NULL_PATH(ZSTR_VAL(str), ZSTR_LEN(str)))) { + zend_string_release(str); + zend_argument_value_error(3, "must not contain values with any null bytes"); + RETURN_THROWS(); + } + ZVAL_STR(&tmp, str); zend_hash_update(intern->parameter, string_key, &tmp); } ZEND_HASH_FOREACH_END(); @@ -714,6 +676,11 @@ PHP_METHOD(XSLTProcessor, setParameter) RETURN_THROWS(); } + if (UNEXPECTED(CHECK_NULL_PATH(ZSTR_VAL(name), ZSTR_LEN(name)))) { + zend_argument_value_error(2, "must not contain any null bytes"); + RETURN_THROWS(); + } + ZVAL_STR_COPY(&new_string, value); zend_hash_update(intern->parameter, name, &new_string); @@ -737,7 +704,7 @@ PHP_METHOD(XSLTProcessor, getParameter) } intern = Z_XSL_P(id); if ((value = zend_hash_find(intern->parameter, name)) != NULL) { - RETURN_STR(zval_get_string(value)); + RETURN_STR_COPY(Z_STR_P(value)); } else { RETURN_FALSE; }