Gaaah! Error codes and everything!
Moderator: General Moderators
-
Mightywayne
- Forum Contributor
- Posts: 237
- Joined: Sat Dec 09, 2006 6:46 am
Yeah, I know, I don't really like caps though. They annoy me. I'm one to instantly ignore spammers regardless.
And... defines can't be changed by hackers. I don't know how they hack, but post values are supposed to be defined so they can't be manipulated. That's what I learned. =/ I'm ready to experience slight slowness for security, you see.
Edit: And, if defines are really that bad... then what would be appropriate to use them for?
And... defines can't be changed by hackers. I don't know how they hack, but post values are supposed to be defined so they can't be manipulated. That's what I learned. =/ I'm ready to experience slight slowness for security, you see.
Edit: And, if defines are really that bad... then what would be appropriate to use them for?
- RobertGonzalez
- Site Administrator
- Posts: 14293
- Joined: Tue Sep 09, 2003 6:04 pm
- Location: Fremont, CA, USA
Where did you learn this? Constants cannot be changed after they are defined. That is true. But why in the world would you need to define your POST vars? If a hacker is going to hack you, they are more than likely going to do it at the entry point (where the post data comes in) at which point you are hosed because you just made malicious data unchangeable.Mightywayne wrote:And... defines can't be changed by hackers. I don't know how they hack, but post values are supposed to be defined so they can't be manipulated. That's what I learned.
It depends on your needs. I use them for URL params sometimes, like this...Mightywayne wrote:Edit: And, if defines are really that bad... then what would be appropriate to use them for?
Code: Select all
<?php
define('PAGENAME', 'p');
?>Code: Select all
<?php
$link_to = 'fatpenguinpicures';
echo '<a href="index.php>' . PAGENAME . '=' . $link_to . '" title="See penguins!">Show me the penguins!</a>';
?>Code: Select all
<?php
if (isset($_GET[PAGENAME]))
{
$page = $_GET[PAGENAME];
}
?>- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
Everah's example demonstrates two advantages to constants. Whilst...
does achieve the same result as the use of the constant is preferred as the meaning of p is made clear and if you mistype a constant you get an error telling you about it instead of mistyping the parameter itself where you would get a silent bug.
Code: Select all
echo '<a href="index.php?p=' . $link_to . '"Code: Select all
echo '<a href="index.php?' . PAGENAME . '=' . $link_to . '"
Last edited by Ollie Saunders on Fri Dec 22, 2006 6:07 pm, edited 1 time in total.
- jyhm
- Forum Contributor
- Posts: 228
- Joined: Tue Dec 19, 2006 10:08 pm
- Location: Connecticut, USA
- Contact:
Hmm just a curiosity,
You guys don't use "?" to identify vars in th URL
i.e.
You guys don't use "?" to identify vars in th URL
i.e.
Code: Select all
$wutevervals=$_POST ["wutevah"];
$urlString="http://www.yoursite.com/index.php";
$addQuest="?"; // In its own var just for demonstration purposes
$daVars="variable=";
$setURLvars= $urlString . $addQuest . $daVars . $wutevervals;
//$setURLvars Would equal "http://www.yoursite.com/index.php?variable=10" if I set $wutevah to 10- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
- RobertGonzalez
- Site Administrator
- Posts: 14293
- Joined: Tue Sep 09, 2003 6:04 pm
- Location: Fremont, CA, USA
Given the nature of the code monkeying I have been doing, I would say that little omission was a small oversightole wrote:Hehe.. nahh. Everah can't type and I can't read
Small happening at work yesterday wrote:me: Why does my error trigger keep throwing this friggin error?!?!?!?!?
coworker: Maybe if you added a second equal sign when checking the equality?
me: %$$@#&T%T*@%$*&(&^%@@))*@()*&$^^@#^*(@)$()_*^*(@^)(_@_((*@$. I'm going home.
- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
lol. We all have our gotchas; happens to the best of us.Everah wrote:Small happening at work yesterday wrote:me: Why does my error trigger keep throwing this friggin error?!?!?!?!?
coworker: Maybe if you added a second equal sign when checking the equality?
me: %$$@#&T%T*@%$*&(&^%@@))*@()*&$^^@#^*(@)$()_*^*(@^)(_@_((*@$. I'm going home.
-
Mightywayne
- Forum Contributor
- Posts: 237
- Joined: Sat Dec 09, 2006 6:46 am
- RobertGonzalez
- Site Administrator
- Posts: 14293
- Joined: Tue Sep 09, 2003 6:04 pm
- Location: Fremont, CA, USA
-
timclaason
- Forum Commoner
- Posts: 77
- Joined: Tue Dec 16, 2003 9:06 am
- Location: WI
Here's How I would do it
Here's how I would do it procedurally (in reality, I'd do it with objects/classes AND I'd make the methods a bit different, but I was just kind of converting your code, as opposed to writing it from scratch):
I'd start out with the functions:
Then I'd make the code simpler:
PHP Gurus: Feel free to crucify me with "That's bad coding..." comments 
I'd start out with the functions:
Code: Select all
// Function List
function findSymbol($address, $symbol) {
if($symbol == "AT")
$returnVal = strpos($address, "@");
elseif($symbol == "DOT")
$returnVal = strpos($address, ".");
else
$returnVal = 0;
return $returnVal;
}
function checkForBlanks($field, $input) {
if(!$input) {
print("You are missing input for " . $field . ".<BR><a href='....'>Click Here</A> to fix it");
$errorCount = 1;
return $errorCount;
}
}
function checkAllInput($P_password, $P_cpassword, $atSymbol, $dotSymbol) {
if($P_password != $P_cpassword) {
print("Passwords Do Not Match");
$errorCount = 1;
return $errorCount;
}
elseif(!$atSymbol || !$dotSymbol) {
print("Invalid Email Address");
$errorCount = 1;
return $errorCount;
}
}
function preventDuplicates($field, $val) {
if($field == "USERNAME") {
$query = "SELECT username FROM...";
$SQL = mysql_query($query, $dbLink);
}
elseif($field == "EMAIL") {
$query = "SELECT email FROM...";
$SQL = mysql_query($query, $dbLink);
}
if(@mysql_num_rows($SQL) {
print("That " . $field . " already exists");
$errorCount = 1;
return $errorCount;
}
}Code: Select all
//Start Code
$con = mysql_connect("localhost","****","****");
if (!$con)
{
die('Could not connect: ' . mysql_error());
}
$un = $_POST['username'];
$pw = $_POST['password'];
$cpw = $_POST['cpassword'];
$email = $_POST['email'];
//Checking for typo's and stuff now.
$findat = findSymbol($email, AT);
$finddot = findSymbol($email, DOT);
if( !checkForBlanks(USERNAME, $un) &&
!checkForBlanks(PASSWORD, $pw) &&
!checkForBlanks(CONFIRM_PASSWORD, $cpw) &&
!checkForBlanks(EMAIL, $email) &&
!checkAllInput($pw, $cpw, $findat, $finddot) &&
!checkForUniqueEmail($email) &&
!preventDuplicates(USERNAME, $un) &&
!preventDuplicates(EMAIL, $email)
) {
mysql_select_db("burnttoa_monbre", $con);
mysql_query("INSERT INTO user (ID, name, pword, email, money, ranch, barn, happy, tlevel, verified)
VALUES ('', '".$un."', '".$pw."', '".$email."', '3000', '1', '1', '70', '1', 'no')")
or die(mysql_error());
mysql_query("INSERT INTO items (userbagID)
VALUES ('')")
or die(mysql_error());
echo "Welcome to Monbre, ".username."! Soon you will get an email that requires your email adress to be verified before actually allowing acess to the game.";
} //End if statement- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
Am I guru enough? Because, you know, I just can't resist such invitationsPHP Gurus: Feel free to crucify me with "That's bad coding..." comments
I'm just going to critique findSymbol() only I think.
- No docblock comment. What is findSymbol() supposed to do?
- findSymbol() seems to be a very simple wrapper for strpos() to restrict what needle you can use, is there really any point to its existence at all?
- Personally I don't like any if, else, elseif, for, while etc. without braces ( {} ); unbraced control structures are more prone to bugs.
- Usage of double quoted strings when not using double quote functionality. They are slightly slower to process and generally I like to reserve them for when I'm using escapes or inline variable substitution so when I do such things it is more obvious.
- Temp variable $returnVal makes code complex. return, continue and break are your friends and often really go a long way to improving readability.
- Future modifications to findSymbol to search for other symbols would require you to add more conditions. Array?
Code: Select all
/**
* Wrapper to strpos() designed to restrict to certain hard-coded needles.
*
* @param string $haystack
* @param string $needleName name of hard-coded needle to use
* @param int $limit as in strpos()
* @return int
*/
function findSymbol($hayStack, $needleName, $limit = null)
{
static $needles = array('AT' => '@', 'DOT' => '.');
if (!isset($needles[$needleName])) {
return false;
}
if (is_int($limit)) {
return strpos($hayStack, $needles[$needleName], $limit);
}
return strpos($hayStack, $needles[$needleName]);
}- RobertGonzalez
- Site Administrator
- Posts: 14293
- Joined: Tue Sep 09, 2003 6:04 pm
- Location: Fremont, CA, USA
Alright, here's my take on it...
Code: Select all
<?php
// Connect to the database server
if (!$con = mysql_connect('localhost','****','****'))
{
die('Could not connect: ' . mysql_error());
}
// Select our database
if (!mysql_select_db('mydatabase', $con))
{
die('Could not select the database: ' . mysql_error());
}
// Default values, to prevent uninitialized variables
$username = '';
$password = '';
$cpassword = '';
$email = '';
if (!empty($_POST))
{
$username = $_POST['username'];
$password = $_POST['password'];
$cpassword = $_POST['cpassword'];
$email = $_POST['email'];
if (empty($username) || empty($password) || empty($cpassword) || empty($email))
{
die('You must fill out all fields! Go back and fix it.');
}
// This is not the most effective way to check email addresses
// But this is moreso to show you an easier way to do what you were doing
if (!strpos($email, '@') || !strpos('.', $email))
{
// Yes, there are better ways to handle bad
// information, but I'll let you figure those ways out
die('There is an error in your email address.');
}
// Check password/confirm match
if ($password != $cpassword)
{
die('Your password confirmation did not match your password.');
}
// Check username and emails in the table
$sql = "SELECT `ID`
FROM `user`
WHERE `name` = '$username'
OR `email` = '$email'";
if (!$result = mysql_query($sql))
{
die('There was an error in the query: ' . mysql_error());
}
if (mysql_num_rows($result) > 0)
{
// There is a record somewhere with that username or email
die('The email or username you entered is not available.');
}
// We need to enter the user if they get this far
$sql = "INSERT INTO user
(`ID`, `name`, `pword`, `email`, `money`, `ranch`, `barn`, `happy`, `tlevel`, `verified`)
VALUES
('', '$username', '$password', '$email', 3000, 1, 1, 70, 1, 'no')";
if (!mysql_query($sql))
{
die('Could not insert the user: ' . mysql_error());
}
$sql = "INSERT INTO `items` (`userbagID`) VALUES ('')";
if (!mysql_query($sql))
{
die('Could not insert the bag: ' . mysql_error());
}
echo 'Welcome to Monbre, ' . $username . '! Soon you will get an email that requires your email adress to be verified before actually allowing acess to the game.';
}
?>You blindfully assume they will exist in $_POST...Everah wrote:Code: Select all
if (!empty($_POST)) { $username = $_POST['username']; $password = $_POST['password']; $cpassword = $_POST['cpassword']; $email = $_POST['email']; if (empty($username) || empty($password) || empty($cpassword) || empty($email)) { die('You must fill out all fields! Go back and fix it.'); }
You did not prepare your variables for use in a MySQL query.Everah wrote:Code: Select all
$sql = "SELECT `ID` FROM `user` WHERE `name` = '$username' OR `email` = '$email'"; $sql = "INSERT INTO user (`ID`, `name`, `pword`, `email`, `money`, `ranch`, `barn`, `happy`, `tlevel`, `verified`) VALUES ('', '$username', '$password', '$email', 3000, 1, 1, 70, 1, 'no')";
Why do you assign a link to the db resource $con if you're not going to use it for your following mysql_* calls?
You haven't prepared the data for use in a html context...Everah wrote:Code: Select all
echo 'Welcome to Monbre, ' . $username . '! Soon you will get an email that requires your email adress to be verified before actually allowing acess to the game.'; ?>