Forum Moderators: coopster

Message Too Old, No Replies

Database accept blank entry even when flagged as empty by php validation.

Going out of my mind, need help please

         

codeblock

3:17 pm on Mar 1, 2010 (gmt 0)

10+ Year Member



Hello all this is my second post here and i really hope you can help me. The problem that i am having is with my validation code. It will validate the tesxtfields correctly but any textfield after age and nofl wheather string or int will be sent into the database when flagged as empty/blank. The result is that i have a whole row with blank fields in my database that has been flag empty,blank," ". i have tried everything and can't think of anything else too try. Will be much appreciated if someone,anyone can help me.
Thanks

<?php
session_start();

require("config.php");
require("functions.php");

$db = mysql_connect($dbhost, $dbuser, $dbpassword);
mysql_select_db($dbdatabase, $db);

$age = $_POST['age'];
$nofl = $_POST['nofl'];
$adres = $_POST['adres'];
$arcd = $_POST['arcd'];
$pstcd = $_POST['pstcd'];
$borough = $_POST['borough'];
$city = $_POST['city'];
$country = $_POST['country'];
$startingprice = $_POST['startingprice'];

if(isset($_SESSION['USERNAME']) == FALSE) {
header("Location: " . $config_basedir . "/login.php?ref=newitem");
}
if($_POST['submitted'])
{

$validdate = checkdate($_POST['month'], $_POST['day'], $_POST['year']);
if($validdate == TRUE) {
$concatdate = $_POST['year']
. "-" . sprintf("%02d", $_POST['month'])
. "-" . sprintf("%02d", $_POST['day'])
. " " . $_POST['hour']
. ":" . $_POST['minute']
. ":00";

$itemsql = "INSERT INTO items(user_id,propertytype,age,grade,style,nofl,adres,arcd,pstcd,borough,city,country,startingprice,dateends)
VALUES(
".$_SESSION['USERID']
. ",'" . addslashes($_POST['propertytype'])
. "'," . $_POST['age']
. ",'" . addslashes($_POST['grade'])
. "','". addslashes( $_POST['style'])
. "'," . $_POST['nofl']
. ",'" . addslashes( $_POST['adres'])
. "','" . $_POST['arcd']
. "','" . $_POST['pstcd']
. "','" . $_POST['borough']
. "','" . $_POST['city']
. "','" . $_POST['country']
. "'," . $_POST['startingprice']
. ",'" . $concatdate
. "');";
mysql_query($itemsql);
$itemid = mysql_insert_id();

header("Location: " . $config_basedir . "/addimages.php?id=".$itemid);
}
else {
header("Location: " . $config_basedir . "/newitem.php?error=date");
}
if($_POST['age']=="")
{
header("Location: " . $config_basedir . "/newitem.php?error=age");
}
if($_POST['nofl']=="")
{
header("Location: " . $config_basedir . "/newitem.php?error=nofl");
}

if($_POST['adres']=="")
{
header("Location: " . $config_basedir . "/newitem.php?error=adres");
}

}
else {
require("header.php");
?>
<?php
switch($_GET['error']) {
case "date":
echo "<strong>Invalid date - please choose another!</strong>";
break;
case "age":
echo "<strong>Please provide age of property!</strong>";
break;
case "nofl":
echo "<strong>Please provide number of floor levels!</strong>";
break;
case "adres":
echo "<strong>Please provide address of property!</strong>";
break;

}
?>

Matthew1980

3:35 pm on Mar 1, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi there codeblock,
Are you making sure that the post array is actually poulated after pressing the submit button, ie checking to see if there is any value before you assign to vars/sql query.

Also I HIGHLY recommend $_POST santising, as you dont want your site being hacked, and as you are posting directly into sql, using mysql_real_escape_string(); around the $_POST will greatly reduce risks.

Firstly, check to see what values are being sent from the form.

Cheers,
MRb

Matthew1980

7:55 pm on Mar 1, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi there codeblock,

I have re-done the sql query for you, as there were a couple of issues with the concatenations:-


$itemsql = "INSERT INTO `items` (user_id, propertytype, age, grade, style, nofl, adres, arcd, pstcd, borough, city, country, startingprice, dateends)
VALUES('".$_SESSION['USERID']."','".addslashes($_POST['propertytype'])."','".$_POST['age']."','".addslashes($_POST['grade'])."','".addslashes( $_POST['style'])."','".$_POST['nofl']."','".addslashes($_POST['adres'])."','".$_POST['arcd']."','".$_POST['pstcd']."','".$_POST['borough']."','".$_POST['city']."',
'".$_POST['country']."','".$_POST['startingprice']."','".$concatdate."')";


Just copy & paste, that will be Ok.

Also for the mysql_query, add this to the end:-

mysql_query($itemsql)or die(mysql_error());


This will then give you any errors that may be occurring from the sql side of things, as not all errors are reported, that why when developing, it is recommended as you put error_reporting(E_ALL); at the top of your main index file, or which ever file you are working on - this will then give detail to any errors or notices that might be stopping the script from executing the way you expect.

I know I am repeating myself here but I think it worth it:-


$db = mysql_connect($dbhost, $dbuser, $dbpassword);
mysql_select_db($dbdatabase, $db);

$age = strip_tags(mysql_real_escape_string($_POST['age']));
$nofl = strip_tags(mysql_real_escape_string($_POST['nofl']));
$adres = strip_tags(mysql_real_escape_string($_POST['adres']));
$arcd = strip_tags(mysql_real_escape_string($_POST['arcd']));
$pstcd = strip_tags(mysql_real_escape_string($_POST['pstcd']));
$borough = strip_tags(mysql_real_escape_string($_POST['borough']));
$city = strip_tags(mysql_real_escape_string($_POST['city']));
$country = strip_tags(mysql_real_escape_string($_POST['country']));
$startingprice = strip_tags(mysql_real_escape_string($_POST['startingprice']));


Sanitise the $_POST array, this will then strip any unwanted html & protect your site against any hackers. Its not bullet proof, but it the next best thing.

Good luck with the project.

Cheers,
MRb

CyBerAliEn

9:54 pm on Mar 1, 2010 (gmt 0)

10+ Year Member



mysql_query($itemsql)or die(mysql_error());


This is great for development work.

But if this hits production (public), I would highly encourage you to remove/change this line of code so that mySQL does not output any specific error.

Preferably, you'd want it to error out, saying something generic like:
Sorry, but your request could not be processed at this time. Please try again later.

A visitor then knows something (error) occurred, but isn't given any technical jargon (which many public users will find confusing/cryptic) and an advanced/malicious user will not be able to use the error message from mySQL to attempt any type of hacking attempts.

sometimes the mySQL errors reveal enough info about your DB setup that a smart user can force commands or intended/desired actions through to a site, such as a fake login, getting customer/user info, etc

Matthew1980

10:00 pm on Mar 1, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi CyBerAliEn,

I never really considered that before, but then again I have never had that instance happen to me yet (touch wood!)

I usually have error_reporting(E_ALL); on throughout dev stages, but then remove once I am happy with a release. But I can see your point.

Cheers,

MRb

CyBerAliEn

10:15 pm on Mar 1, 2010 (gmt 0)

10+ Year Member



It is not common. But it can happen. It is very intertwined with SQL injection. (ie: hacker can learn the names of tables, column fields, etc)

Namely, I would remove it from production simply for a friendly user experience. 95%+ of computer users will not understand a technical error message, where as they will understand "oops, we broke, try later" (lol).

Escaping the input strings will help a lot to prevent SQL injection and manipulation, but hiding mySQL errors is also crucial part to protecting against injection.

rocknbil

10:52 pm on Mar 1, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Simul-post, oops . . . the fate of multi-tabbed browsing . . .

To clarify for the O.P., CyberAlien is not talking about error_reporting, he/she is referring to

mysql_query($itemsql) or die(mysql_error());

Which functions independently of error_reporting(). Error_reporting is for PHP errors, which is nowhere near as dangerous as the above (but can be if it reveals path info.) The mysql_error() function pipes the raw mySQL error to PHP output.

I like to do something like

mysql_query($itemsql) or error_func("Cannot process request ", mysql_error());
//

function error_func($generic,$db_err=null) {
echo "<h1>Error</h1>";
echo "<p>$generic</p>";
// THEN log the value of $db_err here
if ($db_err) {
// open log file, store error
}
}


So you get both - generic output for the user (which hopefully never happens) and a log of WTH happened. The beauty is you don't have to chase down and remove the error outputs when deploying it.

codeblock

11:55 pm on Mar 1, 2010 (gmt 0)

10+ Year Member



Thanks guys for all your help i'll get on it straight away.

I had this before i logged back in and saw all of your examples.is this any good?

function safe($value){
return mysql_real_escape_string($value);
}

$adres = safe($_POST["adres"]);

codeblock

3:53 pm on Mar 2, 2010 (gmt 0)

10+ Year Member




Hello Matthew1980,
You said "checking to see if there is any value before you assign to vars/sql query." i've hard code a string into the form value="29 codeblock street" and typed it into the form textfield on the form page also. when i check the database the value has gone through, so the post array must be poulated for that to happen. If i remove the string value="29 codeblock street" then validate the form input. it recognizes that a value is missing and flags the appropriate message, so i check the database and it shows that the database has accepted the form input and the blank textfield value.I've tried to write a function to check for string because the for seems to only validate the $age and $nifl both are both carry int values.

[size=2]
function pf_validate_string($value, $function, $redirect) {
if(isset($value) == TRUE) {
if(is_string($value) == FALSE) {
$error = 1;
}

if($error == 1) {
header("Location: " . $redirect);
}
else {
$final = $value;
}
}
else {
if($function == 'redirect') {
header("Location: " . $redirect);
}

if($function == "value") {
$final = NULL;
}
}

return $final;
}


But it doesn't help. i am totally lost now.
[/size]

CyBerAliEn

4:30 pm on Mar 2, 2010 (gmt 0)

10+ Year Member



To the OP... I'm not exactly sure what you're trying to accomplish with your function, but see if this works:

function pf_validate_string($value, $function, $redirect)
{
$final = NULL;
if($value!="")
{
if(!is_string($value))
{
header("Location: {$redirect}");
exit();
}
else
{
$final = $value;
}
}
else
{
if ($function=='redirect')
{
header("Location: {$redirect}");
exit();
}
}
return $final;
}


How it works:
1) If the 'value' is empty/null:
... a) it will check the 'function' for a value of "redirect", it it is, it will do the redirect.
... b) if 'function' isn't "redirect", the func will simply output "NULL"
2) If the 'value' is NOT empty/null:
... a) It will check if the value is of type STRING;
... ... a) if it is NOT a string, it will do the redirect.
... ... b) if it IS a string, it will cause the func to output the 'value'

