-1

I am learning some PHP and am trying to create a form to update an image and set its name to "img".$_POST['something']."jpg", but the result is always img.jpg.

I know $_POST['something'] has a value because I am using it to run a query in the same page.

I spent the whole day (today) trying to figure this out, but I had ran out of ideas by now. Please help.

There might be something wrong with the way I am using msqli_fetch_array (I say that because of the warnings), I have tried to change it but am not sure how.

After a lot of echos through the code, it LOOKS TO ME that when I click submit, part of the page is reloaded with a new $_POST variable, and that is why my postedId vanishes at this point.. Does that make sense? How can I fix it?

<?php
  /* Displays user information and some useful messages */
  require 'db.php';
  session_start();

  // Check if user is logged in using the session variable
  if ($_SESSION['logged_in'] != 1) {
    $_SESSION['message'] = "You must log in before viewing your profile page!";
    header("location: error.php");  
  }

  $postedId = $_POST['stid'];

  $result = mysqli_query($mysqli, "SELECT stcontents.id, `st_id`, `name` FROM `students`, `stcontents` WHERE stcontents.tc_id = ".$_SESSION['tcid']." AND students.id = st_id AND st_id = ".$postedId." GROUP BY st_id");

  $row = mysqli_fetch_array($result);
?>

<!doctype html>
<html lang="en">
  <body>
    <div class="container" id="turma-container">
      <form action="" method="post" enctype="multipart/form-data">
        <p>Image:</p>
        <input type="file" name="fileToUpload" id="fileToUpload">
        <input type="submit" value="Upload Image" name="submit">
      </form>
      <?php
        $target_dir = "resources/images/";
        $target_file = $target_dir."img".$postedId.".jpg";
        $uploadOk = 1;
        $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION));
        // Check if image file is a actual image or fake image
        if(isset($_POST["submit"])) {
          $check = getimagesize($_FILES["fileToUpload"]["tmp_name"]);
          if($check !== false) {
            echo "File is an image - " . $check["mime"] . ".";
            $uploadOk = 1;
          } else {
            echo "File is not an image.";
            $uploadOk = 0;
          }
        }
        // Check file size
        if ($_FILES["fileToUpload"]["size"] > 500000) {
          echo "Sorry, your file is too large.";
          $uploadOk = 0;
        }
        // Allow certain file formats
        if($imageFileType != "jpg") {
          echo "Sorry, only JPG files are allowed.";
          $uploadOk = 0;
        }
        // Check if $uploadOk is set to 0 by an error
        if ($uploadOk == 0) {
          echo "Sorry, your file was not uploaded.";
        // if everything is ok, try to upload file
        } else {
          if (move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file)) {
          echo "The file ". basename( $_FILES["fileToUpload"]["name"]). " has been uploaded.";
          } else {
            echo "Sorry, there was an error uploading your file.";
          }
        }
      ?>
    </div>
  </body>
</html>

I was expecting the resulting file name to be something like img6.jpg, not img.jpg as it is. Here are the log I am getting

[19-Jul-2019 23:41:27 UTC] PHP Notice: Undefined index: fileToUpload in /home/[...]/myfolder/myfile.php on line 46 [19-Jul-2019 23:41:27 UTC] PHP Notice: Undefined index: fileToUpload in /home/[...]/myfolder/myfile.php on line 60 [19-Jul-2019 23:41:33 UTC] PHP Notice: Undefined index: stid in /home/[...]/myfolder/myfile.php on line 12 [19-Jul-2019 23:41:33 UTC] PHP Warning: mysqli_fetch_array() expects parameter 1 to be mysqli_result, bool given in /home/[...]/myfolder/myfile.php on line 16

Watercayman
  • 7,970
  • 10
  • 31
  • 49
  • 1
    There doesn't seem to be a `stid` field on your form, but you are trying to make an SQL query based on it. – atymic Jul 20 '19 at 00:01
  • Since `fileToUpload` is an undefined index, it seems that the file upload is not working correctly for some reason. – atymic Jul 20 '19 at 00:02
  • Can you post the output of `var_dump($_FILES)` please. – atymic Jul 20 '19 at 00:03

2 Answers2

0

First, I'd like to thank everyone who sent me tips and spent their time reading my code. I have made changes but I still can't make it work as I expect it to.

As I said, with the tips from ArendE, I have changed a lot of things in my code and I think it makes a lot more sense now (I think I can understand it much better too). One thing I though of changing was to make it more complete and not only giving the option to upload an image but also adding/changing a text and seeing the image if it has already been uploaded. Unfortunately, something still is not working as I would expect. I will try to explain as I post my code.

