Selecting and Displaying mysql data through hidden form

Question

I'm having trouble displaying my selected data through MYsql. I have values already stored in the fields in my database and when the user clicks the submit button and the review of the selected movie should populate but its not.

Here is my code for the form in php.

    echo "<tr><td>".$utitle."</td><td>".$ucategory."</td><td>".$ulength."</td><td>Thumbs up</td>t<td></td>";
    echo '<form action = "videos.php" method="POST">';
    echo "<td><input type='hidden' name='read' value=".$uid."><input type='submit' value='Read the Review' name='read'></td></tr></form>";

And here is my code when the button is clicked.

if(isset($_POST['read'])){
    $readReview = "SELECT Review FROM MyVideos WHERE id='$_POST[read]'";            
    $read = $con->query($readReview);
    if($read->num_rows>0){
        while($row=$read->fetch_assoc()){
            echo "Review:".$row['Review']."<br/>";
        }
    }
$read->close(); 
};

Thanks. Any help is greatly appreciated.

both your hidden and submit bear the same name attribute. I'd call that a conflict. Plus, it's discarding the first attribute, being the hidden one. — Funk Forty Niner, Jun 04 '15 at 18:26
Oh man, you are the best. I can't believe I missed that especially when my other forms were different values for the name attribute. You helped relieved my headache. — steve, Jun 04 '15 at 18:32
Hopefully, user isn't supplying a value such as **`1' OR '1'='1`**, or something more nefarious, like **`Robert'; DROP TABLE MyVideos; --`**. [**OWASP SQL Injection** https://www.owasp.org/index.php/SQL_Injection](https://www.owasp.org/index.php/SQL_Injection) — spencer7593, Jun 04 '15 at 18:33
You're welcome Steve. Now someone's overbound my comment with an answer *sigh*. Oh well, what can you do ;-) — Funk Forty Niner, Jun 04 '15 at 18:35
@steve I decided to put my comment to an answer, including a few more things in there, should you wish to accept it. The choice is yours. Although not obligatory, this will inform the community that a solution was found. Otherwise, the question will remain as open. — Funk Forty Niner, Jun 04 '15 at 18:41

score 4 · Answer 1 · edited May 23 '17 at 12:21

Putting my comment to an answer.

Both your hidden input and submit button bear the same name attribute. I'd call that a conflict. Plus, it's discarding the first attribute, being the hidden one.

Also, your present code is open to SQL injection. Use prepared statements, or PDO with prepared statements, they're much safer.

$id = mysqli_real_escape_string($con, $_POST['read']);
$readReview = "SELECT Review FROM MyVideos WHERE id='id'";

Sidenote: $_POST['read'] will need to be changed to the name attribute you'd of given to the hidden input.

Yet, prepared statements are the way to go here.

Remember to rename your hidden input to something else of your choice.

Add error reporting to the top of your file(s) which will help find errors.

<?php 
error_reporting(E_ALL);
ini_set('display_errors', 1);

// rest of your code

Sidenote: Error reporting should only be done in staging, and never production.

References:

Object oriented style

string mysqli::escape_string ( string $escapestr )
string mysqli::real_escape_string ( string $escapestr )

Procedural style

string mysqli_real_escape_string ( mysqli $link , string $escapestr )

To emphasize this point again... **prepared statements** with **bind placeholders** is the way to go. Using `mysqli_real_escape_string` addresses part of the problem, but some code patterns can still be vulnerable to SQL Injection. (+1 Fred -ii-) Your notes and remarks are spot on, as usual. (I think the sql text should be `... id='$id'` nes pas? — spencer7593, Jun 04 '15 at 19:20
@spencer7593 Indeed you're right and I see you have taken care of that in your answer, also being spot on, upvoted yours as well, *cheers* — Funk Forty Niner, Jun 04 '15 at 19:22

score 1 · Answer 2 · answered Jun 04 '15 at 19:16

In the code, this line:

$readReview = "SELECT Review FROM MyVideos WHERE id='$_POST[read]'";

the single quotes are missing around read. The single quotes appear as we expect them here:

if(isset($_POST['read'])){
                ^    ^

But beyond adding single quotes, the code is vulnerable to SQL Injection.

Use prepared statement with bind placeholder. Assuming that id column in MyVideos is character type:

  if(isset($_POST['read'])){
      $readReview = 'SELECT Review FROM MyVideos WHERE id=?';
      $sth=$con->prepare($readReview);
      $sth->bind_param('s',$_POST['read']);
      $sth->execute();
      $sth->bind_result($review);
      while($sth->fetch()){
          echo "Review:".htmlentities($review)."<br/>";
      }
      $sth->close();
  }

To make the pattern more clear, the code example above doesn't check the return from the prepare or execute. We'd really want to check if the return from prepare is FALSE, we don't want to call bind_param or execute on that.

If id column is integer type, then replace 's' with 'i' (as the first argument in the bind_param.

The use of htmlentities assumes that the contents of the Review column does not contain any HTML markup that should not be escaped/encoded.

score 0 · Answer 3 · answered Jun 04 '15 at 18:30

0

submit and hidden field is same (not sure check it by changing)

change query:-

$readReview = "SELECT Review FROM MyVideos WHERE id='". $_POST[read] . "'"; // If id is varchar type or

$readReview = "SELECT Review FROM MyVideos WHERE id=$_POST[read]";  // If numric type

answered Jun 04 '15 at 18:30

Ashwani

692
2
6
16

that shouldn't matter. MySQL will differentiate between both. The important thing about passing an integer is that the column type must match. – Funk Forty Niner Jun 04 '15 at 18:45

Selecting and Displaying mysql data through hidden form

3 Answers3