[RESOLVED] SQL error when trying to delete an image from my gallery. Plz help!

Paul_help_

Hi guys. I keep getting an SQL error when trying to delete an image from a gallery I have been working on for a customer.

I have been building him an image uploader application for his website and I currently have most of the mechanics in place so it functions correctly.

The only thing I need to do is give my client the ability to choose a gallery he wants to delete an image from and and then click a delete link to delete the image.

I have managed to set everything up on his admin page so he can choose an image to delete and then click a delete link.

I have also set things up so when a delete link is clicked, it then triggers a method inside a class.

This method is pasted below:

public static function deleteimage( $deleteimggallery, $deleteimg ) {

$conn = parent::connect();
$sql = "DELETE FROM :deleteimggallery WHERE img_name = :deleteimg";

try{
$st = $conn->prepare($sql);
$st->bindValue(":deleteimggallery", $deleteimggallery, PDO::PARAM_STR);
$st->bindValue(":deleteimg", $deleteimg, PDO::PARAM_STR);
$st->execute();
}catch(PDOException $e){
parent::disconnect($conn);
die( "query failed: " . $e->getMessage() );
}

}

...... and I keep getting the following error message:

query failed: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''paths' WHERE img_name = '201707112136247.jpg'' at line 1

And I can't figure out how to fix it.

By the way,,, just to let you know ( in the above error ),,, ''paths' is the name of one of the galleries tables and 'img_name' is one of the columns inside each gallery table.

Also, the 2 parameters passed to the method (above):

$deleteimggallery, $deleteimg

.... hold the image gallery name and the image name (to delete), respectively.

Paul.

pbismad

Firstly, some points about what you are or are not doing -

1) You should be using a post method form when modifying data on the server. This will prevent things like search engines indexing your site from triggering the deletion code.

2) You should not have a separate table for each gallery. To support multiple galleries, you would have a gallery table, where the defining gallery information is stored, and then you would use the gallery id from that table to associate the images to the gallery they belong to. All the image data would be stored in a single table.

3) You should use the id (auto-increment column) of the image to reference the correct row when modifying data.

4) You should not make and close a database connection for each query. The connection is a time consuming process, taking even longer then the time it takes to send the sql query statement from php to the database server. Your main application code should make one database connection and supply it to any code that needs to use the connection.

5) Based on the error message (with the actual values being shown in the error), you are using emulated prepared queries, which is the default. Emulated prepared queries are open to sql injection if you haven't set the character set for the connection to match your database character set. You need to set the PDO emulated prepared query setting to a false value to use real prepared queries. You also should always set the character set of the connection to match your database character set.

6) You should never output the actual php/mysql error information on a live site, i.e. the $e->getMessage() information. If you eliminate the try/catch logic, and let php catch the PDO exception, php will use it's error_reporting/display_errors/log_errors settings to control what happens with the actual error information. When developing and debugging, you would display the errors. When on a live server, you would log the errors.

Lastly, you cannot bind table names. You can only bind data values. If you organize the database tables properly, as outlined in this reply, you won't even need to dynamically supply a table name. If you did have a need to dynamically supply a table name, you would need to validate that it is exactly and only a permitted table name (there's no way of preventing sql injection in the parts of the query that are not data values) and that the current visitor has permission to operate on the selected table name, then directly put the table name into the sql query statement.

NogDog

Yeah, you cannot use a bound parameter place-holder for a table or column identifier, only for actual column values. Also, you probably want to use the bindParam() method, not bindValue() method for the image name.