0

I wrote this code, but it was pointed out by people here at stackoverflow that these functions will deprecate. So I'm updating it with mysqli functions. The new one won't return the image url i want to show though.

Here's the old working code :

<html>
<head>

<title>My first PHP script</title>
</head>
<body> 

<?php

$dbhost = 'access.website';
$dbname = 'my_db';
$dbuser = 'usr_nam';
$dbpass = 'passwrd';

$mysql_handle = mysql_connect($dbhost, $dbuser, $dbpass)
    or die("Error Connecting To Database Server");

mysql_select_db($dbname, $mysql_handle)
    or die("Error selecting database: $dbname");

$query = sprintf("SELECT image_url, Type FROM Pokemon 
    c WHERE c.name='%s'", 
    mysql_real_escape_string($_GET["fname"]));

$result = mysql_fetch_assoc(mysql_query($query));   

echo '<img height="450" width="330" src="'.$result['image_url'].'" />';

mysql_close($mysql_handle);

?>

</body>
</html>

And here is my new code:

<html>
<head>

  <title>My first PHP script</title>
</head>
<body> 

<?php

$dbhost = 'access.website';
$dbname = 'my_db';
$dbuser = 'usr_nam';
$dbpass = 'passwrd';

$link = mysqli_connect($dbhost,$dbuser,$dbpass,$dbname);

mysqli_select_db($link,$dbname);

$query = sprintf("SELECT image_url, Type FROM Pokemon 
    c WHERE c.name='%s'", 
    mysqli_real_escape_string($link,$_GET["fname"]));

$result = mysqli_query($link,$query);   

echo '<img height="450" width="330" src="'.$result['image_url'].'" />';

mysqli_close($link);

?>

</body>
</html>
Robert Bain
  • 395
  • 1
  • 7
  • 19

1 Answers1

3

You have not actually fetched a result from the $result resource via mysqli_fetch_assoc() or similar:

$result = mysqli_query($link,$query);   
$row = mysqli_fetch_assoc($result);

echo '<img height="450" width="330" src="'.$row['image_url'].'" />';

Another suggestion: Although you have switched to MySQLi, you are not receiving its primary security benefit through prepared statements. This would be better done with a prepared statement and placeholders:

// MySQLi object-oriented version with a prepared statement
$mysqli = new mysqli('host','user','pass','dbname');
// Prepare the query and placeholder
$stmt = $mysqli->prepare("SELECT image_url, Type FROM Pokemon c WHERE c.name=?");
// Bind input var & execute
$stmt->bind_param('s', $_GET['fname']);
$stmt->execute();
$stmt->bind_result($img_url)
$stmt->fetch();

echo '<img height="450" width="330" src="'.$img_url.'" />';

Or the non-OO version:

// $link is already defined
$stmt = mysqli_prepare($link, "SELECT image_url, Type FROM Pokemon c WHERE c.name=?");
mysqli_stmt_bind_param($stmt, 's', $_GET['fname']);
mysqli_stmt_execute($stmt);
// Bind output var & fetch
mysqli_stmt_bind_result($stmt, $img_url);
mysqli_stmt_fetch($stmt);
// $img_url now holds the value
Michael Berkowski
  • 267,341
  • 46
  • 444
  • 390
  • Could i also: $result = mysqli_fetch_assoc(mysqli_query($link,$query)); – Robert Bain Nov 08 '12 at 03:21
  • 1
    @RobertBain It is never recommended to nest a query call inside a fetch call, in case the query fails, the fetch would error out. – Michael Berkowski Nov 08 '12 at 03:25
  • @AnthonyRutledge Not overkill, [I generally aim to anticipate future questions and problems](http://meta.stackoverflow.com/questions/252439/how-to-answer-a-homework-question-when-the-entire-example-is-bad-syntax/252441#252441). When making the switch from the old `mysql_*()` to MySQLi or PDO, the most important thing to understand is how they can be used more securely, sending the OP down that path with a clear first example. In more generic examples the community generally [links to this question](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Michael Berkowski Jun 07 '14 at 11:59
  • 1
    @AnthonyRutledge It's what we make it. A lot of here us use it to improve the overall code quality leaking out into the world while also helping to solve the small immediate problems. – Michael Berkowski Jun 07 '14 at 12:01