-1

So, I am trying to figure out how do this this and it boggling me. THIS WILL NOT BE USED ONLINE LIVE SO SQL INJECTION I DONT' CARE ABOUT. What am I doing wrong/right?

    <?php
        $db = mysql_connect("localhost", "root", "root");
        if (!$db) {
            die("Database connect failed: " . mysql_error());
        }

        $db_select = mysql_select_db("UNii", $db);
        if (!$db_select) {
            die("Database selection failed: " . mysql_error());
        }

    $comment = $_GET['comment'];
    $id = $_GET['id'];

       $sql = "UPDATE Dbsaved SET comment = '{$comment}' WHERE id = $id";

        $comment1 = mysql_query($sql);

           if (!$comment1) {
               die("did not save comment: " . mysql_error());
           }

    echo $sql;

The main problem is with the statement itself, the connection is fine. I am trying to read $comment, and then update that into a MYSQL table and then have it read back in a different file.

EDIT: Mark up for the form I'm taking $comment from.

<!DOCTYPE html>
<html lang="en">
<LINK href="stylesheet.css" rel="stylesheet" type="text/css">
<script src ="js/validateform.js"></script>
<head>
    <meta charset="UTF-8">

    <title>UniHelp Home</title>

    </head>

    <body>
        <div id="headeruni">
            <h1>Welcome <?php echo $_GET["name"]; ?> to UniHelp!</h1>
        </div>

    <div id ="infouni">
        <h3>Welcome to UniHelp. The social Network getting you connected to other people all over the University for any help you require!</h3>
    </div>

  <div id ="nameandemail">
        <form action="formsend.php" method="post">
            First name: <br> <input type="text" name="name"><br>
            Email:  <br> <input type="text" name="email"><br>
            Comment: <br> <input type="text" name="message"><br>
            <input type="submit" name="submit">
        </form>`enter code here`
       </div>
    <div id="grabphpdiv">

        <?php
        $db = mysql_connect("localhost", "root", "root");
        if (!$db) {
            die("Database connect failed: " . mysql_error());
        }

        $db_select = mysql_select_db("UNii", $db);
        if (!$db_select) {
            die("Database selection failed: " . mysql_error());
        }
        $result = mysql_query("SELECT * FROM Dbsaved", $db);
        if (!$result) {
            die ("Database query failed: " . mysql_error());
        }

    $comment = $_POST['$comment'];

        while ($row = mysql_fetch_array($result)) {
            echo "<div id='posts'>";;
            echo "<h2>";
            echo $row[1] . "";
            echo "</h2>";
            echo "<p>";
            //echo $timestamp = date('d-m-y G:i:s ');
            echo "<br>";
            echo "<br>";
            echo $row[2] . "";
            echo "</p>";
            echo "<p>";
            echo $row[3] . "";
            echo "</p>";
            echo '<a href=delete.php?id=' . $row[0]. '">Delete</a>';
            echo "<br>";
            echo "<br>";
            echo 'Comment: <br>
                           <input type=text name=comment><br>
                           <a href=addcomment.php?id=' . $row[0]. '&comment='. $row['$comment'].'>Comment</a>';
            echo "<p>";
            echo $row['comment'] . "";
            echo "</p>";
            echo "</div>";
            echo "<br>";
        }
        ?>
        </div>
</body>

