0

My player message system isn't working. You start on the Diplomacy page which lists the player nations. Once a player nation is selected they are sent to the send message page which shows any messages between themselves and the nation selected. It also gives them a message box to write their own message to send. Here is the code.

if(isset($_POST['message']) && !empty($_POST['message'])){
    $sender = $_GET['nation'];
    $receiver = $_GET['receiver'];
    $random_number = rand();
    $message = $_POST['message'];
    $type = $_GET['type'];

    $check_con = mysql_query("SELECT `hash` FROM `message_group` WHERE (`user_one`='$sender' AND `user_two`='$receiver') OR (`user_one`='$receiver' AND `user_two`='$sender')");

    if(mysql_num_rows($check_con) ===1){
        $get_hash = mysql_fetch_assoc($check_con);

        $hash = $get_hash['hash'];
        mysql_query("INSERT INTO messages (group_hash, from_id, message, seen) VALUES('$hash','$sender','$message','0')");
        echo "<p>Message Sent!</p>";
   }else{   
        mysql_query("INSERT INTO message_group (user_one, user_two, hash) VALUES('$sender','$receiver','$random_number')");
        mysql_query("INSERT INTO messages (group_hash, from_id, message, seen) VALUES('$random_number','$sender','$message','0')");
        echo "<p>Conversation Started!</p>"; 
    }
    }
    ?>
    <form method="POST" action="index.php?page=gc3025/send_beta.php&game=<?php echo $game; ?>&type=<?php echo $type; ?>&nation=<?php echo $nations_id; ?>&user=<?php echo $user_id; ?>&receiver=<?php echo $receiver_id; ?>">
    <table>
    Enter Message:
    <tr>
   <td></td>
   <td><textarea name='message' rows='7' cols='60'></textarea></td>
   </tr>
    <td><input type='submit' value="Send Message" /></td>
    </table>
    </form>

If under FORM ACTION I link the page to itself it works but you have to refresh the page to see the new message which also resends the message. If the FORM ACTION goes to the previous page then it does not INSERT the message into the table on the server.