Every id variable is a int and the initial $_POST comes from a button in another page.

Top php, here I make the connection and declare some variables:

<?php
/* Displays user information and some useful messages */
session_start();
require 'db.php';

// Check if user is logged in using the session variable
if (empty($_SESSION['logged_in'] != 1) {
    $_SESSION['message'] = "You must log in before viewing your profile page!";
    header("location: error.php");
    exit();
}
else {
    // Makes it easier to read
    $name = $_SESSION['name'];
}

if (isset($_POST['stid'])) {
    $pstid = $_POST['stid'];
    $result = mysqli_query($link, "SELECT `id`, `tc_id`, `name`, `essay`, `image` FROM `students` WHERE tc_id = ".$tcid." AND id = ".$pstid.";");
    $row = mysqli_fetch_array($result);

    $stname = $row['name'];
    $cessay = $row['essay'];
    $cimage = $row['image'];
}
?>

Some html, here I am creating the text field (which already contais text if there are text in the database), show the image if it still exists and give a upload image option to change/add it. If the user changes the text or the image, it will click a save button.

<form method="post" enctype="multipart/form-data">
    <div class="form-group">
        <label for="TextToUpload">Text:</label>
        <textarea type="text" class="form-control" id="TextToUpload" rows="5" name="stessay"><?php echo $cessay; ?></textarea>
    </div>

    <p>Imagem do(a) aluno(a):</p>
    <?php
        if (isset($row['image'])) {
            echo "<img src=\"resources/images/studentsImages/".$cimage."\">";
        }

        echo "<input type=\"hidden\" name=\"stid\" value=".$pstid.">";
    ?>
    <input type="file" name="fileToUpload" id="fileToUpload">
    <input formmethod="post" type="submit" value="savechanges" name="submit">
</form>

And bottom php. Here I check if there is text and/or if there is a file selected to update. There are some checking as if the file is corrupt and them I try to send both the information.

<?php
    if(isset($_POST["submit"])) {
        $essayOk = 0;
        $imageOk = 0;

        if(isset($_POST['stessay'])) {
            $essayOk = 1;
            $pessayc = mysqli_real_escape_string($link, $_POST['stessay']);
        }

        if (isset($_POST['fileToUpload'])) {
            $target_dir = "resources/images/";
            $target_file = $target_dir."img".$pstid.".jpg";

            $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION));
            // Check if image file is a actual image or fake image
            if($mime = finfo_file($finfo, $target_dir . $_FILES["fileToUpload"]["tmp_name"])) {
                if($mime == 'image/jpg' || $mime == 'image/jpeg') {
                    if(getimagesize($target_dir . $_FILES["fileToUpload"]["tmp_name"]) === false) {
                        echo "File is corrupt";
                        $uploadOk = 0;
                    } else {
                        echo "File is an image - " . $mime . ".";
                        $uploadOk = 1;
                    }                           
                } else {
                    echo $mime . " is not supported.";
                    $uploadOk = 0;
                }
            } else {
                echo "Invalid file";
                $uploadOk = 0;
            }
            // Check file size
            if ($_FILES["fileToUpload"]["size"] > 500000) {
                echo "The file is too big.";
                $imageOk = 0;
            }
            // Check if $imageOk is set to 0 by an error
            if ($imageOk == 0) {
                echo "Erros sending image, try again later.";
            // if everything is ok, try to upload file
            }
        }

        if ($essayOk == 1) {
            $query = "UPDATE students SET essay = '".$pessayc."' WHERE id = '".$pstid."';";
            if (mysqli_query($link, $query)) {
                echo "<p>Essay updated!</p>";
            } else {
                printf("Errormessage: %s\n", mysqli_error($link));
                echo "<p>There was an error updating the ssay, try again later.</p>";
            }
        }

        if ($imageOk == 1) {
            if (move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file)) {
                echo "The file ".basename($_FILES["fileToUpload"]["name"])." was updated.";
            } else {
                echo "Your image was not updated, try again later.";
            }
        }
    }    
?>