<div id="footer">Copyright &copy James Taylor 2016</div>
</html>
James Taylor
  • 25
  • 1
  • 7
  • 1
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)***. Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Jul 07 '16 at 13:01
  • 2
    ***Please [stop using `mysql_*` functions](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php).*** [These extensions](http://php.net/manual/en/migration70.removed-exts-sapis.php) have been removed in PHP 7. Learn about [prepared](http://en.wikipedia.org/wiki/Prepared_statement) statements for [PDO](http://php.net/manual/en/pdo.prepared-statements.php) and [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php) and consider using PDO, [it's really pretty easy](http://jayblanchard.net/demystifying_php_pdo.html). – Jay Blanchard Jul 07 '16 at 13:01
  • 1
    I hate when people say *"I'm not that far along..."* or *"This site will not be public..."* or *"It's only for school, so security doesn't matter..."*. If teachers and professors are not talking about security from day one, they're doing it wrong. Challenge them. They're teaching sloppy and dangerous coding practices which students will have to unlearn later. I also hate it when folks say, *"I'll add security later..."* or *"Security isn't important now..."*. If you don't have time to do it right the first time, when will you find the time to add it later? – Jay Blanchard Jul 07 '16 at 13:01
  • 1
    @JayBlanchard Security is being taught, But for my purposes, it really isn't relevant. I just need it to work, and useful input would be grand thanks. – James Taylor Jul 07 '16 at 13:03
  • 2
    All ^^^ plus need space before where condition as `" WHERE id = '$id'` – Saty Jul 07 '16 at 13:04
  • 1
    It is *always* relevant. Outside of that have you checked your error logs? – Jay Blanchard Jul 07 '16 at 13:04
  • @JayBlanchard. Error logs are saying "did not save comment: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '1' at line 1" – James Taylor Jul 07 '16 at 13:05
  • Something that has helped me is that I hardcode the SQL statement first, just to know the SQL is working, then replace the dynamic values with PHP variables once you have the SQL structure down. – Katie Jul 07 '16 at 13:05
  • 1
    @saty solved this for you. You're missing a space. – Jay Blanchard Jul 07 '16 at 13:06
  • @Saty. That still did not work. Still giving the same above error. – James Taylor Jul 07 '16 at 13:07
  • 1
    Move the SQL statement into it's own variable $sql = "UPDATE ...", then examine that statement through echo or var_dump to check the sanity of the statement – Katie Jul 07 '16 at 13:09
  • It is in its own variable. called $comment1. and echoing it seems to bring up nothing. – James Taylor Jul 07 '16 at 13:10
  • No, the function call is in that variable. Your query should be this: `"UPDATE Dbsaved ". "SET comment = '$comment'". "WHERE id = $id"` remove the quotes around `$id`. – Jay Blanchard Jul 07 '16 at 13:12
  • Please add the markup for your form to your question. – Jay Blanchard Jul 07 '16 at 13:19
  • added.@JayBlanchard. – James Taylor Jul 07 '16 at 13:23
  • That is not the code for the entire form. We need to be able to see everything in the form. – Jay Blanchard Jul 07 '16 at 13:26
  • that is the form. Its being echoed in a while loop so that it prints all rows from the database. @JayBlanchard. – James Taylor Jul 07 '16 at 13:27
  • We need to see the `
    ` tag. You do want us to help you, right?
    – Jay Blanchard Jul 07 '16 at 13:29
  • done. @JayBlanchard. – James Taylor Jul 07 '16 at 13:31
  • Yes there is. @JayBlanchard. Trust me. – James Taylor Jul 07 '16 at 14:00
  • A `
    ` tag on its own defaults to the GET method. The query string is malformed. It also looks like you're duplicating id's in other HTML elements, which will not work.
    – Jay Blanchard Jul 07 '16 at 14:06
  • still the same issue @JayBlanchard. I'm getting this error: did not save comment: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '1' at line 1. – James Taylor Jul 07 '16 at 14:18
  • Change `echo ['$comment'];` to `echo $comment;` Also, view the source of the HTML and show us what the rendered markup for the ` – Jay Blanchard Jul 07 '16 at 14:19
  • changed that @JayBlanchard. Still Nothing. Here is the mark up:

    Comment
    – James Taylor Jul 07 '16 at 14:58
  • In the markup you're showing here the comment is not being parsed from the variable. `comment=$comment` should be `comment=this is a comment` – Jay Blanchard Jul 07 '16 at 15:02
  • I deleted my answer as it is no where near being the help you need to get this working. You need to refactor your code to make sure you can move from one statement to the next logically. – Jay Blanchard Jul 07 '16 at 18:24
  • Note the comment field in the form has the attribute `name="message"` which would change the identifier in either the `$_GET` or `$_POST` array. – Jay Blanchard Jul 07 '16 at 18:29
  • Thats for a different form. – James Taylor Jul 07 '16 at 18:30
  • Where is the form for *this* one? Because that is the comment you're trying to re-use. – Jay Blanchard Jul 07 '16 at 18:30
  • the form for the `comment` is in the while loop. That other form is for a separate `post` creation. – James Taylor Jul 07 '16 at 18:32
  • OK - and I am not being demeaning when I say this - you really need to refactor the code. It is confusing and messy and seems to be thwarting our repair efforts. You need to move logically from one statement to the next and it just isn't happening smoothly here. My apologies for not being able to help you more, but without the greater context of this code it would be nearly impossible for any of use to help. – Jay Blanchard Jul 07 '16 at 18:35

2 Answers2

0

I just ran this code:

$comment = "Hello World!";
$id = 1;
$sql = "UPDATE Dbsaved SET comment = '{$comment}' WHERE id = {$id}";
echo $sql;

and saw:

UPDATE Dbsaved SET comment = 'Hello World!' WHERE id = 1

which is a correct SQL statement, so if it is not working, you might want to play with SQL directly to get something working. Hope that helps!

Katie
  • 2,594
  • 3
  • 23
  • 31
  • Since the id is an integer you may not want to quote it. It would appear, based on comments, the OP is using the wrong request array, for one. Bad spacing for two. and some other items... – Jay Blanchard Jul 07 '16 at 13:16
  • Thanks! The problem is that the ID will always change. – James Taylor Jul 07 '16 at 13:19
  • @JamesTaylor, the hard-coding of the variables was just an example of how you can echo out the SQL statement to inspect it (and so I could run the code on my own system), and also to give you construction of the SQL statement well-formed, the rest is up to you – Katie Jul 07 '16 at 13:21
  • @Katie. It is now say that the query was empty? – James Taylor Jul 07 '16 at 13:42
  • If you echo out your sql statement before it is executed, do you see the statement (with your values filled in?) – Katie Jul 07 '16 at 13:45
  • @katie. No I don't. I echo out and it says Array. – James Taylor Jul 07 '16 at 13:51
  • It might be helpful if you work with SQL at the command line, working through the SQL commands needed to perform what will happen programmatically, then when running your code, inspect your SQL statements as they are built and ensure they match what you know works with your database structure at the command line. – Katie Jul 07 '16 at 13:52
  • Then you haven't copied the code correctly, The code above is tested and will give you a well-formed SQL statement with values you send into $command and $id, I think it is just too hard without being able to look over your shoulder as we do in an office :) – Katie Jul 07 '16 at 13:55
  • Okay @katie. It is send the data through with just a regular SQL statement. But it is not picking up the $ comment from the form. I am $_POSTing the comment, but it is still not taking it and saying this now : did not save comment: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '1' at line 1 – James Taylor Jul 07 '16 at 13:57
  • @JamesTaylor - I see, so it is really about how you got the $comment value in the first place (or what it might be filled with). Sounds like you might have a problem with the $_POST values coming in from the form...at least you are honing in on the problem! You can var_dump($_POST) to see what's what. – Katie Jul 07 '16 at 14:02
  • @Katie. Where would I use that? I am new to the whole PHP&MYSQL stuff. – James Taylor Jul 07 '16 at 14:06
  • AHH...so it sounds like you need to learn about PHP forms, it is a big subject, any PHP book (or online resource) will help you through learning about it. Remember, the server is not where the form is being filled out, once the form is submitted, the data is sent back to the server, where the PHP awaits it in the $_POST variable, but you have some reading ahead of you, have fun with it! – Katie Jul 07 '16 at 14:15
  • echoing the query gave me back '1'. I know enough about it, I just am struggling with this weird part of it. @Katie. – James Taylor Jul 07 '16 at 15:01
  • 1
    I think it is just too hard, without seeing all the code involved (the HTML that creates the form, the processing of the form data through $_POST, then finally the SQL statement to put the comment field back into the database. Sorry I can't be of more help, it looks like @JayBlanchard is giving it a go with another answer, all I can say is the above code is tested and will work with proper values for $comment and $id, that much I am sure. – Katie Jul 07 '16 at 15:12
0

SOLUTION:

$comment = $_GET['$comment'];
$id = $_GET['$id'];

        while ($row = mysql_fetch_array($result)) {
            echo "<div id='posts'>";;
            echo "<h2>";
            echo $row[1] . "";
            echo "</h2>";
            echo "<p>";
            //echo $timestamp = date('d-m-y G:i:s ');
            echo "<br>";
            echo "<br>";
            echo $row[2] . "";
            echo "</p>";
            echo "<p>";
            echo $row[3] . "";
            echo "</p>";
            echo '<a href=delete.php?id=' . $row[0]. '">Delete</a>';
            echo "<br>";
            echo "<br>";
            echo $row[4] . "";
            echo "<br>";
            echo 'Comment: <br>
                           <form action="addcomment.php?id=' . $row[0]. '" method="post">
                           <input type=text name=comment><br>
                           <input type=submit name="submit">
                           </form>';
            echo "<p>";
            echo $row['comment'] . "";
            echo "</p>";
            echo "</div>";
            echo "<br>";
        }
        ?>

and:

<?php
        $db = mysql_connect("localhost", "root", "root");
        if (!$db) {
            die("Database connect failed: " . mysql_error());
        }

        $db_select = mysql_select_db("UNii", $db);
        if (!$db_select) {
            die("Database selection failed: " . mysql_error());
        }

    $comment = $_POST['comment'];
    $id = $_GET['id'];

       $sql = "UPDATE Dbsaved SET comment = '$comment' WHERE id = $id ";

        $comment1 = mysql_query($sql);

    echo $sql;

           if (!$comment1) {
               die("did not save comment: " . mysql_error());
           }
    else {
    header("location: UniHelpindex.php");
    }

It was to do with mainly needing to get the id which was used in $row[0]' in the form created in the while loop. And actually using the correct syntax for the update Dbsaved... bit.

James Taylor
  • 25
  • 1
  • 7