That seems to be what you're trying to do?

**********************************


^ rocknbil is correct!

In production, I always force PHP errors to hide as well (via error_reporting function). Why? Visitors don't like "cryptic" errors and it does nothing useful to show the user the error.

A bit off topic... but as rocknbil notes, wrapping your own code around the errors is incredibly useful. On many of my own projects, I override PHP's natural error handling by implementing custom error handlers (try searching about it). Basically, if an error is triggered in PHP, instead of the standard PHP error, the error gets passed to this custom code... the code determines what type of error it is and handles it appropriately. Minor errors can be ignored (ie: var not defined). But fatal errors can be used to trigger a "website unavailable" to the screen for the visitor, while capturing a log on the filesystem backend of the particular session that caused the error. I prefer to dump as much info into the log as possible, because usually in my projects an error is incredibly unusual and difficult to trace. So I'll usually dump all the global var's via print_r() to the log (session, server, request, etc) as well as calling PHP's debugging "back trace" function. Then my code will trigger an email (or however needed) to notify of the error and the log. Very useful --- you get the errors to debug/trace a problem, and the user never gets jumped with error codes/etc.

Matthew1980

4:32 pm on Mar 2, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi there codeblock,

All as I meant was, doing a print_r($_POST); after the form has been submitted, this way you can tell what keys are set.

To stop null or empty values, you will need to do something like this after the $_POST has been set:-


if(!isset($_POST['name']) || !isset($_POST['age']) || !isset($_POST['town']))
{
//redirect to login page as values are empty,place an exit
//after the redirect to make sure that nothing else downstream executes
exit;
}else{
//values are set, process the form!
}


That said, if you only want to catch the empty values, you dont necessarily need the else part, as the redirect is all your after.

Also just reading back to your original post, you have a similar method of checking for empty post vars. But I also notice that those checks you have in place occur after the sql has been sent? SO you will have null values in the db.

Empty checks first, then if thats pass, check how the sql prints to screen ie echo the var, and see if it populates the way as you expect it to, then go from there.. ;-p

I hope I haven't confused you too much with that ;-p

Cheers,

MRb

codeblock

8:03 pm on Mar 3, 2010 (gmt 0)

10+ Year Member



I got it working thanks for your time anyway.

Matthew1980

8:09 pm on Mar 3, 2010 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member



Hi Codeblock,

Curiosity, what was the fix in the end then?

MRb

codeblock

10:24 am on Mar 9, 2010 (gmt 0)

10+ Year Member




I put the IFELSE conditions after the $_post['submit'] & the exit(); function i put after the header location. in the if statment.