I think that the problem is with the bottom php part of the code, for some reason (which I couldn't figure out), it looks like isset($_POST['fileToUpload']) are returning false every time, even when a file is selected. Shouldn't ir come with stessay, stid and submit? This time there is absolutely nothing on the error_log. A print_r of $_POST returns just Array ( [stessay] => Test [stid] => 1 [submit] => savechanges ) Why isn't fileToUpload in there too?

-1

As you've spend the whole day on this, let me give you a bunch of pointers. I hope you can learn something from it and understand why those "Undefined" and "Notices" are there. I've not tested the code, but if you go thru all the comments I hope you'll understand and will be able to make it getting to work.

<?php
    // Always start sessions first:
    // if your db.php throws an error, the session can't start anymore
    // and throws a warning
    session_start(); 

    require('db.php');

    // Do check if a variable exists. PHP should throw you a warning otherwise.

    if(empty($_SESSION['logged_in']) || $_SESSION['logged_in'] != 1) {
        $_SESSION['message'] = "You must log in before viewing your profile page!";
        header("Location: error.php");

        // Stop running the script after a redirect! 
        // A header is an instruction, a client 
        // might simple ignore it and show the page content anyway
        exit();
    }

    // Check if the variable exists!
    // Long way:
    // if(isset($_POST['stid'])) { $postedId = $_POST['stid'] } else { $postedId = false; }
    // Medium way:
    // $postedId = (isset($_POST['stid']) ? $_POST['stid'] : false;
    // Short way:
    $postedId = $_POST['stid']?:false;

    // Make your query look nice, makes your life easy and debugging too
    // Query questions: 
    // 1. What if $_SESSION['tcid'] doesn't exist?
    // 2. What if $postedId doesn't exist?
    // 3. What if $postedId is 0; DROP TABLE students; ?
    // Remember, a client can send anything via $_POST['stid']
    //
    //$qry = "SELECT stcontents.id, `st_id`, `name` 
    //      FROM `students`, `stcontents` 
    //      WHERE stcontents.tc_id = " . $_SESSION['tcid'] . " 
    //      AND students.id = st_id 
    //      AND st_id = " . $postedId . " GROUP BY st_id";
    //
    // Read about mysqli_real_escape_string
    // https://www.php.net/manual/en/mysqli.real-escape-string.php
    // Want to do it really right? Use prepared statements
    // https://www.php.net/manual/en/mysqli.prepare.php

    $qry = "SELECT stcontents.id, `st_id`, `name` 
            FROM `students`, `stcontents` 
            WHERE stcontents.tc_id = " . mysqli_real_escape_string($mysqli, $_SESSION['tcid']) . " 
            AND students.id = st_id 
            AND st_id = " . mysqli_real_escape_string($mysqli, $postedId) . " GROUP BY st_id";

    $result = mysqli_query($mysqli, $qry);
    $row = mysqli_fetch_array($result);

    // But what if no result was found?
    if(empty($postedId) || empty($row)) {
        exit('Something above went wrong!');
    }
?>
<!doctype html>
<html lang="en">
    <body>
        <div class="container" id="turma-container">
            <!-- 
            // Leave out the action if it's empty anyway
            // https://stackoverflow.com/questions/1131781/is-it-a-good-practice-to-use-an-empty-url-for-a-html-forms-action-attribute-a
            -->
            <form method="post" enctype="multipart/form-data">
                <p>Image:</p>
                <input type="file" name="fileToUpload" id="fileToUpload">
                <input type="submit" value="Upload Image" name="submit">
            </form>

            <?php
                $target_dir = "resources/images/"; // Better use the full path

                // $target_file = $target_dir."img".$postedId.".jpg";
                // What if $postedId is /../../logo ?
                // Is resources/images/img/../../logo.jpg a valid path?             
                //
                // I'll assume $postedId will be an integer (number)
                // using https://www.php.net/manual/en/function.settype.php

                settype($postedId, 'int');

                // Another approach: basename()
                // https://www.php.net/manual/en/function.basename.php
                // https://www.php.net/manual/en/features.file-upload.post-method.php

                $target_file = $target_dir . basename("img" . $postedId . ".jpg");
                $uploadOk = 1;

                // $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION));
                // You have just made a string $target_file.. so
                // nothing is there, or it would be jpg anyway, since you've said ".jpg"

                if(isset($_POST["submit"])) {

                    // So $_POST['submit'] might be there, but was the fileToUpload too?

                    if(empty($_FILES["fileToUpload"])) {
                        exit('no file!');
                    }

                    // $check = getimagesize($_FILES["fileToUpload"]["tmp_name"]);
                    //
                    // Well although getimagesize indeed does return false on failure,
                    // read the caution "Do not use to check that a given file is a valid image."
                    // here https://www.php.net/manual/en/function.getimagesize.php
                    // the path it also incomplete ($target_dir is missing)

                    $finfo = finfo_open(FILEINFO_MIME_TYPE);

                    if($mime = finfo_file($finfo, $_FILES["fileToUpload"]["tmp_name"])) {
                        if($mime == 'image/jpg' || $mime == 'image/jpeg') {

                            // Now you could use getimagesize as extra check
                            // But there might be better alternatives
                            if(getimagesize($_FILES["fileToUpload"]["tmp_name"]) === false) {
                                echo "File is corrupt";
                                $uploadOk = 0;
                            } else {                        
                                echo "File is an image - " . $mime . ".";
                                $uploadOk = 1;
                            }                           
                        } else {
                            echo $mime . " is not supported.";
                            $uploadOk = 0;
                        }                       
                    } else {
                        echo "Invalid file";
                        $uploadOk = 0;
                    }               

                    if($_FILES["fileToUpload"]["size"] > 500000) {
                        echo "Sorry, your file is too large.";
                        $uploadOk = 0;
                    }


                    // if($imageFileType != "jpg") {
                    //  echo "Sorry, only JPG files are allowed.";
                    //  $uploadOk = 0;
                    // }
                    //
                    // Done this above.
                    // If the idea is some pre-flight check, consider $_FILES['fileToUpload']['type'] 

                    if($uploadOk == 0) {
                        echo "Sorry, your file was not uploaded.";
                    } else {
                        if(move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file)) {
                            echo "The file ". basename($_FILES["fileToUpload"]["name"]). " has been uploaded.";
                        } else {
                            echo "Sorry, there was an error uploading your file.";
                        }
                    }
                }
            ?>
        </div>
    </body>
