Cleaning up this code!

[Teensweb]

I have a php code for a 5 star rating swervice i am gonna provide. But unfortunately, I had to write the code myself and I have messed it up a lot.So I need help to make it clean and fast.

<?php
require ( '/home/avin/public_html/teensweb.co.cc/Login/settings.php' );
?>
<?php

if(isset($_POST['submit'])) //has the form been submitted?

{
$id = $login->get_username ( $_SESSION [ AUTH_SESSION_ID ] );

$file_directory = "files/"; //the directory you want to store the new file in

$file_name = strip_tags($_POST['file_name']);//the file's name, stripped of any dangerous tags

$prefix = "$id-$file_name"; 

$file_ext = ".php"; //the file's extension, stripped of any dangerous tags

$file = $file_directory.$prefix.$file_ext; //this is the entire filename

$outof = strip_tags($_POST['number']);

$percent = strip_tags($_POST['percent']);

$votes = strip_tags($_POST['votes']);

$tagopen = '<?';

$tagclose = '?>';

$color = strip_tags($_POST['color']);

$votes = strip_tags($_POST['votes']);

$css = strip_tags($_POST['extension']); 
$html = '<body';
$htclose = '>';
$filename = "files/$id-$file_name$file_ext";
$background = "$html bgcolor=$color$htclose";
if (file_exists($filename))
{
    unlink($filename);
}
$create_file = fopen($file, "w+"); //create the new file
if ($css = png) {
$content = '<? include("includes/rating_functions.php"); ?>
<link href="css/rating_style.css" rel="stylesheet" type="text/css" media="all">
<script type="text/javascript" src="js/rating_update.js"></script>';
}
else {
$content = '<? include("includes/rating_functions.php"); ?>
<link href="css/rating_style_gif.css" rel="stylesheet" type="text/css" media="all">
<script type="text/javascript" src="js/rating_update.js"></script>';
}
$phfiles = "$tagopen echo pullRating($file_name,$outof,$percent,$votes) $tagclose";
$code = 'Rating sytem created success fully! copy and paste the following code where you want the stars to appear<textarea rows="5" cols="50"><iframe src=http://www.teensweb.co.cc/' . $filename . ' frameborder="0" ></textarea>';
if(!$create_file)

{

die("There was an error creating/opening the file! Make sure you have the correct permissions!\n");

}

$chmod = chmod($file, 7545); //set the appropriate permissions.

//attempt to set the permissions

if(!$chmod) 

{ 

echo("There was an error changing the permissions of the new file!\n"); //error changing the file's permissions

}

//attempt to write basic content to the file

if (fwrite($create_file, "$content$phfiles$background") === FALSE) {

echo "Error writing to file: ($file)\n";

}

fclose($create_file);

}else{ //the form hasn't been submitted!

header("Location: addfile.html"); //redirect the user back to the add file page

}

if($create_file)
{echo $code;}

?>

Thank you in advance!
Avin

 

[TechAsh]

I've cleaned up the layout a bit (This won't speed it up, but it makes it easier to read.)

<?php
require('/home/avin/public_html/teensweb.co.cc/Login/settings.php');

if(isset($_POST['submit'])) { //has the form been submitted?
	$id = $login->get_username ($_SESSION['AUTH_SESSION_ID']);
	$file_directory = 'files/'; //the directory you want to store the new file in
	$file_name = strip_tags($_POST['file_name']); //the file's name, stripped of any dangerous tags
	$prefix = "$id-$file_name"; 
	$file_ext = '.php'; //the file's extension, stripped of any dangerous tags
	$file = $file_directory.$prefix.$file_ext; //this is the entire filename
	$outof = strip_tags($_POST['number']);
	$percent = strip_tags($_POST['percent']);
	$votes = strip_tags($_POST['votes']);
	$tagopen = '<?';
	$tagclose = '?>';
	$color = strip_tags($_POST['color']);
	$votes = strip_tags($_POST['votes']);
	$css = strip_tags($_POST['extension']); 
	$html = '<body';
	$htclose = '>';
	$filename = "files/$id-$file_name$file_ext";
	$background = "$html bgcolor=$color$htclose";
	
	if (file_exists($filename)) {
		unlink($filename);
	}
	
	$create_file = fopen($file, "w+"); //create the new file
	if ($css = png) {
		$content = '<? include("includes/rating_functions.php"); ?>
		<link href="css/rating_style.css" rel="stylesheet" type="text/css" media="all">
		<script type="text/javascript" src="js/rating_update.js"></script>';
	}
	else {
		$content = '<? include("includes/rating_functions.php"); ?>
		<link href="css/rating_style_gif.css" rel="stylesheet" type="text/css" media="all">
		<script type="text/javascript" src="js/rating_update.js"></script>';
	}
	$phfiles = "$tagopen echo pullRating($file_name,$outof,$percent,$votes) $tagclose";
	$code = 'Rating sytem created success fully! copy and paste the following code where you want the stars to appear<textarea rows="5" cols="50"><iframe src=http://www.teensweb.co.cc/'.$filename.' frameborder="0" ></textarea>';
	
	if(!$create_file) {
		die("There was an error creating/opening the file! Make sure you have the correct permissions!\n");
	}
	
	$chmod = chmod($file, 7545); //set the appropriate permissions.
	//attempt to set the permissions
	if(!$chmod) { 
		echo("There was an error changing the permissions of the new file!\n"); //error changing the file's permissions
	}
	
	//attempt to write basic content to the file
	if (fwrite($create_file, "$content$phfiles$background") === FALSE) {
		echo "Error writing to file: ($file)\n";
	}
	
	fclose($create_file);
	}
	
	else { //the form hasn't been submitted!
		header("Location: addfile.html"); //redirect the user back to the add file page
	}
	
	if($create_file) {
		echo $code;
	}
?>

 

[woiwky]

I believe you meant to use a double = (equality test) rather than a single one for "if ($css = png)". You should also have png in quotes unless it's a constant defined in that require()'d file. Though php will only issue a Notice level error for an unquoted string in that context, it's not good practice to do that.

Also, you should have exit(0); after the header() redirect. In this case it's probably not absolutely needed, but it's always a good idea to end script execution after a redirect.

And are hyphens allowed in usernames? If so, then there's possible filename collision.

Furthermore, you should have a closing </iframe> tag in the code provided inside the <textarea>. If that code were copied without the closing tag, some browsers may render the iframe improperly.

Lastly, you should sanitize $color further. strip_tags() won't take out a single right angle bracket(>), which would end the body tag. Nor would it clean a string like:

black onload="some JS code here"

Simple way to clean it would be to just remove ='s and >'s.

Everything else looks ok, though I might have missed something.

 

[Teensweb]

I mean I used certain dirty tricks such as $tagopen, $tagclose,
$html,etc can you clean those up?