Help with makeing the code beter, faster, and find stupid code lops!!

Chrysanthus

it woulde be great if some one coulde take a look att the code and se if i can make some improvments and if there are any botnecks i the code!!

<?php
$username=$POST["username"];

$password=$POST["password"];

include ("../managment/mysqlcondbrr.php");

$mysqlcon = "select * from reg_info where username = '$username' and password = '$password';";

$result = mysql_query($mysqlcon) or die("Query failed");

if (mysql_num_rows($result) == '1') {

print "<center><big><big>Welcome $username Please click <a href=\"isloggedin.php\">here to continue to flameline.com</big></big></center>";

$SESSION["username"] = $username;
$SESSION["password"] = $password;
$SESSION["validuser"] = "1";
$SESSION["loggins"] = "1";
}

else

{
$_SESSION["unvaliduser"] = "0";
print "<center><big><big>Sorry, incorrect Password $password. or Nickname $username.</big></big></center>";

print "<center><big>if not a member <a href=\"register.php\">click here to register</big></center>";
}

HalfaBee

When you post php use the php button just above or put the code in [ PHP] [/ PHP] tags ( without the space.
Like this.

<?php
$username = $_POST["username"];
$password = $_POST["password"];

include ("../managment/mysqlcondbrr.php");

$mysqlcon = "select * from reg_info where username = '$username' and password = '$password';";
$result = mysql_query($mysqlcon) or die("Query failed");

if (mysql_num_rows($result) == '1') {
    print "<p><center><big><big>Welcome $username Please click <a href=\"isloggedin.php\">here to continue to flameline.com</big></big></center></p>";

$_SESSION["username"] = $username;
$_SESSION["password"] = $password;
$_SESSION["validuser"] = "1";
$_SESSION["loggins"] = "1";
} else {
    $_SESSION["unvaliduser"] = "0";
    print "<p><center><big><big>Sorry, incorrect Password $password. or Nickname $username.</big></big></center></p>";
    print "<p><center><big>if not a member <a href=\"register.php\">click here to register</big></center></p>";
} 

?>

Your code has no loops, unless they are in the include file.

HalfaBee

shareaweb

include_once() is marginally faster. Any reason why your selecting all instead of just the columns you need? Just a thought...
Any do you have output bufferring enabled?

Chrysanthus

Your code has no loops, unless they are in the include file.

HalfaBee

Nop that's only the db connection !!

///

shareaweb

include_once() is marginally faster. Any reason why your selecting all instead of just the columns you need? Just a thought...

Answer: That's a good question i shoulde use only password and username columns?? :p

Any do you have output bufferring enabled?

Whats tha't what do that Improve

BuzzLY

Originally posted by Chrysanthus
$SESSION["loggins"] = "1";

session_destroy();
if ($_SESSION['loggins'] != "1") {
    echo "Oh my God, they killed Kenny!!!";
}

😃😃😃😃

BTW... use single quotes in your $_SESSION array, not double quotes.

Chrysanthus

okay but i have one stupid question that i wanna ask what makes the diffrence with use single quotes insted of dubbel ???

PHP:

session_destroy();
if ($_SESSION['loggins'] != "1") {
echo "Oh my God, they killed Kenny!!!";
}

goldbug

IIRC, one reason is because PHP evaluates everything in double quotes (which is why you can throw variable names in there), and just treats single-quoted text as a scalar value, throwing it around as-is. Single quotes thusly provides you with a very minor performance increase.

my 2c.

goldbug

Also, you're setting a bunch of session variables to 0 or 1.

Is this supposed to indicate a true/false state of some kind? (Are numbers really required?)

If possible consider something like:

$_SESSION['foo'] = TRUE;

then it makes your code a little cleaner:

if ($_SESSION['foo']) {
...
}

Of course, you could still use the if statement above with your current values (0/1 evaluate to false/true).

HalfaBee

TRUE & FALSE are just numbers.

It could take longer to put a defined number rather than an explicit number.

But I prefer TRUE/FALSE also. 🙂

Also I would not use
$_SESSION["unvaliduser"] = "0";

If they are not valid set
$_SESSION["validuser"] = 0;

Halfabee

Chrysanthus

okay sow this is the best way! my ide was that give the user a login level per specified user but that can be a later brainstorm.

<?php
$username = $POST["username"];
$password = $POST["password"];

include ("../managment/mysqlcondbrr.php");

$mysqlcon = "select * from reg_info where username = '$username' and password = '$password';";
$result = mysql_query($mysqlcon) or die("Query failed");

if (mysql_num_rows($result) == '1') {
print "<center><big><big>Welcome $username Please click <a href=\"isloggedin.php\">here to continue to flameline.com</big></big></center>";

$_SESSION["username"] = $username; 
$_SESSION["password"] = $password; 
$_SESSION['validuser'] = TRUE;
$_SESSION["loggins"] = "1";

} else {
$_SESSION['unvaliduser'] = FALSE;
print "<center><big><big>Sorry, incorrect Password $password. or Nickname $username.</big></big></center>";
print "<center><big>if not a member <a href=\"register.php\">click here to register</big></center>";
}

?>
BuzzLY

did you mean thate code shoulde look like this

$_SESSION['loggins'] = "1";

HalfaBee

There is no need for the 'unvaliduser' session variable.
Just set $_SESSION['validuser']=FALSE;

HalfaBee

Chrysanthus

Thats rigtht way do i need that if there are now value in the session file tha's not TRUE The user will be kicked!

Chrysanthus

ahha PHP button thanks i wounder how all you user gott color now i know that to!

rythmic

Hi!

I think that the logincheck that you use is not safe. It can easily be passed using sql injection where you would just limit your answer to return one line.

You should check the password so that it actually equals the password in the database.. like this example below:

else{
	/* Let's validate login */

$given_user = $_POST['username'];
$given_pass = md5($_POST['password']);

$query = "SELECT $tbl_users_uid,$tbl_users_pass,$tbl_users_isb FROM $tbl_users WHERE $tbl_users_uname='$given_user' LIMIT 1";
$result = mysql_query($query,$connection);
if(mysql_num_rows($result) != 0){
    $row = mysql_fetch_assoc($result)or die(mysql_error());

    /* If password is correct and user is not blocked login should be approved */
    if($row["$tbl_users_pass"] == $given_pass && $row["$tbl_users_isb"]!= 1){
	$_SESSION['user_name'] = $given_user;
	$_SESSION['user_id'] = $row["$tbl_users_uid"];
	register_login($row["$tbl_users_uid"]);
	unset($_SESSION['failed_attempts']);
	header('Location: index_inside.php');
}

It is taken from a login code from one of my pages and as you can see there is little hope of getting through without providing a correct password for an existing user.

cheers 🙂