</html>

There can still be a lot of optimization, but this should get you going. Best of luck with it! If you have some questions, feel free to comment.


After your comments, let's do a new round! Here is the code again:

<?php
session_start();
require 'db.php';

// So empty($var) returns true/false based on if a variable exists (isset()) and it's value 
// Read: https://www.php.net/empty what is considered FALSE is it exists
// I'm guessing this will do:
if (empty($_SESSION['logged_in'])) {
    $_SESSION['message'] = "You must log in before viewing your profile page!";
    header("location: error.php");
    exit();
} else {

    // If you are sure $_SESSION['name'] exists if a user is logged in, this is fine.
    // Otherwise consider  $name = $_SESSION['name']?:'unknown';
    $name = $_SESSION['name'];
}

if (isset($_POST['stid'])) {

    // So you still allow a raw POST variable in your database query..
    // Don't do that or you might find someone messed around with your database.
    // https://www.w3schools.com/sql/sql_injection.asp
    // $pstid = $_POST['stid'];

    $pstid = mysqli_real_escape_string($link, $_POST['stid']);
    $tcid = mysqli_real_escape_string($link, $tcid); // Where does $tcid come from? Does it exist?

    // Check if the query is succesful and if there are results..
    // In the function documentation always peek at the Parameters and Return Values
    // https://php.net/manual/en/mysqli.query.php : Returns FALSE on failure.
    // https://www.php.net/manual/en/mysqli-result.fetch-array.php : Returns NULL if there are no more rows

    if($result = mysqli_query($link, "SELECT `id`, `tc_id`, `name`, `essay`, `image` FROM `students` WHERE tc_id = ".$tcid." AND id = ".$pstid.";")) {
        if($row = mysqli_fetch_array($result)) {
            $stname = $row['name'];
            $cessay = $row['essay'];
            $cimage = $row['image'];
        } else {
            $_SESSION['message'] = "No essay was found, please create one first.";
            header("location: error.php");
            exit();
        }
    } else {
        $_SESSION['message'] = "Something went wrong..";
        header("location: error.php");
        exit();
    }

    // What if $_POST['stid'] does not exist though?
    // You use it as hidden input for your form, so let's throw an error.
} else {
    $_SESSION['message'] = "Student id not found.";
    header("location: error.php");
    exit();
}
?>

<form method="post" enctype="multipart/form-data">
    <div class="form-group">
        <label for="TextToUpload">Text:</label>

