-1

I have following php code, I try to remove record from SQL table and all associated image files from file system. I use glob() function with variable $list so get all image files start from $list and give output to unlink function but it doesn't work, I believe I do something wrong here:

<?
define('SITE',true);
include("../adm/conf.php");
$del=mysql_query("SELECT id FROM notes WHERE id = 26571"); 
$count=mysql_num_rows($del);
if(@$count){
    while($q=mysql_fetch_assoc($del)){
        $p_del=mysql_query("SELECT LEFT(logo,LOCATE('_',logo) - 1) imagefile FROM `notes` WHERE id_message = '".$q['id']."'");   
        if(@mysql_num_rows($p_del)){
            while($list=mysql_fetch_assoc($p_del)){             
                $filename=glob($list ."*");
                if(file_exists("../upload/notes/".$filename))unlink("../upload/notes/".$filename);
            }
        }
        mysql_query("DELETE FROM notes WHERE id='".$q['id']."'");       
    }
}
?>

Thanks, S

RiggsFolly
  • 83,545
  • 20
  • 96
  • 136
AliBaba
  • 13
  • 2
  • Every time you use [the `mysql_`](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) database extension in new code **[this happens](https://media.giphy.com/media/kg9t6wEQKV7u8/giphy.gif)** it is deprecated and has been for years and is gone for ever in PHP7.0+. If you are just learning PHP, spend your energies learning the `PDO` or `mysqli` database extensions and prepared statements. [Start here](http://php.net/manual/en/book.pdo.php) – RiggsFolly Apr 15 '20 at 09:18
  • Your script is open to [SQL Injection Attack](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php). Even [if you are escaping inputs, its not safe!](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) You should consider using [prepared parameterized statements](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) in either the `MYSQLI_` or `PDO` API's instead of concatenated values – RiggsFolly Apr 15 '20 at 09:19
  • 1
    Good code indentation would help us read the code and more importantly it will help **you debug your code** [Take a quick look at a coding standard](https://www.php-fig.org/psr/psr-12/) for your own benefit. You may be asked to amend this code in a few weeks/months and you will thank me in the end. – RiggsFolly Apr 15 '20 at 09:21

1 Answers1

0

glob() always returns an array, consisted results, and $list is also an array, so it should be

foreach ($list as $itemToDelete) {
    $filenames=glob($itemToDelete ."*");
    foreach($filenames as $filename) {
        if(file_exists("../upload/notes/".$filename)) {
            unlink("../upload/notes/".$filename);
        }
    }
}

I would recommend for more short and clear search/deleting as

while($list=mysql_fetch_assoc($p_del)){
    foreach ($list as $itemToDelete) {   
        array_map('unlink', glob("../upload/notes/{$itemToDelete}*"));
    }
}
Alex.Default
  • 226
  • 2
  • 16
  • tried with array_walk but for some reason doesn't work – AliBaba Apr 15 '20 at 10:28
  • maybe it doesn't like {$list}* in quote in glob? – AliBaba Apr 15 '20 at 10:40
  • Double quotes make available to include variable's value to a string. – Alex.Default Apr 15 '20 at 10:42
  • Are you sure that $list has any value? I see error suppressing, it's not good practice and can lead to unknown errors. – Alex.Default Apr 15 '20 at 10:44
  • Here how I put: while($q=mysql_fetch_assoc($del)){ $p_del=mysql_query("SELECT LEFT(logo,LOCATE('_',logo) - 1) imagefile FROM `jb_news` WHERE id_message = '".$q['id']."'"); if(@mysql_num_rows($p_del)){ while($list=mysql_fetch_assoc($p_del)){ array_walk('unlink', glob("../upload/news/{$list}*")); } } } – AliBaba Apr 15 '20 at 10:47
  • Have you debugged that `$list` has any value by that point? For example, does `var_dump($list) ` output any value? – Alex.Default Apr 15 '20 at 10:53
  • it gives output for $list it seems unlink or glob doesn't like it – AliBaba Apr 15 '20 at 11:14
  • Since `$list` is an array too, I updated my code. – Alex.Default Apr 15 '20 at 11:22
  • With first method script run but doesn't remove files, with second method(array_walk) script doesn't run. – AliBaba Apr 15 '20 at 11:43
  • Fixed. I strongly recommend to enable PHP errors, as was suggested in your's post comment, so you could easily see any code mistakes and their reasons. – Alex.Default Apr 15 '20 at 12:07
  • sorry, you said Fixed, what did you fix? – AliBaba Apr 15 '20 at 12:12
  • I see you changed to array_map. – AliBaba Apr 15 '20 at 12:28
  • now the second method runs but doesn't remove the files, trying to debug it but don't know how get the output to the browser inside while. – AliBaba Apr 15 '20 at 13:03
  • I've tested unlink locally, so I believe the problem is behind there's so files at all in the noted directory. You can output result like this: ```php while($list=mysql_fetch_assoc($p_del)){ foreach ($list as $itemToDelete) { var_dump(%info you need%); array_map('unlink', glob("../upload/notes/{$itemToDelete}*")); } } ``` – Alex.Default Apr 15 '20 at 13:19
  • I also suggest to google for some material about debugging a web application, it would be definitely better explained than I could do that =) – Alex.Default Apr 15 '20 at 13:21
  • sorry, the second method works, my fault the second sql query field name in condition was wrong. Thanks a lot to Alex.Default!! – AliBaba Apr 15 '20 at 13:39
  • and our change "foreach ($list as $itemToDelete)" with array_map perfectly works. Thanks again!! – AliBaba Apr 15 '20 at 13:51