0

I'm trying to add a category for images. Everything is fine working in my code. But the image is not moving to the image folder. I see the file name in the database column, so what is wrong in my php code?

I don't see any obvious syntax error:

<?php
require('top.inc.php');
$categories = '';
$msg        = '';
if (isset($_GET['id']) && $_GET['id'] != '') {
$id    = get_safe_value($link, $_GET['id']);
$res   = mysqli_query($link, "select * from categories where id='$id'");
$check = mysqli_num_rows($res);
if ($check > 0) {
    $row        = mysqli_fetch_assoc($res);
    $categories = $row['categories'];
} else {
    header('location:categories.php');
    die();
}
}


if (isset($_POST['submit'])) {
$filename   = $_FILES["uploadfile"]["name"];
$tempname   = $_FILES["uploadfile"]["tmp_name"];
$folder     = "image/" . $filename;
$categories = get_safe_value($link, $_POST['categories']);
$res        = mysqli_query($link, "select * from categories where categories='$categories'");
$check      = mysqli_num_rows($res);
if ($check > 0) {
    if (isset($_GET['id']) && $_GET['id'] != '') {
        $getData = mysqli_fetch_assoc($res);
        if ($id == $getData['id']) {
            
        } else {
            $msg = "Categories already exist";
        }
    } else {
        $msg = "Categories already exist";
    }
    // Now let's move the uploaded image into the folder: image
    if (move_uploaded_file($tempname, $folder)) {
        $msg = "Image uploaded successfully";
    } else {
        $msg = "Failed to upload image";
    }
}

if ($msg == '') {
    if (isset($_GET['id']) && $_GET['id'] != '') {
        mysqli_query($link, "update categories set categories='$categories' where id='$id'");
    } else {
        mysqli_query($link, "insert into categories(categories,filename,status) values('$categories','$filename','1')");
    }
    header('location:categories.php');
    die();
}
  }
  ?>
ouflak
  • 2,458
  • 10
  • 44
  • 49
guna
  • 17
  • 2
  • 2
    1st check: do you have `enctype='multipart/form-data` in your form ? (or similar statement if you submit by ajax) – Ken Lee Oct 05 '21 at 00:46
  • yes i do have that. still remains same. is my code is right.? – guna Oct 05 '21 at 00:51
  • 2
    Please consider PDO prepared statements instead of putting id and category directly into the string. I don’t trust get_safe_value(). Use XDebug to single-step through your code. – Gogowitsch Oct 05 '21 at 00:56
  • check whether your image folder has write permission too – Ken Lee Oct 05 '21 at 00:57
  • Have you checked `$msg` for _"Failed to upload image"_? I'd also ensure that the `image` folder is _exactly_ where you think it is via `$folder = __DIR__ . "/image/$filename";` – Phil Oct 05 '21 at 01:00
  • @Gogowitsch or even [MySQLi prepared statements](https://www.php.net/manual/mysqli.quickstart.prepared-statements.php) – Phil Oct 05 '21 at 01:00
  • those are fine i think problem with this code – guna Oct 05 '21 at 01:03
  • @guna, it looks like your upload folder URL is not correct; use as what @Phil ```$folder = __DIR__ . "/image/$filename";``` mentioned, the second thing you can check, ```$filename``` does it has the file extension, otherwise concatenate the file extension with it. – Mainul Hasan Oct 05 '21 at 01:27
  • _"i dont see any syntax error though"_ - it is obviously not a _syntax_ error, because then your whole script would not run. You say the file name made it into the database, so that simply _can't_ have been the case. – CBroe Oct 05 '21 at 06:39
  • **Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Oct 05 '21 at 07:01

1 Answers1

0

Sorry, had to revise my post. The basename function is hardly an issue and your SQL code appears good enough considering the intent is uploading. Based on what you are stating and visually stepping through your code it seems like the action is firing inside the "if ($id == $getData['id']) block and in return invoking the if($msg == '') else block due to you seeing the filename in the database.

Hope I have not butchered your intent, but I'm trying to figure this thing out too.

Save your file and try this:

<?php
require('top.inc.php');
$categories = '';
$msg        = '';
if (isset($_GET['id']) && $_GET['id'] != '') {
$id    = get_safe_value($link, $_GET['id']);
$res   = mysqli_query($link, "select * from categories where id='$id'");
$check = mysqli_num_rows($res);
    if ($check > 0) {
        $row        = mysqli_fetch_assoc($res);
        $categories = $row['categories'];
    } else {
    header('location:categories.php');
    die();
    }
}

if (isset($_POST['submit'])) {
$filename   = $_FILES["uploadfile"]["name"];
$tempname   = $_FILES["uploadfile"]["tmp_name"];
$folder     = "image/" . $filename;
$categories = get_safe_value($link, $_POST['categories']);
$res        = mysqli_query($link, "select * from categories where categories='$categories'");
$check      = mysqli_num_rows($res);
    if ($check > 0) {
        if (isset($_GET['id']) && $_GET['id'] != '') {
        $getData = mysqli_fetch_assoc($res);
            if ($id == $getData['id']) {
                if ($msg == '') {
                mysqli_query($link, "update categories set categories='$categories' where id='$id'");
                header('location:categories.php');
                mysqli_close($link);
                die();
                } else {
                    // Now let's move the uploaded image into the folder: image
                    if (move_uploaded_file($tempname, $folder)) {
                    $msg = "Image uploaded successfully";
                    mysqli_query($link, "insert into categories(categories,filename,status) values('$categories','$filename','1')");
                    header('location:categories.php');
                    mysqli_close($link);
                    die();
                    } else {
                    $msg = "Failed to upload image @move_upload_file";
                    die($msg);
                    }
                }
            }else{
            $msg = "Categories already exist @getData ";
            die($msg);
            }          
        } else {
        $msg = "Categories already exist @getID";
        die($msg);
        }
    } 
}
?>
Navee
  • 11
  • 2
  • Yes, but it's not recommended to use the original file name, as the original file name might contain special characters or non-ASCII characters. It's better to generate a new random filename for the uploaded file. – Raptor Oct 05 '21 at 03:24
  • can you share me condensed version of my code – guna Oct 05 '21 at 03:28
  • Yes, that is definitely truth @Raptor considering real-world scenarios. What would you suggest, the use of str_replace(), uniqid(), microtime(), or some sort of routine to generate a timestamp new filename? – Navee Oct 05 '21 at 04:09
  • depends on scenario, but most of the time, time-based functions combined with random string is enough for most cases. Or if there is a need to remember the file is from which user, putting user ID as a prefix of the file is also a reasonable choice. – Raptor Oct 05 '21 at 04:31
  • Hm, @guna my answer may not possibly be the actual solution to your problem. How are you viewing your $msg variable? If the file has not moved "Failed to upload image" could be invoked or your code is not reaching the move_upload_file block. Try using die($msg) after all of your $msg statements to makeshift debug. Where if($msg == '') must be firing since you are getting the filename. if ($id == $getData['id']) is the only place $msg does not appear...and die("Her I am!"); there and run your code. If it appears move your move_upload_file block there. – Navee Oct 05 '21 at 04:46