<!--
So, $cessay comes from your database; but how does it get entered? By students?
Imagine a students enters "</textarea><img src="https://i.imgur.com/BBcy6Wc.jpg">"
Right.. a cat picture will be shown.. Solution: Escape it.
https://www.w3schools.com/php/func_string_htmlspecialchars.asp
So:
--> 
        <textarea type="text" class="form-control" id="TextToUpload" rows="5" name="stessay"><?php echo $cessay; ?></textarea>
    </div>

    <p>Imagem do(a) aluno(a):</p>
    <?php
        if (isset($row['image'])) {
            // Same applies here, although cimage might be under your control, so less critical
            // Just make it a habbit to escape and you'll never have trouble :)

            $cimage = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');
            echo "<img src=\"resources/images/studentsImages/".$cimage."\">";
        }

        // So I've used it before in mysqli_real_escape_string, so it now could be something like '123' instead of 123
        // Since we now need to escape it not for SQL but HTML, just use the original value again:

        echo "<input type=\"hidden\" name=\"stid\" value=".htmlspecialchars($_POST['stid'], ENT_QUOTES, 'UTF-8').">";
    ?>
    <input type="file" name="fileToUpload" id="fileToUpload">
    <input formmethod="post" type="submit" value="savechanges" name="submit">
</form>

<?php
    if(isset($_POST["submit"])) {
        $essayOk = 0;
        $imageOk = 0;

        if(isset($_POST['stessay'])) {
            $essayOk = 1;
            $pessayc = mysqli_real_escape_string($link, $_POST['stessay']); 
            // Very good! :)
        }

        // Uploaded files should be in the $_FILES array! So don't use $_POST
        if (isset($_FILES['fileToUpload'])) {
            $target_dir = "resources/images/";
            $target_file = $target_dir."img".$pstid.".jpg";

            // $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION)); // Not needed, fails anyway      
            // But where is $finfo ? The code below will always fail without it..
            // Adding it back:
            $finfo = finfo_open(FILEINFO_MIME_TYPE);        

            if($mime = finfo_file($finfo, $_FILES["fileToUpload"]["tmp_name"])) {
                if($mime == 'image/jpg' || $mime == 'image/jpeg') {
                    if(getimagesize($_FILES["fileToUpload"]["tmp_name"]) === false) {
                        echo "File is corrupt";
                        $uploadOk = 0;
                    } else {
                        echo "File is an image - " . $mime . ".";
                        $uploadOk = 1;
                    }                           
                } else {
                    echo $mime . " is not supported.";
                    $uploadOk = 0;
                }
            } else {
                echo "Invalid file";
                $uploadOk = 0;
            }

            if ($_FILES["fileToUpload"]["size"] > 500000) {
                echo "The file is too big.";
                $imageOk = 0;
            }

            if ($imageOk == 0) {
                echo "Erros sending image, try again later.";
            }
        }

        if ($essayOk == 1) {
            $query = "UPDATE students SET essay = '".$pessayc."' WHERE id = '".$pstid."';";
            if (mysqli_query($link, $query)) {
                echo "<p>Essay updated!</p>";
            } else {
                printf("Errormessage: %s\n", mysqli_error($link));
                echo "<p>There was an error updating the ssay, try again later.</p>";
            }
        }

        if ($imageOk == 1) {
            if (move_uploaded_file($_FILES["fileToUpload"]["tmp_name"], $target_file)) {
                echo "The file ".basename($_FILES["fileToUpload"]["name"])." was updated.";
            } else {
                echo "Your image was not updated, try again later.";
            }
        }
    }    
?>

As for your final question, check if it exists in the $_FILES array, but also I believe the filename should still be in the $_POST array. Not sure though. Add this on top of your code to see what variables are found:

print_r($_POST);
print_r($_FILES);

It will give you a nice list of POST fields and uploaded files.

ArendE
  • 957
  • 1
  • 8
  • 14
  • Thank you so much for your help ArendE! I have read many of the references you sent me and changed a lot of things on my file. I think my code make a lot more sense now. Although it still isn't working. I will post bellow my alterations, if you can, please take a look at it. – Daniel Ayres Jul 20 '19 at 21:06
  • I'm happy you've appreciated it @DanielAyres! I've taken a look at it and reviewed the code once more, see my edited answer for it. – ArendE Jul 21 '19 at 01:29
  • Thank you so much ArendE for all the help with my code! It is working great now and I understand php WAY better than before. Just so others can use the code, I have one small correction: in the function "getimagesize($target_dir . $_FILES["fileToUpload"]["tmp_name"])", we should not use $target_dir, since we are talking about the temporary directory, not the final one. – Daniel Ayres Jul 22 '19 at 13:13
  • You are very right, sorry for the oversight. :) I'm happy it's working and you understand now what's going on -- it's just like learning a language, right? ^^ Good luck with all the essays! – ArendE Jul 22 '19 at 14:34