From 4aec01cc4ca132034280e5753db9c3dfa6641087 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 7 Jul 2016 11:26:31 +0800 Subject: [PATCH 1/2] MDL-52544 oracle: php7 fixes for oci driver These are our bugs actually - the oci docs are very clear that bound params should be in-scope when oci_execute is called - and due to pass by value etc, this was not true for our driver. There is another bug that needed fixing - OCI barfs if null values are bound - this can be avoided by setting them to '' which oci treats as null. And finally - all our lob/clob/blob hacks were also binding to local "out-of-scope" vars. --- lib/dml/oci_native_moodle_database.php | 57 +++++++++++++++++++------- 1 file changed, 43 insertions(+), 14 deletions(-) diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php index d8f8d67c60c..71d387eba1b 100644 --- a/lib/dml/oci_native_moodle_database.php +++ b/lib/dml/oci_native_moodle_database.php @@ -766,6 +766,8 @@ class oci_native_moodle_database extends moodle_database { return ' '; } else if (is_bool($value)) { return (int)$value; + } else if (is_null($value)) { + return ''; } else { return $value; } @@ -778,6 +780,8 @@ class oci_native_moodle_database extends moodle_database { return ' '; } else if (is_bool($value)) { return (int)$value; + } else if (is_null($value)) { + return ''; } else { return $value; } @@ -847,6 +851,9 @@ class oci_native_moodle_database extends moodle_database { } else if (gettype($value) == 'integer') { return '0'; // Transform 0 to '0' that evaluates the same for PHP + } else if (is_null($value)) { + return ''; + } else if ($value === '') { return ' '; // Transform '' to ' ' that DON'T EVALUATE THE SAME // (we'll transform back again on get_records_XXX functions and others)!! @@ -913,8 +920,7 @@ class oci_native_moodle_database extends moodle_database { return true; } - protected function bind_params($stmt, array $params=null, $tablename=null) { - $descriptors = array(); + protected function bind_params($stmt, array & $params=null, $tablename=null, array & $descriptors = null) { if ($params) { $columns = array(); if ($tablename) { @@ -934,15 +940,21 @@ class oci_native_moodle_database extends moodle_database { if (is_array($value)) { // Let's go to bind special cases (lob descriptors) if (isset($value['clob'])) { $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'); + } + $descriptors[] = $lob; oci_bind_by_name($stmt, $key, $lob, -1, SQLT_CLOB); $lob->writeTemporary($this->oracle_dirty_hack($tablename, $columnname, $params[$key]['clob']), OCI_TEMP_CLOB); - $descriptors[] = $lob; continue; // Column binding finished, go to next one } else if (isset($value['blob'])) { $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'); + } + $descriptors[] = $lob; oci_bind_by_name($stmt, $key, $lob, -1, SQLT_BLOB); $lob->writeTemporary($params[$key]['blob'], OCI_TEMP_BLOB); - $descriptors[] = $lob; continue; // Column binding finished, go to next one } } else { @@ -952,9 +964,12 @@ class oci_native_moodle_database extends moodle_database { // conditions and other raw SQLs not covered by the above function. if (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'); + } + $descriptors[] = $lob; oci_bind_by_name($stmt, $key, $lob, -1, SQLT_CLOB); $lob->writeTemporary($this->oracle_dirty_hack($tablename, $columnname, $params[$key]), OCI_TEMP_CLOB); - $descriptors[] = $lob; continue; // Param binding finished, go to next one. } } @@ -1000,6 +1015,7 @@ class oci_native_moodle_database extends moodle_database { protected function free_descriptors($descriptors) { foreach ($descriptors as $descriptor) { + $descriptor->close(); oci_free_descriptor($descriptor); } } @@ -1036,8 +1052,10 @@ class oci_native_moodle_database extends moodle_database { list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_UPDATE); $stmt = $this->parse_query($sql); - $this->bind_params($stmt, $params); + $descriptors = []; + $this->bind_params($stmt, $params, null, $descriptors); $result = oci_execute($stmt, $this->commit_status); + $this->free_descriptors($descriptors); $this->query_end($result, $stmt); oci_free_statement($stmt); @@ -1100,8 +1118,10 @@ class oci_native_moodle_database extends moodle_database { list($rawsql, $params) = $this->tweak_param_names($rawsql, $params); $this->query_start($rawsql, $params, SQL_QUERY_SELECT); $stmt = $this->parse_query($rawsql); - $this->bind_params($stmt, $params); + $descriptors = []; + $this->bind_params($stmt, $params, null, $descriptors); $result = oci_execute($stmt, $this->commit_status); + $this->free_descriptors($descriptors); $this->query_end($result, $stmt); return $this->create_recordset($stmt); @@ -1135,8 +1155,10 @@ class oci_native_moodle_database extends moodle_database { list($rawsql, $params) = $this->tweak_param_names($rawsql, $params); $this->query_start($rawsql, $params, SQL_QUERY_SELECT); $stmt = $this->parse_query($rawsql); - $this->bind_params($stmt, $params); + $descriptors = []; + $this->bind_params($stmt, $params, null, $descriptors); $result = oci_execute($stmt, $this->commit_status); + $this->free_descriptors($descriptors); $this->query_end($result, $stmt); $records = null; @@ -1174,8 +1196,10 @@ class oci_native_moodle_database extends moodle_database { list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_SELECT); $stmt = $this->parse_query($sql); - $this->bind_params($stmt, $params); + $descriptors = []; + $this->bind_params($stmt, $params, null, $descriptors); $result = oci_execute($stmt, $this->commit_status); + $this->free_descriptors($descriptors); $this->query_end($result, $stmt); $records = null; @@ -1232,16 +1256,17 @@ class oci_native_moodle_database extends moodle_database { list($sql, $params, $type) = $this->fix_sql_params($sql, $params); $sql .= $returning; - $id = null; + $id = 0; // note we don't need tweak_param_names() here. Placeholders are safe column names. MDL-28080 // list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_INSERT); $stmt = $this->parse_query($sql); - $descriptors = $this->bind_params($stmt, $params, $table); if ($returning) { oci_bind_by_name($stmt, ":oracle_id", $id, 10, SQLT_INT); } + $descriptors = []; + $this->bind_params($stmt, $params, $table, $descriptors); $result = oci_execute($stmt, $this->commit_status); $this->free_descriptors($descriptors); $this->query_end($result, $stmt); @@ -1355,7 +1380,8 @@ class oci_native_moodle_database extends moodle_database { // list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_UPDATE); $stmt = $this->parse_query($sql); - $descriptors = $this->bind_params($stmt, $params, $table); + $descriptors = []; + $this->bind_params($stmt, $params, $table, $descriptors); $result = oci_execute($stmt, $this->commit_status); $this->free_descriptors($descriptors); $this->query_end($result, $stmt); @@ -1445,7 +1471,8 @@ class oci_native_moodle_database extends moodle_database { list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_UPDATE); $stmt = $this->parse_query($sql); - $descriptors = $this->bind_params($stmt, $params, $table); + $descriptors = []; + $this->bind_params($stmt, $params, $table, $descriptors); $result = oci_execute($stmt, $this->commit_status); $this->free_descriptors($descriptors); $this->query_end($result, $stmt); @@ -1476,8 +1503,10 @@ class oci_native_moodle_database extends moodle_database { list($sql, $params) = $this->tweak_param_names($sql, $params); $this->query_start($sql, $params, SQL_QUERY_UPDATE); $stmt = $this->parse_query($sql); - $this->bind_params($stmt, $params); + $descriptors = []; + $this->bind_params($stmt, $params, null, $descriptors); $result = oci_execute($stmt, $this->commit_status); + $this->free_descriptors($descriptors); $this->query_end($result, $stmt); oci_free_statement($stmt); From 373d1eb7d0c6bcd9fda5bdadd7d0b599f3d92782 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Tue, 12 Jul 2016 16:44:23 +0800 Subject: [PATCH 2/2] MDL-52544 oracle: Apply upstream pull request to AdoDB oracle driver Upstream: https://github.com/ADOdb/ADOdb/pull/259 Prevent segfault with ocipo driver on php7. The OCIFetchinto function is causing segfaults on php7 - probably because the fields array is not initialised or it is optimised out. This fixes just changes to use the safer function oci_fetch_array instead. --- lib/adodb/drivers/adodb-oci8po.inc.php | 13 +++++++++---- lib/adodb/readme_moodle.txt | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/adodb/drivers/adodb-oci8po.inc.php b/lib/adodb/drivers/adodb-oci8po.inc.php index cf76aa6aed5..43687308bac 100644 --- a/lib/adodb/drivers/adodb-oci8po.inc.php +++ b/lib/adodb/drivers/adodb-oci8po.inc.php @@ -138,8 +138,10 @@ class ADORecordset_oci8po extends ADORecordset_oci8 { // 10% speedup to move MoveNext to child class function MoveNext() { - if(@OCIfetchinto($this->_queryID,$this->fields,$this->fetchMode)) { + $ret = @oci_fetch_array($this->_queryID,$this->fetchMode); + if($ret !== false) { global $ADODB_ANSI_PADDING_OFF; + $this->fields = $ret; $this->_currentRow++; $this->_updatefields(); @@ -169,10 +171,12 @@ class ADORecordset_oci8po extends ADORecordset_oci8 { $arr = array(); return $arr; } - if (!@OCIfetchinto($this->_queryID,$this->fields,$this->fetchMode)) { + $ret = @oci_fetch_array($this->_queryID,$this->fetchMode); + if ($ret === false) { $arr = array(); return $arr; } + $this->fields = $ret; $this->_updatefields(); $results = array(); $cnt = 0; @@ -188,8 +192,9 @@ class ADORecordset_oci8po extends ADORecordset_oci8 { { global $ADODB_ANSI_PADDING_OFF; - $ret = @OCIfetchinto($this->_queryID,$this->fields,$this->fetchMode); + $ret = @oci_fetch_array($this->_queryID,$this->fetchMode); if ($ret) { + $this->fields = $ret; $this->_updatefields(); if (!empty($ADODB_ANSI_PADDING_OFF)) { @@ -198,7 +203,7 @@ class ADORecordset_oci8po extends ADORecordset_oci8 { } } } - return $ret; + return $ret !== false; } } diff --git a/lib/adodb/readme_moodle.txt b/lib/adodb/readme_moodle.txt index db762b11051..369c78d8faf 100644 --- a/lib/adodb/readme_moodle.txt +++ b/lib/adodb/readme_moodle.txt @@ -27,5 +27,6 @@ Our changes: * MDL-52286 fixed usage of /e in preg_replace, incorrect constructor in ADORecordSet_ext_mysqlt and ADORecordSet_mysqli::_close(), added muting erros in ADORecordSet::__destruct(). Check if fixed upstream during the next upgrade and remove this note. + * MDL-52544 Pull upstream patch for php7 and ocipo. skodak, iarenaza, moodler, stronk7