Skip to content

Commit

Permalink
+ A small overhaul of the database class.
Browse files Browse the repository at this point in the history 
  Renamed wesql::query_* to wesql::get_*, to make it clearer these functions do all the work for you.
  wesql::get() will now return a single variable (instead of an array) if it only returns a single column & row. $my_var = wesql::get('SELECT my_var FROM table LIMIT 1'), welcome to a world where MySQL isn't so tedious anymore.
  $db_string renamed to $query internally (no impact expected).
  Added placeholder for a future utf8mb4 conversion function (I'm on the edge regarding this, as I'd rather add support in the installer itself).

* Updated uses of wesql::query_* (Load.php, ManagePlugins.php, Subs-BoardIndex.php)

! Fixed sessionRead() to return an empty string on failure, instead of 'false', since these callbacks functions are now properly documented at php.net. (Load.php)

! Rare bug in the error viewer. (ManageErrors.php)

Signed-off-by: Nao <nao@wedge>
  • Loading branch information
@Nao
Nao committed Sep 16, 2016
1 parent 241b8c5 commit 14c4ea6
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 59 deletions.
100 changes: 58 additions & 42 deletions core/app/Class-DB.php
View file
Expand Up @@ -79,27 +79,27 @@ public static function fix_prefix(&$db_prefix, $db_name)
self::register_replacement('db_prefix', $db_prefix);
}

public static function quote($db_string, $db_values, $connection = null)
public static function quote($query, $db_values, $connection = null)
{
global $db_callback;

// Only bother if there's something to replace.
if (strpos($db_string, '{') !== false)
if (strpos($query, '{') !== false)
{
// This is needed by the callback function.
$db_callback = array($db_values, $connection == null ? self::$_db_con : $connection);

// Do the quoting and escaping
$db_string = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', 'wesql::replace_value', $db_string);
$query = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', 'wesql::replace_value', $query);

// Clear this global variable.
$db_callback = array();
}

return $db_string;
return $query;
}

