0

My site has this about-page with a list of links to click with diffrent classes. These links obviously takes you to diffrent sub-content. Why do these links have diffrent classes? Because i use javascript and jquery, to grab some content out from a database and smack it in a div on the without reloading... The database has 3 fields: id, headline and content.

The javascript works fine. It does what it should do. it takes the links' class (which is the an ID in the database) and uses it to grab the right content..

Basically:

<?php
//take the post'ed variable you've been given.
if(isset($_POST['id']));

//for convenience use this variable insted
$id =  $_POST['id'];

//connect to the database
mysql_connect("localhost","root","") or die('Could not connect: ' . mysql_error());

//select database
mysql_select_db("ropox");
mysql_query("SET NAMES utf8");

//Grab this data    
$query = mysql_query("SELECT * FROM about WHERE id=$id");
while($row = mysql_fetch_array($query))
{

//echo it bask to the original page. This is printed on the original page
echo $row['content'];
}
?>

When you click the link, it takes 1,03 second before the content appears on the page. At first it was 2 seconds, but I've managed to cut it down. all of my other database connects happens almost instantly and they even echo lots of content through loops. Why is this so slow?

tshepang
  • 12,111
  • 21
  • 91
  • 136
TehHO
  • 43
  • 4
  • 4
    **warning** your code may be vulnerable to sql injection attacks! – Daniel A. White Aug 24 '12 at 20:13
  • 1
    is `id` an indexed column? How large is the `about` table? Also your code is vulnerable to SQL Injection – Cfreak Aug 24 '12 at 20:13
  • There's nothing in the code that seems slow, but maybe you can take a look at the `AJAX` request to see what is taking the longest. You can take a look at requests under the 'Net' panel. – Grampa Aug 24 '12 at 20:15
  • Add profiling statements (endTime - startTime) around each statement to determine what's slowest. – EthanB Aug 24 '12 at 20:18
  • It could be the `mysql_connect`, it could be the `mysql_query`, it could be your web-server, it could be ..... – EthanB Aug 24 '12 at 20:19
  • How can I be vulnerable to SQL injection? There is no way for the user to input a custom value. I know they could change the ID but what fun would that be? it would then load nothing... @Grampa Yeah, that's what i did I tried to do diffrent stuff on the onclick function like alert. which happens instantly, it's only when I conenct to the DB it seems to take ages. Can't get my head around it. On the 'network' panel in chrome the db connect takes 1,02 second. – TehHO Aug 24 '12 at 20:20
  • @Cfreak yes id is indexed, the table is not very big. It holds 8 rows. 1 ID, INT 1 headline, varchar(20) 1 content, text - a few lines - not huge at all.. – TehHO Aug 24 '12 at 20:21
  • 1
    It is important to note that the `mysql_xxx()` functions are considered obsolete. The PHP manual recommends switching to using either the `mysqli_xx()` functions or the PDO library. Either of these is more up-to-date than `mysql_xx()`. In addition, they both feature parameterised query features, which allows you to avoid SQL injection attacks without having to escape everything. – Spudley Aug 24 '12 at 20:46
  • Your code has nothing that would prevent a user from entering a custom value. – Cfreak Aug 24 '12 at 20:51
  • @TehHO Set `$_POST['id']` to `'0'; DROP TABLE about;` and see if you still have your data. You could change `DROP TABLE about` to `DELETE FROM about` if your DB user doesn't have permissions to do that. That's one way it would be vulnerable to SQL injection. – WWW Aug 24 '12 at 21:01
  • @Crontab `mysql_query` only executes one query at a time but you have a point. Anyone can POST anything as `id` to the server, they don't need to use a browser. – Esailija Aug 24 '12 at 21:03

2 Answers2

1

write to phpmyadmin the sql: Explain SELECT * FROM about WHERE id=5

if is nagging for id is not indexed, than there you have the problem, easy:)

if you have a loooot of fields in about table, better use field1,field2 instead of *

That is all what can you do for speed up select from sql, others are settings, and hardware, like mysql sever has 1Mb free to run you server and need to do a lot of swapping or your processor is Intel Pentium 1 or 2 something like that...bcause you are connection to localhost, not remote

Aaron
  • 55,518
  • 11
  • 116
  • 132
  • Alright! Cheers. it didn't bring up any errors. The table holds 3 fields only, so that should not be the problem and also I'm not working with anything that's so big that the hardwars is the problem. Without being an expert I'm pretty sure of that... Guess I'll have to live with it... – TehHO Aug 24 '12 at 20:24
  • it could be the cheap php hosting on the oversold site too: you get less cpu time –  Aug 24 '12 at 20:30
1

Addressing the SQL Injection aspect:

How can I be vulnerable to SQL injection? There is no way for the user to input a custom value. I know they could change the ID but what fun would that be? it would then load nothing...

You're pulling $id directly from the post.

$id =  $_POST['id'];

Assuming we have a smart attacker, manipulating the post's data isn't difficult. The SQL Injection attack type that @DanielA.White is warning you of banks on the fact that you're not making sure $id is a number. Because of that, let's pretend that I set the value for id in the post to:

'3; DROP TABLE about;'

Now you're in trouble - the table you were referencing is gone. See http://xkcd.com/327/ for more info. ;)

Sanitizing your input is actually pretty simple - just look here: What's the best method for sanitizing user input with PHP?

Community
  • 1
  • 1
Nick Rippe
  • 6,465
  • 14
  • 30