HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PHP & AJAX CAESAR CIPHER ENCRYPTED chat script - Slow in performance

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptajaxchatphpcaesarencryptedslowperformancecipher

Problem

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


Transmit

BITE HIM 

    
    X
    $


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
{

Solution

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.

Context

StackExchange Code Review Q#134043, answer score: 2

Revisions (0)

No revisions yet.