Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recently comitted rawAddPrefix() breaks queries #928

Closed
killua-eu opened this issue Oct 22, 2020 · 3 comments
Closed

Recently comitted rawAddPrefix() breaks queries #928

killua-eu opened this issue Oct 22, 2020 · 3 comments

Comments

@killua-eu
Copy link

killua-eu commented Oct 22, 2020

$query = 'UPDATE `t_fin_trx` SET `c_json` = JSON_SET(`c_json`, "$.id", `c_uid`) WHERE (NOT `c_uid` = c_json->>"$.id") or (NOT JSON_CONTAINS_PATH(c_json, "one", "$.id"));';
$this->db->rawQuery($query);

return str_replace($table[0], self::$prefix.$table[0], $query);
will result in an Undefined offset: 0 notice. What's the intent of the function, @bejutassle ? Would you kinly look into fixing it?

@vermeerpaul
Copy link

Another issue - same result, but a quick fix could be to replace the function with below suggestion to bypass the problem. The function assumes that all queries are applied on tables and that the 'list' call has a result. However in your case you may want to look at the use of the ` in your statements - below regex fixes that issue too. Either way the $table variable is null (empty) in both cases.
Slight performance improvement made to the function in case no prefix is being used.

BE AWARE ... This is not fully tested! So, use the applied changes to fit your needs.

function rawAddPrefix($query,$prefix){
if ($prefix == '') return $query;
$query = str_replace(PHP_EOL, null, $query);
$query = preg_replace('/\s+/', ' ', $query);
preg_match_all("/(from|into|update|join) [\'\´\]?([a-zA-Z0-9_-]+)[\\'\\´\\]?/i", $query, $matches);
list($from_table, $from, $table) = $matches;
if ($table == null) return $query;
return str_replace($table[0], $prefix.$table[0], $query);
}

@vermeerpaul
Copy link

Grrr ... I noticed that my test functions are a bit muddled up by github's editor. Here's the 'what to do to fix it' and use it in the class.

insert this line to skip the regex function when there's no prefix
if (self::$prefix == '') return $query;

insert this line after the 'list' function to check if there were any table names to prefix or not to prevent the $table[0] error
if ($table == null) return $query;

Change the REGEX to include your backtick [sets to now include; single quote, tick and backtick]
use ..... backslash backslash backtick .... to do so
and apply it to BOTH sets.

Hope it helps

@RyadPasha
Copy link

RyadPasha commented Dec 12, 2021

The rawAddPrefix function has many bugs, first thing is that only the first table in the query gets prefixed, (so if using query with a JOIN clause for example only first table will get prefixed), also a lot of statements such as (DROP TABLE, TRUNCATE TABLE, CREATE TABLE, LOCK TABLE, FLASHBACK TABLE, ALTER TABLE, ANALYZE TABLE, DESCRIBE and EXPLAIN) are not supported ..
You can fix all these bug by replacing that function with mines:

/**
 * Prefix add raw SQL query.
 *
 * @author Mohamed Riyad <https://github.com/RyadPasha>
 * @param string $query User-provided query to execute.
 * @return string Contains the returned rows from the query.
 */
public function rawAddPrefix($query){
    $query = preg_replace(['/[\r\n]+/', '/\s+/'], ' ', $query); // Replace multiple line breaks/spaces with a single space
    if (preg_match_all("/(FROM|INTO|UPDATE|JOIN|DROP TABLE|TRUNCATE TABLE|CREATE TABLE|LOCK TABLE|FLASHBACK TABLE|ALTER TABLE|ANALYZE TABLE|DESCRIBE|EXPLAIN) [\\'\\´\\`]?(?!SELECT|DELETE|INSERT|REPLACE|UPDATE)([a-zA-Z0-9_-]+)[\\'\\´\\`]?/i", $query, $matches)) {
        for ($i = 0; $i < count($matches[0]); $i++) {
            list($from_table, $from, $table) = $matches;
            $query = str_replace($table[$i], self::$prefix.$table[$i], $query);
        }
    }
    return $query;
}

ThingEngineer added a commit that referenced this issue May 24, 2024
…d issues (#995, #967, #928)

This commit fixes the "Undefined offset: 0" error that occurred in the rawAddPrefix function when using backtick delimiters for table names in SQL queries. It also addresses issues reported in tickets #995, #967, and #928, which were likely related to the same underlying problem with table name identification.

Changes:

Updated the regular expression in rawAddPrefix to match backtick characters (\) as valid delimiters for table names.

This fix ensures that the function correctly identifies and prefixes table names regardless of the delimiter used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants