-2

I'm using a MySQLI API to try to make a modify system to change the price of a product. When I try to run this code I see no errors but nothing in my database changes. Here are the code and the table.

Database

<!doctype html>
<html>
<head>
<meta charset="utf-8">
<link href="css/design.css" rel="stylesheet">
<title>Home Page</title>
</head>

<body>
    <nav class="navbar">
        <table border="0" height="100%" class="tablenav" >
            <tr>
                <td class="logo">Mask Emporium
                </td>
                <td class="navcell"><a href="adminpage.html" class="linknav">Admin</a>
                </td>
            </tr>
        </table>
    </nav>
    <div align="center">
        <div align="center" class="container">
            <br>
            <img src="images/banner.png" alt="banner" width="100%">
            <br>
            <br>

<?php
//Gets input info for stock manager
 $price=$_POST['price'];
$ID=$_POST['mask_id'];
 
 //Connects to database
 $conn = new mysqli('localhost','teamavatar','teamavatarpass');
 
 //Selecting database
 $conn->select_db("teamavatar");
 
 //Querys the database and updates stock info with a error checker
 $query = "UPDATE stock SET price='".$price."' WHERE access= .$ID." or die("Error: ".mysql_error());
            
 $result = $conn->query($query);
 
 //Simply makes sure that info was logged, if not info must be inputted incorrectly
 if($result== TRUE){
     echo "The Mask Price has been updated";
 }
 

  $conn->close();

 
?>
        </div>
    </div>
</body>
</html>
Dharman
  • 30,962
  • 25
  • 85
  • 135
  • 1
    If you echo $query what is result? – Maxqueue Dec 06 '20 at 00:40
  • See about sql injection and the importance of prepared and bound queries – Strawberry Dec 06 '20 at 00:42
  • Maxqueue this is what happens when a echo the query "UPDATE stock SET price='677' WHERE access= .3." – Andrew Steven Dec 06 '20 at 00:44
  • You have an error. [`mysql_error()`](https://www.php.net/manual/en/function.mysql-error.php) worked only for the old API. Please consider switching error mode on instead. [How to get the error message in MySQLi?](https://stackoverflow.com/a/22662582/1839439) – Dharman Dec 06 '20 at 12:58

2 Answers2

2

Problems

Query

 $query = "UPDATE stock SET price='".$price."' WHERE access= .$ID." or die("Error: ".mysql_error());
  1. You don't have a column access in your database structure

    stock
       - id
       - name
       - price
       - inventory
       - type
    
  2. You're open to SQL injection because you're putting user generated content directly into a query which is run on the database

    price='".$price."' WHERE access= .$ID.
    
  3. You have periods either side of your variable so this wouldn't work even if access was the right column name

    .$ID. ==BECOMES==> .1.
    
  4. Reporting your errors with die is not an efficient way to do things

  5. In MySQL all column names are treated as lower case. It's good practice not to mix and match cases though!

Code

  1. You can access the database directly in the connection. There's not need to run a separate function to choose a database

    //Connects to database
    $conn = new mysqli('localhost','teamavatar','teamavatarpass');
    //Selecting database
    $conn->select_db("teamavatar");
    
  2. You haven't activated error reporting for mysqli which means you have a lot of additional code to check for errors needlessly

    mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); // Comes before connection
    
  3. Checking that the query completed the way you have doesn't prove much. Checking if/how many rows on the other hand would confirm exactly what has happened

    $result = $conn->query($query);
    
    //Simply makes sure that info was logged, if not info must be inputted incorrectly
    if($result== TRUE){
        echo "The Mask Price has been updated";
    }
    
  4. There's nothing stopping your query running with blank/empty values

  5. Again it's good to have consistency naming variables and functions don't mix

    // Common naming conventions
    $variableOne  = "..."; // Camel case
    $variable_two = "..."; // Snake case
    

Working solution

// Gets input values or sets variables to NULL if nothing has been posted
$price = $_POST['price']   ?? NULL;
$id    = $_POST['mask_id'] ?? NULL;

// Check we have input variables before running the code
if($price && $id){

    // Set error reporting and connect to the database
    mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
    $mysqli = new mysqli('localhost','teamavatar','teamavatarpass', 'teamavatar');
 

    $sql   = "UPDATE stock SET price = ? WHERE id = ?"; // Query with ? as placeholders for variables
    $query = $mysqli->prepare($sql);                    // Prepare query
    $query->bind_param("ii", $price, $id);              // Bind variables to place holders (assuming both are integers based on SS)
    $query->execute();                                  // Run the query
    echo $mysqli->affected_rows;                        // Print the number of rows updated in the query
 }
 else{
    echo "Nothing submitted.";
 }
Steven
  • 6,053
  • 2
  • 16
  • 28
-2

Are you sure the connection to the DB is successful? Try adding this under the connection code:

if ($conn->connect_error) {
   echo 'Error connecting to DB: ' . $mysqli->connect_error;
}
freefall
  • 319
  • 1
  • 8
  • yes it is connected properly, i have other code for removing rows and that works successfully – Andrew Steven Dec 06 '20 at 00:41
  • Then I'd try what @Maxqueue suggested and echo $query to make sure that is what you expect – freefall Dec 06 '20 at 00:44
  • 1
    You need to stop manually checking for errors. Please read: [Should we ever check for mysqli_connect() errors manually?](https://stackoverflow.com/q/58808332/1839439) and [Should I manually check for errors when calling “mysqli_stmt_prepare”?](https://stackoverflow.com/q/62216426/1839439) – Dharman Dec 06 '20 at 13:00