public static function query($db_string, $db_values = array(), $connection = null)
public static function query($query, $db_values = array(), $connection = null)
{
global $db_cache, $db_count, $db_show_debug, $time_start;
global $db_unbuffered, $db_callback, $settings;
Expand All @@ -124,27 +124,27 @@ public static function query($db_string, $db_values = array(), $connection = nul
// One more query....
$db_count = !isset($db_count) ? 1 : $db_count + 1;

if (empty($settings['disableQueryCheck']) && strpos($db_string, '\'') !== false && empty($db_values['security_override']))
if (empty($settings['disableQueryCheck']) && strpos($query, '\'') !== false && empty($db_values['security_override']))
wesql::error_backtrace('Hacking attempt...', 'Illegal character (\') used in query...', true);

// Use "ORDER BY null" to prevent Mysql doing filesorts for Group By clauses without an Order By
if (strpos($db_string, 'GROUP BY') !== false && strpos($db_string, 'ORDER BY') === false && strpos($db_string, 'INSERT') === false && strpos($db_string, 'REPLACE') === false)
if (strpos($query, 'GROUP BY') !== false && strpos($query, 'ORDER BY') === false && strpos($query, 'INSERT') === false && strpos($query, 'REPLACE') === false)
{
// Add before LIMIT
if ($pos = strpos($db_string, 'LIMIT '))
$db_string = substr($db_string, 0, $pos) . "\t\t\tORDER BY null\n" . substr($db_string, $pos, strlen($db_string));
if ($pos = strpos($query, 'LIMIT '))
$query = substr($query, 0, $pos) . "\t\t\tORDER BY null\n" . substr($query, $pos, strlen($query));
else
// Append it.
$db_string .= "\n\t\t\tORDER BY null";
$query .= "\n\t\t\tORDER BY null";
}

if (empty($db_values['security_override']) && (!empty($db_values) || strpos($db_string, '{db_prefix}') !== false))
if (empty($db_values['security_override']) && (!empty($db_values) || strpos($query, '{db_prefix}') !== false))
{
// Pass some values to the global space for use in the callback function.
$db_callback = array($db_values, $connection);

// Inject the values passed to this function.
$db_string = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', 'wesql::replace_value', $db_string);
$query = preg_replace_callback('~{([a-z_]+)(?::([a-zA-Z0-9_-]+))?}~', 'wesql::replace_value', $query);

// This shouldn't be residing in global space any longer.
$db_callback = array();
Expand All @@ -169,7 +169,7 @@ public static function query($db_string, $db_values = array(), $connection = nul

$st = microtime(true);
// Don't overload it.
$db_cache[$db_count]['q'] = $db_count < 50 ? $db_string : '...';
$db_cache[$db_count]['q'] = $db_count < 50 ? $query : '...';
$db_cache[$db_count]['f'] = $file;
$db_cache[$db_count]['l'] = $line;
$db_cache[$db_count]['s'] = $st - $time_start;
Expand All @@ -183,15 +183,15 @@ public static function query($db_string, $db_values = array(), $connection = nul
$pos = -1;
while (true)
{
$pos = strpos($db_string, '\'', $pos + 1);
$pos = strpos($query, '\'', $pos + 1);
if ($pos === false)
break;
$clean .= substr($db_string, $old_pos, $pos - $old_pos);
$clean .= substr($query, $old_pos, $pos - $old_pos);

while (true)
{
$pos1 = strpos($db_string, '\'', $pos + 1);
$pos2 = strpos($db_string, '\\', $pos + 1);
$pos1 = strpos($query, '\'', $pos + 1);
$pos2 = strpos($query, '\\', $pos + 1);
if ($pos1 === false)
break;
elseif ($pos2 == false || $pos2 > $pos1)
Expand All @@ -206,7 +206,7 @@ public static function query($db_string, $db_values = array(), $connection = nul

$old_pos = $pos + 1;
}
$clean .= substr($db_string, $old_pos);
$clean .= substr($query, $old_pos);
$clean = trim(strtolower(preg_replace($allowed_comments_from, $allowed_comments_to, $clean)));

// Comments? We don't use comments in our queries, we leave 'em outside!
Expand All @@ -219,13 +219,13 @@ public static function query($db_string, $db_values = array(), $connection = nul
$fail = true;

if (!empty($fail) && function_exists('log_error'))
self::error_backtrace('Hacking attempt...', 'Hacking attempt...' . "\n" . $db_string, E_USER_ERROR);
self::error_backtrace('Hacking attempt...', 'Hacking attempt...' . "\n" . $query, E_USER_ERROR);
}

$ret = @mysqli_query($connection, $db_string, empty($db_unbuffered) ? MYSQLI_STORE_RESULT : MYSQLI_USE_RESULT);
$ret = @mysqli_query($connection, $query, empty($db_unbuffered) ? MYSQLI_STORE_RESULT : MYSQLI_USE_RESULT);

if ($ret === false && empty($db_values['db_error_skip']))
$ret = self::serious_error($db_string, $connection);
$ret = self::serious_error($query, $connection);

// Debugging.
if (!empty($db_show_debug))
Expand Down Expand Up @@ -266,7 +266,7 @@ public static function error($connection = null)
return mysqli_error($connection === null ? self::$_db_con : $connection);
}

public static function serious_error($db_string, $connection = null)
public static function serious_error($query, $connection = null)
{
global $txt, $context, $webmaster_email, $settings, $db_last_error, $db_persist;
global $db_server, $db_user, $db_passwd, $db_name, $db_show_debug, $ssi_db_user, $ssi_db_passwd;
Expand Down Expand Up @@ -299,7 +299,7 @@ public static function serious_error($db_string, $connection = null)

// Log the error.
if ($query_errno != 1213 && $query_errno != 1205 && function_exists('log_error'))
log_error((empty($txt) ? 'Database error' : $txt['database_error']) . ': ' . $query_error . (!empty($settings['enableErrorQueryLogging']) ? "\n\n$db_string" : ''), 'database', $file, $line);
log_error((empty($txt) ? 'Database error' : $txt['database_error']) . ': ' . $query_error . (!empty($settings['enableErrorQueryLogging']) ? "\n\n$query" : ''), 'database', $file, $line);

// Database error auto fixing ;)
if (function_exists('cache_get_data') && (!isset($settings['autoFixDatabase']) || $settings['autoFixDatabase'] == '1'))
Expand All @@ -316,7 +316,7 @@ public static function serious_error($db_string, $connection = null)
// We know there's a problem... but what? Try to auto detect.
if ($query_errno == 1030 && strpos($query_error, ' 127 ') !== false)
{
preg_match_all('~(?:[\n\r]|^)[^\']+?(?:FROM|JOIN|UPDATE|TABLE) ((?:[^\n\r(]+?(?:, )?)*)~s', $db_string, $matches);
preg_match_all('~(?:[\n\r]|^)[^\']+?(?:FROM|JOIN|UPDATE|TABLE) ((?:[^\n\r(]+?(?:, )?)*)~s', $query, $matches);

$fix_tables = array();
foreach ($matches[1] as $tables)
Expand Down Expand Up @@ -364,7 +364,7 @@ public static function serious_error($db_string, $connection = null)
$settings['cache_enable'] = $old_cache;

// Try the query again...?
$ret = self::query($db_string, false, false);
$ret = self::query($query, false, false);
if ($ret !== false)
return $ret;
}
Expand Down Expand Up @@ -393,7 +393,7 @@ public static function serious_error($db_string, $connection = null)
// Try a deadlock more than once more.
for ($n = 0; $n < 4; $n++)
{
$ret = self::query($db_string, false, false);
$ret = self::query($query, false, false);

$new_errno = mysqli_errno(self::$_db_con);
if ($ret !== false || in_array($new_errno, array(1205, 1213)))
Expand All @@ -412,7 +412,7 @@ public static function serious_error($db_string, $connection = null)

// Nothing's defined yet... just die with it.
if (empty($context) || empty($txt))
exit($db_string . '<br><br>' . $query_error);
exit($query . '<br><br>' . $query_error);

// Show an error message, if possible.
$context['error_title'] = $txt['database_error'];
Expand All @@ -422,7 +422,7 @@ public static function serious_error($db_string, $connection = null)
$context['error_message'] = $txt['try_again'];

if (allowedTo('admin_forum') && !empty($db_show_debug))
$context['error_message'] .= '<br><br>' . nl2br($db_string, false);
$context['error_message'] .= '<br><br>' . nl2br($query, false);

// It's already been logged... don't log it again.
fatal_error($context['error_message'], false);
Expand Down Expand Up @@ -564,7 +564,7 @@ public static function replace_value($matches)
return (string) (int) $replacement;

case 'string':
return sprintf('\'%1$s\'', mysqli_real_escape_string($connection, $replacement));
return sprintf('\'%1$s\'', handle_utf8mb4(mysqli_real_escape_string($connection, $replacement)));

case 'array_int':
if (is_array($replacement))
Expand Down Expand Up @@ -592,7 +592,7 @@ public static function replace_value($matches)
self::error_backtrace('Database error, given array of string values is empty. (' . $matches[2] . ')', '', E_USER_ERROR);

foreach ($replacement as $key => $value)
$replacement[$key] = sprintf('\'%1$s\'', mysqli_real_escape_string($connection, $value));
$replacement[$key] = sprintf('\'%1$s\'', handle_utf8mb4(mysqli_real_escape_string($connection, $value)));

return implode(', ', $replacement);
}
Expand All @@ -618,6 +618,11 @@ public static function replace_value($matches)
}
}

public static handle_utf8mb4($str)
{
return $str;
}

public static function error_backtrace($error_message, $log_message = '', $error_type = false, $file = null, $line = null)
{
if (empty($log_message))
Expand Down Expand Up @@ -710,33 +715,44 @@ public static function free_result($result)
return mysqli_free_result($result);
}

public static function query_get($db_string, $db_values = array(), $connection = null, $job = 'assoc')
/*
A simplified query system.
- get() returns a variable if a single column was specified. (Use get_assoc() if this is unwanted.)
- Returns an array of variables if multiple columns were specified.
Use get_all() or get_rows() to get an array of arrays. Avoid on large tables, it would hurt memory.
On other methods (get/get_assoc/get_row), LIMIT 1 is implied, but you should add it for better performance.
$single_var = wesql::get('SELECT var1 FROM table');
$array_of_assoc_rows = wesql::get_all('SELECT var1 FROM table');
list ($var1, $var2) = wesql::get('SELECT var1, var2 FROM table LIMIT 1');
*/
public static function get($query, $db_values = array(), $connection = null, $job = '')
{
$request = self::query($db_string, $db_values, $connection);
$results = call_user_func('self::fetch_' . $job, $request);
$request = self::query($query, $db_values, $connection);
$results = call_user_func('self::fetch_' . ($job ?: 'assoc'), $request);
wesql::free_result($request);

return $results;
return $job || !is_array($results) || count($results) > 1 ? $results : reset($results);
}

public static function query_assoc($db_string, $db_values = array(), $connection = null)
public static function get_assoc($query, $db_values = array(), $connection = null)
{
return self::query_get($db_string, $db_values, $connection, 'assoc');
return self::get($query, $db_values, $connection, 'assoc');
}

public static function query_row($db_string, $db_values = array(), $connection = null)
public static function get_row($query, $db_values = array(), $connection = null)
{
return self::query_get($db_string, $db_values, $connection, 'row');
return self::get($query, $db_values, $connection, 'row');
}

public static function query_all($db_string, $db_values = array(), $connection = null)
public static function get_all($query, $db_values = array(), $connection = null)
{
return self::query_get($db_string, $db_values, $connection, 'all');
return self::get($query, $db_values, $connection, 'all');
}

public static function query_rows($db_string, $db_values = array(), $connection = null)
public static function get_rows($query, $db_values = array(), $connection = null)
{
return self::query_get($db_string, $db_values, $connection, 'rows');
return self::get($query, $db_values, $connection, 'rows');
}

public static function data_seek($result, $row_num)
Expand Down
10 changes: 4 additions & 6 deletions core/app/Load.php
View file
Expand Up @@ -994,20 +994,18 @@ function loadMemberData($users, $is_name = false, $set = 'normal')
// Load last post data. Can't restrict to $loaded_ids, in case a previous user was loaded through a non-profile set...
if (!empty($loaded_ids) && $set == 'profile')
{
$row = wesql::query_all('
$last = wesql::get_assoc('
SELECT m.id_member, m.poster_time, m.id_msg, m.id_topic, m.subject
FROM {db_prefix}messages AS m
LEFT JOIN {db_prefix}topics AS t ON t.id_topic = m.id_topic
WHERE m.id_member' . (count($new_loaded_ids) == 1 ? ' = {int:loaded_ids}' : ' IN ({array_int:loaded_ids})') . '
AND {query_see_topic}
ORDER BY m.id_msg DESC
LIMIT 1',
array(
'loaded_ids' => count($new_loaded_ids) == 1 ? $new_loaded_ids[0] : $new_loaded_ids,
)
array('loaded_ids' => count($new_loaded_ids) == 1 ? $new_loaded_ids[0] : $new_loaded_ids)
);

foreach ($row as $last)
if (!empty($last))
$user_profile[$last['id_member']]['last_post'] = array(
'on_time' => on_timeformat($last['poster_time']),
'link' => '<a href="' . SCRIPT . '?topic=' . $last['id_topic'] . '.msg' . $last['id_msg'] . '#new">' . $last['subject'] . '</a>',
Expand Down Expand Up @@ -2319,7 +2317,7 @@ function sessionRead($session_id)
list ($sess_data) = wesql::fetch_row($result);
wesql::free_result($result);

return isset($sess_data) ? $sess_data : false;
return empty($sess_data) ? '' : $sess_data;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions core/app/ManageErrors.php
View file
Expand Up @@ -211,8 +211,8 @@ function ViewErrorLog()
foreach ($context['errors'] as $id => $dummy)
{
$memID = $context['errors'][$id]['member']['id'];
$context['errors'][$id]['member']['username'] = $members[$memID]['member_name'];
$context['errors'][$id]['member']['name'] = $members[$memID]['real_name'];
$context['errors'][$id]['member']['username'] = isset($members[$memID]['member_name']) ? $members[$memID]['member_name'] : '(?)';
$context['errors'][$id]['member']['name'] = isset($members[$memID]['real_name']) ? $members[$memID]['real_name'] : '(?)';
$context['errors'][$id]['member']['href'] = empty($memID) ? '' : '<URL>?action=profile;u=' . $memID;
$context['errors'][$id]['member']['link'] = empty($memID) ? $txt['guest_title'] : '<a href="<URL>?action=profile;u=' . $memID . '">' . $context['errors'][$id]['member']['name'] . '</a>';
}
Expand Down
11 changes: 5 additions & 6 deletions core/app/ManagePlugins.php
View file
Expand Up @@ -1594,25 +1594,24 @@ function editPluginRepo()
$repo_id = (int) $_GET['editrepo'];
if ($repo_id > 0)
{
$query = wesql::query('
$row = wesql::get_assoc('
SELECT id_server, name, url, username, password, status
FROM {db_prefix}plugin_servers
WHERE id_server = {int:repo}',
WHERE id_server = {int:repo}
LIMIT 1',
array(
'repo' => $repo_id,
)
);
while ($row = wesql::fetch_assoc($query))
if (!empty($row))
$context['repository'] = array(
'name' => $row['name'],
'url' => $row['url'],
'username' => $row['username'],
'password' => $row['password'],
'status' => $row['status'],
'id_server' => $repo_id,
);
wesql::free_result($query);
if (!empty($context['repository']))
$context['repository']['id_server'] = $repo_id;
else
fatal_lang_error('plugins_edit_invalid_error', false);
}
Expand Down
4 changes: 2 additions & 2 deletions core/app/Subs-BoardIndex.php
View file
Expand Up @@ -78,11 +78,11 @@ function getBoardIndex($boardIndexOptions)
// Run through the categories and boards (or only boards)....
while ($row_board = wesql::fetch_assoc($result_boards))
{
// If our Last Message in this particular board isn't visible to us, we should recalculate it.
// If our last message in this particular board isn't visible to us, we should recalculate it.
// This has a small cost, but it's better than hurting people by saying they can't read something.
if (empty($row_board['topic_exists']) && $row_board['num_topics'] > 0)
{
$visible_board = wesql::query_get('
$visible_board = wesql::get('
SELECT
IFNULL(m.poster_time, 0) AS poster_time, IFNULL(mem.member_name, m.poster_name) AS poster_name,
m.subject, m.id_topic, IFNULL(mem.real_name, m.poster_name) AS real_name,
Expand Down
2 changes: 1 addition & 1 deletion core/app/Subs-Editor.php
View file
Expand Up @@ -28,7 +28,7 @@ function getMessageIcons($board_id)
{
$icon_data = cache_get_data('posting_icons-' . $board_id, 480, function () use ($board_id)
{
return wesql::query_all('
return wesql::get_all('
SELECT title, filename
FROM {db_prefix}message_icons
WHERE id_board IN (0, {int:board_id})',
Expand Down

0 comments on commit 14c4ea6

Please sign in to comment.