durfy Posted September 21, 2015 Share Posted September 21, 2015 (edited) Hello guys. I have little problem with my MySQL. home/xxx/set.php It is: Warning: mysql_fetch_assoc(): supplied argument is not a valid MySQL result resource in home/xxx/set.php on line 10 Here is set.php: <?php $sitename = "http://xxx.xxx"; $link = mysql_connect("localhost", "xxx_admin", "xxxxxx"); $db_selected = mysql_select_db('xxx_database1', $link); mysql_query("SET NAMES utf8"); function fetchinfo($rowname,$tablename,$finder,$findervalue) { if($finder == "1") $result = mysql_query("SELECT $rowname FROM $tablename"); else $result = mysql_query("SELECT $rowname FROM $tablename WHERE `$finder`='$findervalue'"); $row = mysql_fetch_assoc($result); return $row[$rowname]; } ?> Whats wrong ? Can u help me guys ? Please. :') Edited September 21, 2015 by durfy Quote Link to comment Share on other sites More sharing options...
cyberRobot Posted September 21, 2015 Share Posted September 21, 2015 Have you checked if MySQL is throwing errors? More information about how you would do that can be found here: http://php.net/manual/en/function.mysql-error.php Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted September 21, 2015 Share Posted September 21, 2015 I'm sorry to tell you, durfy, but pretty much every single line of the code is wrong. The mysql_* functions are ancient and will be removed in the future. Your PHP interpreter should actually tell you that (unless you're suppressing all warnings). You can't just insert variables directly into query strings. What if there's a space in $rowname or $tablename? What if there's a single quote in $findervalue? What if somebody deliberately tries to manipulate the query through any of the input (aka SQL injection)? Each of this will either result in a syntax error or a full-blown security vulnerability. Even if you happen to escape the input at some earlier point, your use of SET NAMES may break the escape mechanism, because it changes the database connection encoding without notifying PHP. You're not doing any error checking at all, so the code will continue after a failed query and crash at a later point with a weird error message (which is exactly what happened). So you have quite a lot of work ahead of you. If there's any chance to modernize the code and switch to a contemporary database interface like PDO, do that. This also allows you to use prepared statements and avoid the risk of SQL injections. If you're stuck with the old code, you need to manually escape every single input and check the return value of every single mysql_ function call (false means there was an error). And replace the SET NAMES query with mysql_set_charset(). Quote Link to comment Share on other sites More sharing options...
Barand Posted September 21, 2015 Share Posted September 21, 2015 Just correcting a typo. mysql_set_charset() should be mysqli_set_charset() Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted September 21, 2015 Share Posted September 21, 2015 No, I meant mysql_set_charset() as a fallback solution in case durfy has to stick to the old MySQL extension. If it's possible to modernize the code, I'd use PDO rather than MySQLi. It's much more convenient and not bound to MySQL in particular. Quote Link to comment Share on other sites More sharing options...
ginerjm Posted September 21, 2015 Share Posted September 21, 2015 Besides what others are telling you has to be fixed, what are you seeing wrong with your code? You don't check anything so it can't tell you if something is wrong. AND - since you simply make a connection and run a query, what are you missing? Your code as written will not tell you anything. Do you know what a function is? Quote Link to comment Share on other sites More sharing options...
hansford Posted September 22, 2015 Share Posted September 22, 2015 (edited) This should be noted: the following code is meant as an example for how to query a database using trusted variable data. If the variables you are embedding in the sql string are from forms or other sources, then this data needs to be sanitized first and the code modified to use prepared statements. The mysqli way: $link = new mysqli('localhost', 'xxx_admin', 'xxxxxx','xxx_database1'); if($link->connect_errno) { echo 'Database connection failed: ' . $link->connect_error; exit(); } if( ! $link->set_charset('utf8')) { echo 'Error loading character set: ' . $link->error; } $sql = "SELECT $rowname FROM $tablename WHERE $finder='$findervalue' LIMIT 1"; if( ! $result = $link->query($sql)) { echo 'No results returned from query'; exit(); } The PDO way: try { $link = new PDO('mysql:host=localhost;dbname=xxx_database1','xxx_admin','xxxxxx', array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8')); $con->setAttribute(PDO::ATTR_ERRMODE,PDO::ERRMODE_EXCEPTION); } catch(PDOExecption $e) { echo $e->getMessage(); exit(); } $sql = "SELECT $rowname FROM $tablename WHERE $finder='$findervalue' LIMIT 1"; $result = $link->query($sql); $row = $result->fetch(PDO::FETCH_ASSOC); Edited September 22, 2015 by hansford Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted September 22, 2015 Share Posted September 22, 2015 (edited) This is better than the original code, but it still has a lot of issues. Relying on “trusted” values is very risky and has lead to many security vulnerabilities in the past. A much better approach is to pass all input to a prepared statement and not take any chances. SQL identifiers (table names, field names etc.) also must be whitelisted, possibly even escaped and quoted. That's not only a security measure, it's also a matter of robustness. Many people use nonstandard identifiers which will crash the query when there's no appropriate quoting. Never show internal database errors to the user. That's simply none of their business, and you may leak critical information to attackers. Don't catch exceptions unless you know exactly what you're doing. Do not use SET NAMES as I already said above. This can be used for SQL injection attacks in certain scenarios. In fact, using highly dynamic queries where even the identifiers aren't predefined is stretching the limits of all standard database interfaces. That's simply not what they're made for. If you need this all the time, consider switching to a query builder. Or simply avoid those queries and use hard-coded identifiers. The function you've posted isn't really that useful, anyway. As to PDO and MySQLi in general, you'll want something like this: PDO <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; /* * Set up the database connection * - the character encoding *must* be defined in the DSN string, not with a SET NAMES query * - be sure to turn off "emulated prepared statements" to prevent SQL injection attacks * - turn on exceptions so that you don't have to manually check for errors */ $dSN = 'mysql:host='.DB_HOST.';dbname='.DB_NAME.';charset='.DB_CHARSET; $databaseConnection = new PDO($dSN, DB_USER, DB_PASSWORD, [ PDO::ATTR_EMULATE_PREPARES => false, // activate real prepared statements PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // activate exceptions PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, // fetch associative arrays by default (this isn't critical) ]); // Example: create a prepared statement with two parameters $exampleQuery = $databaseConnection->prepare(' SELECT :left_operand + :right_operand AS sum_result '); $leftOperand = 1; $rightOperand = 2; // Execute statement and pass values to the parameters $exampleQuery->execute([ 'left_operand' => $leftOperand, 'right_operand' => $rightOperand, ]); // Fetch result $exampleResult = $exampleQuery->fetch(); // Inspect result var_dump($exampleResult); MySQLi <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; // Turn on exceptions so that you don't have to manually check for errors $mysqliDriver = new mysqli_driver(); $mysqliDriver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; $databaseConnection = new mysqli('localhost', DB_USER, DB_PASSWORD, DB_NAME); // Define the character encoding; do not use a SET NAMES query $databaseConnection->set_charset(DB_CHARSET); // Example: create a prepared statement with four parameters $exampleQuery = $databaseConnection->prepare(' SELECT ? + ? AS sum_result '); $leftOperand = 1; $rightOperand = 2; // Bind values to the parameters $exampleQuery->bind_param('ii', $leftOperand, $rightOperand); // Execute statement $exampleQuery->execute(); // Bind results to variables and fetch them $exampleQuery->bind_result($sumResult, $productResult); $exampleQuery->fetch(); // Inspect result var_dump($sumResult, $productResult); As you can see, MySQLi is incredibly verbose, and it's naturally MySQL-only. With PDO you get a universal database interface with a much nicer API. Edited September 22, 2015 by Jacques1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.