PHP help

Jekyll

Jekyll

Jekyll

Associate
Joined
25 Feb 2009
Posts
38
Hi all

I started to learn PHP today and I've just finished my first page, yay me! :D

Can you look at my code and see if I've done it right, if theres a better way of doing it, and if there are any security risks I should worry about please?

PHP: 
<?php
// Make a MySQL Connection
mysql_connect("localhost", "username", "password") or die(mysql_error());
mysql_select_db("DB") or die(mysql_error());

// Get all the data from the links table
$result = mysql_query("SELECT * FROM Links ORDER BY name") 
or die(mysql_error());  


// keeps getting the next row until there are no more to get
while($row = mysql_fetch_array( $result )) {
	// Print out the contents of each row into a table

if ( $row['logo'] == NULL ) {
	echo "<p><a href='" . $row['link'] . "' target='_blank' title='" . $row['alt'] . "'>";
	echo $row['name'];
	echo "</a> - ";
} else {
	echo "<p><a href='" . $row['link'] . "' target='_blank'><img border='0' src='" . $row['logo'] . "' width='" . $row['width'] . "' height='" . $row['height'] . "'";
	echo " alt='" . $row['alt'] . "' title='" . $row['alt'] . "' />";
	echo "</a><br />";
	echo "<a href='" . $row['link'] . "' target='_blank' title='" . $row['alt'] . "'>" . $row['name'] . "</a> - ";

}
	echo $row['description'];
	echo "</p>";
} 


?>

Not sure if it matters with security, but when I added the user to the database using MySQL Databases, I set the user privileges to SELECT only.

Thanks,
Jekyll
 
You could use sprintf to make those echos nicer. I.e. you would use something like this instead:

PHP: 
// from this
echo "<p><a href='" . $row['link'] . "' target='_blank' title='" . $row['alt'] . "'>"; 

// to this
printf("<p>a href='%s' target='_blank' title='%s'", $row['link'], $row['alt']);

Or you could mix PHP/HTML together more. This has the benefit of making it easier to read for the most part and it helps to split your template from your code:
PHP: 
<a href="<?="$row['link']?>" target="_blank" title="<?=$row['alt']?>">

Who sets the values for the database tables? You need to escape them when printing to the webpage to prevent SQL injection etc. Have a read here

You shouldn't really echo mysql errors to the user, you could potentially give away your database username, your table structures etc - it's best to keep that private by writing to an error log or something.
 
Steve - I used the tutorials on http://www.w3schools.com and http://www.tizag.com.

Thanks all for the help and info, I appreciate it.

Wow, seems PHP is going to be harder to learn then I thought it would be :( , theres more to it then just connecting to a database and inputting/outputting data.

Pho - Thanks. I had a look at the sprintf, but it went way over my head, i'll have another look sometime when i've got a bit more used to PHP and see if I can figure it out. I did use the mix PHP/HTML option and came up with

PHP: 
<p><a href="<?=$row['link']?>" target="_blank" title="<?=$row['alt']?>">
<?
	echo $row['name'];
	echo "</a> - ";
} else { ?>
	<p><a href="<?=$row['link']?>" target='_blank'><img border='0' src="<?=$row['logo']?>" width="<?=$row['width']?>" height="<?=$row['height']?>" alt="<?=$row['alt']?>" title="<?=$row['alt']?>" /></a><br />
	<a href="<?=$row['link']?>" target='_blank' title="<?=$row['alt']?>">

So now I have two options for making links :)

The error log, I checked on the link you gave me and it had some info there, it said to store the error log ina path that is writeable by the server process .. any suggestions on where this might be? I'm guessing the /public_html wouldn't be the right place.


dangerstat - Thanks for the suggestion about SQL injection. I checked it out but i'm a bit confused, do I need to do that for every page that inputs/ouputs to/from the db? I got the impression that it was for inputting user data from a form? This page i've done is just a normal links page, with a list of links that i've entered into the db using phpMyAdmin.

I did give it ago, it came up with an error at first, saying something about expecting a string .. probably because the id in the links table is an int, which I could probably do without, so I split it up and came up with this

PHP: 
$row = mysql_real_escape_string($result['name']);
$row = mysql_real_escape_string($result['logo']);
$row = mysql_real_escape_string($result['link']);
$row = mysql_real_escape_string($result['alt']);
$row = mysql_real_escape_string($result['description']);
$row = intval($result['id']);
$row = intval($result['width']);
$row = intval($result['height']);

Which could be way off .. I was suffering with infomation overload, which is one of the reasons why i've done nothing with it since saturday :D

msm722 - Thanks for the info. I had a quick look into it, i'll have a better look later though. I found a link on php.net which again went way over my head, but then I found http://www.tuxradar.com/practicalphp/9/18/0 which made it a bit easier to understand, but how is it improved, are they more secure?

Sorry for the long post and many many questions. As I said I only started saturday and i'm no programmer, but I have a bit of an interest in web development and i'd like to be able to do more then just use HTML and CSS.

Thanks again for all the help and suggestions :)
 
msm722 said:
Have a look at the MySQL Improved Extension.
Click to expand...

Ditto.

And to expand on that, I would advise that #1 you use prepared statements, as these are more secure than the old fashion way, and #2 use the InnoDB engine on MySQL. This will give you support for constraints and transactions, which make life much easier.
 
Programming is fairly simple as long as you do not have to consider security or users doing unexpected things. Its a learning process and you cannot expect to grasp every fascet of programming quickly. Most of the time you are only building small scale apps at the start so security isnt an issue and you would be better served learning other things.
 
You must log in or register to reply here.
Back
Top Bottom