Jason B
  • 33
  • 4
  • For one thing, having both `isset` and `!empty` is redundant –  Jan 09 '16 at 22:45
  • @Terminus, wrong. Its not redundant. Try to just use `!empty($_POST['message']` without `isset` and you'll catch the error in case of `'message'` key doesn't exists in array – RomanPerekhrest Jan 09 '16 at 22:47
  • That's not what [the docs](http://php.net/empty) say. `No warning is generated if the variable does not exist. That means empty() is essentially the concise equivalent to !isset($var) || $var == false.` –  Jan 09 '16 at 22:51
  • @Jason B, you are mixing `get` and `post` params. If you just link the page via address bar it will not send `POST` request – RomanPerekhrest Jan 09 '16 at 22:53
  • @Terminus, your cite is reasonable for "simple" variables - not for array elements. I can repeat: try and you'll catch the error – RomanPerekhrest Jan 09 '16 at 22:55
  • @jasonB if you link the form to another page and that other page does not have code to insert `messages` than it wont do that... does the "previous page" have code to insert messages? –  Jan 09 '16 at 23:01
  • Please read up about SQL injection: http://php.net/manual/en/security.database.sql-injection.php – cEz Jan 10 '16 at 00:14
  • @RomanPerekhrest [There's no error reported.](http://ideone.com/XeXNGt) [Also, mixing $_POST and $_GET works fine](http://ideone.com/pinUKz). (whether you should...) –  Jan 10 '16 at 01:48
  • 1
    @Terminus, go further ... `empty()` is someway tricky, with `empty` no warning(notice) is generated if the variable does not exist. Doing so: `if(!empty($_POST['message']))` you don't even know whether `message` offset was set in `$_POST`. http://stackoverflow.com/questions/5657925/php-empty-error-message, http://stackoverflow.com/questions/4261133/php-notice-undefined-variable-and-notice-undefined-index. Check yourself – RomanPerekhrest Jan 10 '16 at 06:44
  • @RomanPerekhrest I know`empty` provides no warnings/notices (see the docs). What's your point about `message` offset was set in `$_POST`? `isset` doesn't tell you that either; you need `array_key_exists` to do that. i don't think we disagree, i think we're both looking at the OP's post and seeing 2 different problems. –  Jan 10 '16 at 06:56
  • 1
    @Terminus, BTW, I didn't say that mixing `$_POST` an `$_GET`works bad. When the OP wrote "I link the page to itself" it were sounding for me as something like "when I'm accessing the page link via address bar or anchor"... maybe I just didn't quite see what did the OP actually mean – RomanPerekhrest Jan 10 '16 at 07:12
  • @RomanPerekhrest re-reading your comment, you didn't but i did infer that. my mistake:( Good discussion:) Happy programming out there! –  Jan 10 '16 at 07:21
  • @Terminus, thanks! Have a good day! – RomanPerekhrest Jan 10 '16 at 07:26

1 Answers1

1

You can use AJAX to load data dynamically into page without reloading. I'm going to post an example using jQuery.


Your current page with the form, lets name it form.php. Removed the posting message logic, kept the form, added ajax request.

<div id="messages"></div>    

<form id="message_form" method="POST" action="#">
  <table>
  <p>Enter Message:</p>
  <tr>
    <td></td>
    <td><textarea name='message' rows='7' cols='60'></textarea></td>
  </tr>
  <tr>
    <td colspan="2"><input type='submit' value="Send Message" /></td>
  </tr>
  </table>
</form>

<script type="text/javascript">
$("#message_form").submit(function(){
  var url = "./send.php";
  var send_data = { game: "<?php echo $game; ?>", type: "<?php echo $type; ?>", nation: "<?php echo $nations_id; ?>", user: "<?php echo $user_id; ?>", receiver: "<?php echo $receiver_id; ?>" };
  $.post(url, send_data).done(function(received_data){
    reload_messages();
  });
});

function reload_messages(){
   var url = "./read_messages.php";
   var send_data = { ... }; // add data you want to send to the reading script, such as filtering or limit numbers
   $.post(url, send_data).done(function(received_data){
    $("#messages").html(received_data);
  });
}
<script>

Now send.php. Here's your original posting message logic, repaired SQL injection vulnerability (read notes in the bottom of the answer).

if(isset($_POST['message']) && !empty($_POST['message'])){
  $sender = mysql_real_escape_string($_GET['nation']);
  $receiver = mysql_real_escape_string($_GET['receiver']);
  $random_number = rand();
  $message = mysql_real_escape_string($_POST['message']);
  $type = mysql_real_escape_string($_GET['type']);

  $check_con = mysql_query("SELECT `hash` FROM `message_group` WHERE (`user_one`='".$sender."' AND `user_two`='".$receiver."') OR (`user_one`='."$receiver."' AND `user_two`='."$sender."')");

  if(mysql_num_rows($check_con) ===1){
    $get_hash = mysql_fetch_assoc($check_con);

    $hash = $get_hash['hash'];
    mysql_query("INSERT INTO messages (group_hash, from_id, message, seen) VALUES('$hash','$sender','$message','0')");
    echo "<p>Message Sent!</p>";
  }else{   
    mysql_query("INSERT INTO message_group (user_one, user_two, hash) VALUES('".$sender."','".$receiver."','".$random_number."')");
    mysql_query("INSERT INTO messages (group_hash, from_id, message, seen) VALUES('".$random_number."','".$sender."','".$message."','0')");
    echo "<p>Conversation Started!</p>"; 
  }
}

Your code doesn't show reading messages logic, so I'm just going to put simple select query here.

$query = mysql_query("SELECT ... ");
while($row = mysql_fetch_assoc($query)){
  echo $row[...];
}

By the way, just few notes to your code:

1.Original code is vulnerable to SQL injection. Use mysql_real_escape_string() to escape input values to the queries or better prepared statements.

2.If you don't have same number of <td> columns in <tr> rows, use colspan attribute.

3.The "Enter message" (or any other text) should be at least in <p>, within a table cell, or outside table.

4.The last <td> wasn't wrapped in any <tr>

cEz
  • 4,932
  • 1
  • 25
  • 38
Petr Hejda
  • 40,554
  • 8
  • 72
  • 100