3
\$\begingroup\$

I have made an attempt to make a chat script and its working as required but the problem is that it is slow on slower connections and sometimes even on fast connections. Sometimes while loading the site it gives "RESOURCE LIMIT EXCEEDED". I made a page speed insight and the result is here.

I would like to know how to improve upon the performance and what changes to make to make it better.

P.S: Its wide open to attacks.

INDEX.HTML

<!doctype html> <html> <head> <meta charset="utf-8"> <title>Transmit</title> <meta name="viewport" content="width=device-width, initial-scale=1.0"> <link href="https://fonts.googleapis.com/css?family=Ubuntu:400,700" rel="stylesheet"> <link href="reset.css" rel="stylesheet"> <link href="index.css" rel="stylesheet"> <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.4/jquery.min.js"></script> <script src="actions.js"></script> </head> <body> <div id="header">BITE HIM <input type="text" id="key"></div> <div id="msg_display_container"> </div> <div id="decrypted_msg_display_container"> </div> <div id="message_bar"> <input type="text" id="message_input" placeholder="Post your data.."> <div class="panel_buttons" id="ext" title="Emergency">X</div> <div type="button" class="panel_buttons" id="snd" title="Send">&#36;</div> </div> </body> </html>

INDEX.CSS

html, body { background-color:#F4D0B0; } #header { font-family: 'Ubuntu', sans-serif; font-size:24px; text-align:center; background-color:#CC2211; color:#F7F2F2; width:100%; padding-top:25px; padding-bottom:25px; } #message_bar { position:fixed; bottom:0px; width:100%; background-color:#37457E; padding-top:15px; padding-bottom:15px; z-index:1000; } #key { width:10px; background-color:transparent; border:none; color:#CC2211; } #key:focus { color:#FFFFFF; } #decrypted_msg_display_container { display:none; } @media only screen and (max-width: 720px) { #message_input { width:70%; height:25px; padding:3px; margin-left:10px; color:#535353; float:left; display:inline; } .panel_buttons { height:27px; width:7%; padding:3px; float:right; text-align:center; vertical-align:middle; display:inline; background-color:#FFFFFF; font-family: 'Ubuntu', sans-serif; font-size:24px; cursor:pointer; } #ext { margin-right:10px; background-color:#CC2211; color:#FFFFFF; } #snd { margin-right:2%; } .enabled { background-color:#266802; color:#FFFFFF; } #msg_display_container, #decrypted_msg_display_container { width:100%; } #msg_display_container ul, #decrypted_msg_display_container ul { margin-left:10%; margin-right:10%; margin-top:25px; } #decrypted_msg_display_container { display:none; } .thread { padding:15px; background-color:#9399AE; color:#252323; margin-bottom:3px; border-radius:5px; font-family: 'Ubuntu', sans-serif; word-wrap: break-word; } }

ACTIONS.JS

$(document).ready(function(){ //ENABLING AND DISABLING SEND BUTTON TO PREVENT BLANK INPUT document.getElementById('snd').style.pointerEvents = 'none'; $('#message_input').on('keyup', function(e){ if($(this).val().length>0){ $('#snd').addClass('enabled'); document.getElementById('snd').style.pointerEvents = ''; } else{ $('#snd').removeClass('enabled'); document.getElementById('snd').style.pointerEvents = 'none'; } }); //EMERGENCY EXIT $('#ext').click(function(){ //window.open('', '_self', ''); //window.close(); //window.open('https://www.google.com', '_blank'); window.location.replace('http://www.google.com'); }); //SENDING THE MESSAGE $('#snd').click(function(){ var thread = $('#message_input').val(); //alert('clicked'); $.ajax({ type:'POST', url: 'send.php', data: {msg: thread}, success: function(response){ //alert(response); if(response!=1){ $('#message_input').val(''); $('#snd').removeClass('enabled'); document.getElementById('snd').style.pointerEvents = 'none'; } } }); }); //FETCHING THE MESSAGE setInterval(function(){ $.ajax({ type: "GET", url: "fetch_e.php", dataType:"json", cache: false, success: function(result){ $('#msg_display_container').html(''); $('#msg_display_container').append($('<ul>').append(result.threadList)); } }); }, 1000); //DECRYPTING THE MESSAGE $('#key').keyup(function (e) { if (e.which == 13) { var key = $('#key').val(); if(key!=""){ //alert('Enter received'); $('#msg_display_container').hide(); $.ajax({ type: "POST", url: "fetch_d.php", data: {key: key}, dataType:"json", cache: false, success: function(result){ console.log(result); $('#decrypted_msg_display_container').html(''); $('#decrypted_msg_display_container').show(); $('#decrypted_msg_display_container').append($('<ul>').append(result.threadList)); if(result.kmatch == 1){ $('#key').val(''); init_destructionProcess(); } } }); } return false; } }); function init_destructionProcess(){ var counter = 5; setInterval(function() { counter--; if (counter >= 0) { console.log(counter); } if (counter === 0) { //alert('Ka BOOOM'); destroy(); clearInterval(counter); } }, 1000); } function destroy(){ $.ajax({ url: "destroy.php", cache: false, success: function(response){ $('#decrypted_msg_display_container').hide(); $('#msg_display_container').delay(700).show(); } }); } });

SEND.PHP

<?php include 'connect.php'; include 'crypt.php'; if($_POST['msg']){ $thread = $_POST['msg']; $thread = mysqli_real_escape_string($db, $thread); $crypted_thread = encrypt($thread, 10); $sql = "INSERT INTO messages (thread) VALUES ('$crypted_thread')"; $results = mysqli_query($db, $sql); if(!$results){ echo 1; } mysqli_close($db); } ?>

CRYPT.PHP

<?php // $message = 'abc ABC 123'; // $crypted = encrypt($message, 5); // echo $crypted; function encrypt($plain_text, $shift){ //the text to be encrypted //letters of alphabet array and numbers $alphabet=array('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '1','2','3','4','5','6','7','8','9','0',' ', '.', ',', ':', ')', '('); //positions of the letters in alphabet $flip=array_flip($alphabet); $mod = count( $alphabet ); //plaintext array $plain_text=str_split($plain_text); $n=count($plain_text); $encrypted_text=''; for ($i=0; $i<$n; $i++){ //$n = strtoupper( $n ); //encryption $encrypted_text.=$alphabet[($flip[$plain_text[$i]]+$shift) %$mod]; } //echo $encrypted_text."<br>"; return $encrypted_text; } function decrypt($encrypted_text, $shift){ //the text to be decrypted //$encrypted_text='BJSJJIHTIJ'; //letters of alphabet array and numbers $alphabet=array('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '1','2','3','4','5','6','7','8','9','0',' ', '.', ',', ':', ')', '('); //positions of the letters in alphabet $flip=array_flip($alphabet); $mod = count( $alphabet ); //plaintext array $encrypted_text=str_split($encrypted_text); $n=count($encrypted_text); $decrypted_text=''; for ($i=0; $i<$n; $i++){ //$n = strtoupper( $n ); //decryption $decrypted_text.=$alphabet[($mod+$flip[$encrypted_text[$i]]-$shift) %$mod]; } return $decrypted_text; } ?>

FETCH_E.PHP

<?php include 'connect.php'; $threads = ''; $count = 0; $resultData[] = array(); $sql = "SELECT * FROM messages ORDER BY mid DESC"; $result = mysqli_query($db, $sql); if($result){ while($row = mysqli_fetch_array($result)) { //echo $row['thread']."<br>"; $message = $row['thread']; $threads.='<li class="thread">'.$message.'</li>'; } } echo json_encode(array('threadList' => $threads)); mysqli_close($db); ?>

FETCH_D.PHP

<?php include 'connect.php'; include 'crypt.php'; if($_POST['key']){ $key_match = 0; //No match $key = $_POST['key']; if($key == 10){ $key_match = 1; //If shift key matches } $threads = ''; $count = 0; $resultData[] = array(); $sql = "SELECT * FROM messages ORDER BY mid DESC"; $result = mysqli_query($db, $sql); if($result){ while($row = mysqli_fetch_array($result)) { $id = $row['mid']; if($key_match == 1){ $readQuery = "UPDATE `messages` SET `read` = '1' WHERE `mid` = '$id'"; $readResult = mysqli_query($db, $readQuery); } //echo $row['thread']."<br>"; $message = $row['thread']; $decrypted = decrypt($message, $key); $threads.='<li class="thread">'.$decrypted.'</li>'; } } echo json_encode(array('threadList' => $threads, 'kmatch' => $key_match)); } mysqli_close($db); ?>

DESTROY.PHP

<?php include 'connect.php'; $sql = "DELETE FROM `messages` WHERE `read`=1"; $result = mysqli_query($db, $sql); mysqli_close($db); ?>
\$\endgroup\$
2

1 Answer 1

Reset to default
2
\$\begingroup\$

SQL Injection

You are not currently vulnerable, but your approach to SQL injections is not very good (you escape values that you think are dangerous, and keep values that you think may be ok as they are).

First of all, it is highly recommended to use prepared statements instead of escaping. It's easy to use, it's safer, and it results in better code.

You should also treat all variables as dangerous. It's just a lot easier than having to think about the dangers of a variable each time (which will go wrong at some point). Checking the code for issues also becomes a lot easier.

Apart from that, if you do escape, the call to escape must be the very last change to the data before inserting it into the query. In your case, you call encrypt after mysqli_real_escape_string. That means if you add ' to your alphabet, it may be possible - although, depending on your encrypt function, unlikely - to perform SQL injections (with a Caesar cipher, it would be easy).

You may also be vulnerable to second order SQL injection, in case mid is in any way user-controlled.

Performance

This doesn't seem to be a problem between the server and the client, but a problem that exists purely on the server-side, so things like page speed will not help you.

You need to profile your application instead.

From a quick look, it seems that you fetch all messages every second, for each client. It's no wonder that you have performance problems.

One possible solution may be to set an already fetched flag and only fetch new messages.

It also seems that every time you decrypt a message, you initiate the destruction process, which is then run every second forever, resulting in a lot of calls to your script.

A quick fix would be to call clearInterval on the right variable, which would be the return value of setInterval. But I'm not sure why you even need an interval here.

Naming

Some of your names aren't all that good, hurting readability. Eg:

  • what's the difference between fetch_e and fetch_d? Newer shorten names, always write them out.
  • $key_match = 0; //No match: The comment doesn't really help. What I would want to know is what is matched. Key? What key? Ideally this is explained via a better variable name, if that's not possible via a more in-depth comment.

Formatting

You have some formatting issues, such as missing spaces between =, <, etc, as well as inconsistencies with the placement of {. You can use any IDE to fix this.

\$\endgroup\$
14
  • \$\begingroup\$1) I didnt understand about profiling the application. 2) I need an Interval in the destruction process so that as soon as the client successfully decrypts the message, he is allowed to view the message for only 5 seconds and then it gets destroyed.\$\endgroup\$
    – Ayan
    CommentedJul 6, 2016 at 12:52
  • \$\begingroup\$fetch_d mean fetching the decrypted results while fetch_e means fetching the encrypted results. $key means the shift key that the user inputs while $key_match is just a check variable to check whether the inputted key or the shift key is 10 which I use to update the read field in the table later on.\$\endgroup\$
    – Ayan
    CommentedJul 6, 2016 at 12:57
  • \$\begingroup\$@Ayan That makes sense. I would still assume that the counter runs on forever, but I misread the code, there is only one connection to the server (it also seems that you do have a working already fetched field which I misread). You can profile your client-side code with add-ons such as firebug, and server-side you may try two things: 1) record all requests. Are there more requests than expected? 2) Time how long each sub-task - retrieving messages, deleting messages, etc takes. Then, try to fix the tasks that take too long.\$\endgroup\$
    – tim
    CommentedJul 6, 2016 at 12:59
  • \$\begingroup\$Am not sure of how to do that already fetched flag part, a guideline might help me to set sail.\$\endgroup\$
    – Ayan
    CommentedJul 6, 2016 at 12:59
  • \$\begingroup\$@Ayan I meant something like your read field. I was just confused, as you fetch all messages, not just the once that are already read. It would be better to only fetch those and append them to the existing messages, and then have a separate field for destroying the messages. But your destroy code should also work for both tasks.\$\endgroup\$
    – tim
    CommentedJul 6, 2016 at 13:03

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.