From eeaaf131ac699fa7fc77c5657eec13dcba1b4ca9 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Thu, 24 Nov 2022 00:58:16 +0100 Subject: [PATCH] MDL-75977 xmldb: Avoid using null params on built-in DB functions This changes a couple of cases, in postgres and oracle where we were trying to calculate strlen() on null values (that causes a PHP 8.1 warning). Also, at the same time, fixing another case in xmldb_field, it was detected that it had a bug around precision (being set to the value of type), being used as object property, when it's not, so it was also fixed by moving the code logic a little bit. Note that the bug has been there since inception because there isn't any code in Moodle using that ->precision property. It was just detected thanks to PHP 8.2, but that's another story. :-) Verified that with the patch: - All DBs can be installed (phpunit install) - Both DDL and DML pass without any ddl/dml warning (in all DBs). --- lib/dml/oci_native_moodle_database.php | 16 ++++++------- lib/dml/pgsql_native_moodle_database.php | 2 +- lib/xmldb/xmldb_field.php | 30 +++++++++++++----------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php index aa073fb3d67..67099bda2c4 100644 --- a/lib/dml/oci_native_moodle_database.php +++ b/lib/dml/oci_native_moodle_database.php @@ -689,15 +689,13 @@ class oci_native_moodle_database extends moodle_database { if (is_bool($value)) { // Always, convert boolean to int $value = (int)$value; - } else if ($column->meta_type == 'B') { // BLOB detected, we return 'blob' array instead of raw value to allow - if (!is_null($value)) { // binding/executing code later to know about its nature - $value = array('blob' => $value); - } + } else if ($column->meta_type == 'B' && !is_null($value)) { + // Not null BLOB detected, we return 'blob' array instead for later handing on binding. + $value = array('blob' => $value); - } else if ($column->meta_type == 'X' && strlen($value) > 4000) { // CLOB detected (>4000 optimisation), we return 'clob' - if (!is_null($value)) { // array instead of raw value to allow binding/ - $value = array('clob' => (string)$value); // executing code later to know about its nature - } + } else if ($column->meta_type == 'X' && !is_null($value) && strlen($value) > 4000) { + // Not null CLOB detected (>4000 optimisation), we return 'clob' array instead for later handing on binding. + $value = array('clob' => (string)$value); } else if ($value === '') { if ($column->meta_type == 'I' or $column->meta_type == 'F' or $column->meta_type == 'N') { @@ -962,7 +960,7 @@ class oci_native_moodle_database extends moodle_database { // passed in an arbitrary sql (not processed by normalise_value() ever, // and let's handle it as such. This will provide proper binding of CLOBs in // conditions and other raw SQLs not covered by the above function. - if (strlen($value) > 4000) { + if (!is_null($value) && strlen($value) > 4000) { $lob = oci_new_descriptor($this->oci, OCI_DTYPE_LOB); if ($descriptors === null) { throw new coding_exception('moodle_database::bind_params() $descriptors not specified for clob'); diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php index 9ab215e4ea1..a61b4f89dec 100644 --- a/lib/dml/pgsql_native_moodle_database.php +++ b/lib/dml/pgsql_native_moodle_database.php @@ -545,7 +545,7 @@ class pgsql_native_moodle_database extends moodle_database { $tablename = $this->prefix.$table; $sql = "SELECT a.attnum, a.attname AS field, t.typname AS type, a.attlen, a.atttypmod, a.attnotnull, a.atthasdef, - CASE WHEN a.atthasdef THEN pg_catalog.pg_get_expr(d.adbin, d.adrelid) END AS adsrc + CASE WHEN a.atthasdef THEN pg_catalog.pg_get_expr(d.adbin, d.adrelid) ELSE '' END AS adsrc FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace as ns ON ns.oid = c.relnamespace JOIN pg_catalog.pg_attribute a ON a.attrelid = c.oid diff --git a/lib/xmldb/xmldb_field.php b/lib/xmldb/xmldb_field.php index 25946b4a80c..12061685cc2 100644 --- a/lib/xmldb/xmldb_field.php +++ b/lib/xmldb/xmldb_field.php @@ -116,25 +116,27 @@ class xmldb_field extends xmldb_object { */ public function set_attributes($type, $precision=null, $unsigned=null, $notnull=null, $sequence=null, $default=null, $previous=null) { $this->type = $type; - /// Try to split the precision into length and decimals and apply - /// each one as needed - $precisionarr = explode(',', $precision); - if (isset($precisionarr[0])) { - $this->length = trim($precisionarr[0]); + + // LOBs (BINARY OR TEXT) don't support any precision (neither length or decimals). + if ($type == XMLDB_TYPE_BINARY || $this->type == XMLDB_TYPE_TEXT) { + $this->length = null; + $this->decimals = null; + + } else if (!is_null($precision)) { + // Try to split the not null precision into length and decimals and apply each one as needed. + $precisionarr = explode(',', $precision); + if (isset($precisionarr[0])) { + $this->length = trim($precisionarr[0]); + } + if (isset($precisionarr[1])) { + $this->decimals = trim($precisionarr[1]); + } } - if (isset($precisionarr[1])) { - $this->decimals = trim($precisionarr[1]); - } - $this->precision = $type; + $this->notnull = !empty($notnull) ? true : false; $this->sequence = !empty($sequence) ? true : false; $this->setDefault($default); - if ($this->type == XMLDB_TYPE_BINARY || $this->type == XMLDB_TYPE_TEXT) { - $this->length = null; - $this->decimals = null; - } - $this->previous = $previous; }