3
\$\begingroup\$

I'm working on prepared statements for my website and I'm wondering if it's possible to have multiple prepared statements one after another. In this example, I have 2 select statements. I'm hoping that the question marks I have as placeholders won't be overwriting one another. After that's been answered, I was wondering if this code was still open to SQL injection.

function getHomeUploads($conn) {
        $userName = $_GET["user"];
        $sqluserid = "SELECT * FROM users WHERE userName = ?;";
        $useridstmt = mysqli_stmt_init($conn);
        if (!mysqli_stmt_prepare($useridstmt, $sqluserid)) {
            echo "SQL statement failed";
        }
        else {
            mysqli_stmt_bind_param($useridstmt, "s", $userName);
            mysqli_stmt_execute($useridstmt);

            $useridresult = mysqli_stmt_get_result($useridstmt);
            while ($useridrow = mysqli_fetch_assoc($useridresult)) {

                if($useridrow['userid'] > 0) {
                    $userid = $useridrow['userid'];
                    $sqlusercontent = "SELECT * FROM posts WHERE hostid = ? AND userid = ? AND commentid = 0;";
                    $stmtusercontent = mysqli_stmt_init($conn);
                    if (!mysqli_stmt_prepare($stmtusercontent, $sqlusercontent)) {
                        echo "SQL statement failed";
                    }
                    else {
                        mysqli_stmt_bind_param($stmtusercontent, "ss", $userid, $userid);
                        mysqli_stmt_execute($stmtusercontent);
                        $usercontent = mysqli_stmt_get_result($stmtusercontent);
                        $postid = 1;

                        echo "<div class='homepostcontainer'>";
                        echo "<div class='homepostcontainerscroll'>";
                        while ($row = mysqli_fetch_assoc($usercontent)) {
                            // Text fileid
                            if ($row['fileid'] == 1) {
                                echo "<div class='postbox'>";
                                echo "<div class='postscroll'>";
                                echo $row['title']."<br>";
                                echo $row['date']."<br>";
                                echo $row['description']."<br>";
                                echo "</div>";
                                echo "<div class='InfoBar'> <a href=view.php?user=".$userName."&post=".$postid.">•••</a> </div>";
                                echo "</div>";
                            }
                            // Picture fileid
                            else if ($row['fileid'] == 2) {
                                $filename = "posts/".$userid."/".$postid."*";
                                $fileinfo = glob($filename);
                                $fileext = explode(".", $fileinfo[0]);
                                $fileactualext = $fileext[1];

                                echo "<div class='postbox'>";
                                echo "<div class='postscroll'>";
                                echo $row['title']."<br>";
                                echo $row['date']."<br>";
                                echo "<img src='posts/".$userid."/".$postid.".".$fileactualext."'><br>";
                                echo $row['description']."<br>";
                                echo "</div>";
                                echo "<div class='InfoBar'> <a href=view.php?user=".$userName."&post=".$postid.">•••</a> </div>";
                                echo "</div>";
                            }
                            else {
                                echo '';
                            }

                            $postid++;
                        }
                        echo "</div>";
                        echo "</div>";  
                    }   
                }
            }
        }
    }
```  
\$\endgroup\$
  • \$\begingroup\$ You have a basic question about the correctness of your code. Such questions have no place of Code Review. Just remove it from your question, after you've made sure your code is working. There's a lot to improve in your code, so Code Review would be the right place to learn how to make your code better. \$\endgroup\$ – KIKO Software Jul 4 at 21:15
  • \$\begingroup\$ Sorry for that, I should clarify that the webpage loads correctly. I just want to know if there's anything wrong with 2 prepared statements within the same function. \$\endgroup\$ – Cole Jul 4 at 21:36
  • \$\begingroup\$ You can have two prepared statements, or three. No problem. Perhaps you could say something about the purpose of the function? \$\endgroup\$ – KIKO Software Jul 4 at 22:42
  • \$\begingroup\$ Thank you KIKO, if you answer the question ill upvote it. The purpose of the function is to pull from two different tables. The first table has info i need to open the second one. So I'm pulling the userid from users table and then im using the same userid to see what kinds of posts theyve made in a posts table. \$\endgroup\$ – Cole Jul 5 at 1:42
  • \$\begingroup\$ What is a "home upload", and why did you choose that name? \$\endgroup\$ – Roland Illig Jul 5 at 5:01
0
\$\begingroup\$
  1. Passing the database connection as a variable is a good technique.

  2. Validate user input as much as possible before considering executing any processes. If the input is missing or invalid, don't do anything with the submission.

    if (empty($_GET["user"])) { // add your validation here if possible
        echo "Missing/Invalid data submitted";
        // halt the script or redirect or whatever
    }
    
  3. Use object-oriented mysqli syntax because it is more brief and IMO cleaner to read.

  4. Do not perform iterated queries. You can easily condense you iterated queries into one simple query using an INNER JOIN. Something like this should do (untested):

    SELECT *
    FROM users u
    INNER JOIN posts p USING userid 
    WHERE u.user_name = ?
      AND p.hostid = p.userid
      AND p.commentid = 0
    

    USING is a convenient shorthand keyword when the two joining columns are identical; otherwise use ON and declare the columns individually. MySQL ON vs USING? You should also replace * in the SELECT clause with the exact columns you need from either table. (Never ask for more than you need.) With the above query, you will only need to bind $_GET"user"] once since there is now only one placeholder.

  5. I don't typically endorse printing from the same function that does the processing. If you are going to print nested html elements, you can improve readability by tabbing them.

  6. There is a cleaner way to extract the file extension than picking up post-explosion debris. How do I get (extract) a file extension in PHP?

    $ext = pathinfo($filename, PATHINFO_EXTENSION);
    
  7. If $row['fileid'] isn't 1 or 2, don't bother printing an empty string.

\$\endgroup\$
  • 1
    \$\begingroup\$ Wow, thank you for all of this info. I'll be using this answer as a reference for a while! \$\endgroup\$ – Cole Jul 5 at 8:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.