STOP dropping in and out of php parsing mode. You’re just making the code more complex for no good reason. (But again, I’m the guy who thinks <?php and ?> should be removed from the language!)
if you’re going to copy values into variables, SANITIZE them unless you’re using prepared queries. (which you aren’t).
you don’t have to say $msg=$msg." – $msg.=" is just fine.
Do not process unused values until you NEED them… otherwise it’s a waste of execution time.
INDENT… your missing closes would stand out like a sore thumb then. Simply adding tabs and a few carriage returns can work WONDERS. A few extra spaces in there couldn’t hurt either… You’ve got this… oddball placement of closing brackets and other bits and pieces of the code that’s just BEGGING for you to have those types of errors.
STOP using double-quotes on your strings. They take longer, and make the code often harder to deal with. (again, WHAT is with people doing that?!?)
you lack enough parenthesis in your evaluations. PHP screws up comparisons as && is also a valid compare… which will run BEFORE your <5
SO… my version would probably looks something more like this:
<?php
function sanitizeFromPost($postName){
if isset($_POST[$postName]) {
$str=(
get_magic_quotes_gpc() ?
stripslashes($_POST[$postName]) :
$_POST[$postName]
);
return (
function_exists('mysql_real_escape_string') ?
mysql_real_escape_string($str) :
addslashes($str)
);
} else return '';
}
// file name is test_form_ck.php
include "include/db_login.php";// database connection details stored here
// Collect the data from post method of form submission //
echo '
<!doctype html>
<html><head>
<meta charset="UTF-8">
<title>TEST Signup FORM</title>
</head><body>';
if (
isset($_POST['todo']) &&
($_POST['todo']=="post")
) {
$msg='';
$userid=sanitizeFromPost('userid');
if (strlen($userid)<5) {
$msg.='User ID should be 5 or more than 5 char length<br>';
}
if (empty($msg)) {
$password=sanitizeFromPost('password');
$password2=sanitizeFromPost('password2');
$email=sanitizeFromPost('email');
$name_last=sanitizeFromPost('name_last');
$name_first=sanitizeFromPost('name_first');
$query=mysql_query("
INSERT INTO member_tbl
(user_id,password,email,name_last,name_first)
VALUES
('$userid','$password','$email','$name_last','$name_first')
");
echo "Welcome, You have successfully submitted new member information<br><br>";
} else {
echo $msg.'<br><input type="button" value="Retry" onClick="history.go(-1)">';
}
}
echo '
</body></html>';
?>
[ot]I’d also suggest kicking the HTML 5 nonsense to the curb since it’s doing nothing but setting coding practices back a decade or more…
But of course, no one ever listens to poor Zathras no, he’s quite mad they say. It is good that Zathras does not mind, has even grown to like it.[/ot]
I’d also kick the mysql_ functions even harder, and switch at LEAST to mySQLi, or even better PDO. This isn’t 2003.
Oh, also notice I got rid of the ok variable. If you add a error message, there are errors; as such all you have to do is check if $msg is empty. No need for the extra variable. I made my sanitize function return an empty string, so for the userid check all you have to do is check the length.
I copied your code to see if it would run in my environment, and it did not. I get a:
Parse error: syntax error, unexpected T_ISSET, expecting ‘(’ in test_form_ck_r01.php on line 4
Not sure why it does not work. Maybe something is not enabled on my site, or the config file is not set correctly.
I did find information on PHP Data Objects, and I have started studying the information.
I appreciate the assistance. I will continue working with this code.
PHPINFO says: PDO Driver for MySQL, client library version 5.0.92, and PHP Version 5.2.17.
Line 4 is a my bad. where it says
if isset($_POST[$postName]) {
should say
if (isset($_POST[$postName])) {
PHP’s a real prig about that, and I’ve been updating some old Paradox code where the extra () aren’t necessary. (sad when I’m still providing support for software I wrote almost twenty years ago) In php, ALL binary comparisons even when resolved need to be wrapped for ‘if’ – really stupid the language isn’t smart enough to work without the extra wrapping parenthesis.
Really funny because I pointed out you did that in your original – then I go and do it myself
PHP 5.2 is old at this point (2006)… even with .17 being a security patch from january this year, I’d SERIOUSLY be asking the provider to get you up to 5.3.6; the latest recommended version… since 5.2 is officially no longer supported, with 5.2.17 being the last official update.
PDO isn’t really all that much difficult from the normal mysql_ functions. The most fundamental difference is prepared queries – if you can use double-quote strings or the printf function, you shouldn’t get too lost on how to use those.
function isValidEmail($address) {
if (filter_var($address,FILTER_VALIDATE_EMAIL)==FALSE) {
return false;
}
/* explode out local and domain */
list($local,$domain)=explode('@',$address);
$localLength=strlen($local);
$domainLength=strlen($domain);
return (
/* check for proper lengths */
($localLength>0 && $localLength<65) &&
($domainLength>3 && $domainLength<256) &&
(
checkdnsrr($domain,'MX') ||
checkdnsrr($domain,'A')
)
);
}
Something a group of us working together on another forums came up with. Checks it for valid characters, then valid lengths, and then finally that it’s pointed at a valid domain.
@ is the error suppression character, you can avoid Warnings displaying on specific function calls by prepending the call with @
For example:
@mysql_num_rows(mysql_query(“…”));
Bear in mind this hides any error from that function, so only use it on num_rows (where a warning is expected if there are no results to the query) or other locations where the same applies, not all over the place or you’ll end with pages showing no error and no idea what is wrong!
If you are using a non-prepared query, where basically you are running anything from the user side ($_POST) into your query directly building it as a string, you should sanitize the values so as to prevent script injections.
If you switch to mysqli or PDO, and use prepared queries, it sanitizes for you and you can forget that function.
I thought I had it working, but if I put in a bad user name (<5 characters) I get a blank page, and the $msg does not display.
<?php
// file name is test_form_ck_r01.php
ini_set('display_errors',1);
error_reporting(E_ALL);
function sanitizeFromPost($postName){
if (isset($_POST[$postName])) {
$str=(
get_magic_quotes_gpc() ?
stripslashes($_POST[$postName]) :
$_POST[$postName]
);
return (
function_exists('mysql_real_escape_string') ?
mysql_real_escape_string($str) :
addslashes($str)
);
} else return '';
}
function isValidEmail($address) {
if (filter_var($address,FILTER_VALIDATE_EMAIL)==FALSE) {
return false;
}
/* explode out local and domain */
list($local,$domain)=explode('@',$address);
$localLength=strlen($local);
$domainLength=strlen($domain);
return (
/* check for proper lengths */
($localLength>0 && $localLength<65) &&
($domainLength>3 && $domainLength<256) &&
(
checkdnsrr($domain,'MX') ||
checkdnsrr($domain,'A')
)
);
}
include "include/db_login.php";// database connection details stored here
// Collect the data from post method of form submission //
echo '
<!doctype html>
<html><head>
<meta charset="UTF-8">
<title>TEST Signup FORM</title>
</head><body>';
if (
isset($_POST['todo']) &&
($_POST['todo']=="post")
) {
$msg=''; // set the error message to empty
$userid=sanitizeFromPost('userid');
if (strlen($userid)<5) {
$msg.='User ID should be 5 or more than 5 char length<br>';
}
if (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'"))) {
$msg.'Userid already assigned. Please select another userid.<br>';
}
if (empty($msg)) {
$password=sanitizeFromPost('password');
$password2=sanitizeFromPost('password2');
if (strlen($password)<8) {
$msg.='Password should be 8 or more than 8 char length<br>';
}
if ( $password <> $password2 ){
$msg."Both passwords do not match.<br>";
}
}
if (empty($msg)) {
$email=sanitizeFromPost('email');
if (!isValidEmail($email)) {
$msg."You must use a valid email address.<br>";
}
}
if (empty($msg)) {
$name_last=sanitizeFromPost('name_last');
$name_first=sanitizeFromPost('name_first');
$query=mysql_query("
INSERT INTO member_tbl
(user_id,password,email,name_last,name_first)
VALUES
('$userid','$password','$email','$name_last','$name_first')
");
echo 'You have successfully submitted new member information<br><br>
<p>Add another member? <input type="button" value="yes" onClick="history.go(-1)"></p>';
}
} else {
echo $msg.'<br><input type="button" value="Retry" onClick="history.go(-1)">';
}
echo '
</body></html>';
?>
Which looks more readable - and would have made the error easier to spot?
if (strlen($userid)<5) {
$msg.='User ID should be 5 or more than 5 char length<br>';
}
if (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'"))) {
$msg.'Userid already assigned. Please select another userid.<br>';
}
if ( strlen($userid) < 5 ) {
$msg .= 'User ID should be 5 or more than 5 char length<br>';
}
if (mysql_num_rows(mysql_query("SELECT user_id FROM member_tbl WHERE user_id = '$userid'"))) {
$msg . 'Userid already assigned. Please select another userid.<br>';
}
Be careful using that sanitize function if you ever end up using something other than mysql - it will break if you dont have a mysql connection instantiated.
