issue with some data not added to db

[ianhaney]

Hi

 

I am having some issues with the following script, it was working last night and adding all the data to the payments table but now it is not adding name and email and the memberID is being added is 0

 

I don't get how it can stop working

<?php

session_start();

ini_set('display_startup_errors',1);
ini_set('display_errors',1);
error_reporting(-1);

$title = "PayPal Success - Security Site";

include ( 'includes/header.php' );

?>

<?php

include 'db-connect.php';

//Store transaction information from PayPal
$item_number = $_GET['item_number'];
$txn_id = $_GET['tx'];
$payment_gross = $_GET['amt'];
$currency_code = $_GET['cc'];
$payment_status = $_GET['st'];

//Get product price
$productResult = $db->query("SELECT price FROM products WHERE id = ".$item_number);

$productRow = $productResult->fetch_assoc();
$productPrice = $productRow['price'];

$id = $_SESSION["memberID"]; // store the user id into session
$name = $_SESSION["name"]; // store the user id into session
$email = $_SESSION["email"]; // store the user id into session

if(!empty($txn_id) && $payment_gross == $productPrice){
    //Insert tansaction data into the database
    $insert = $db->query("INSERT INTO payments(memberID,name,email,item_number,txn_id,payment_gross,currency_code,payment_status) VALUES('".$id."','".$name."','".$email."','".$item_number."','".$txn_id."','".$payment_gross."','".$currency_code."','".$payment_status."')");
    $last_insert_id = $db->insert_id;


echo "<h1>Your payment has been successful.</h1>";
echo "<h1>Your Payment ID - <?php echo $last_insert_id; ?>.</h1>";

// get the records from the database
if ($insert = $db->query("SELECT employers.memberID, employers.name, employers.email, payments.payment_id, payments.payment_status FROM employers 
	INNER JOIN payments ON employers.memberID = payments.memberID WHERE employers.memberID='$id'"));

if ($payment_status == Completed) {
header("Location: http://www.broadwaymediadesigns.co.uk/sites/security-site/payg-job-advert.php");
}else{
header("Location: http://www.broadwaymediadesigns.co.uk/sites/security-site/employer-profile.php");
}
}
?>

<?php include( 'includes/footer.php' ); ?>

[Barand]

I'd start by checking that your SESSION variables are storing what you expect them to be storing.

[ianhaney]

I checked the employers db table that stores the SESSION variables and they are correct and stored correctly

[Barand]

I'd start by checking that your SESSION variables are storing what you expect them to be storing.

[ianhaney]

They do seem to be storing the correct info in the session or do you mean they could be getting confused with something else and not storing the info in the session so is unable to save the data to the payments db table

[ianhaney]

Think I sorted it, it seems to be storing the info again

 

I was thinking I wonder if is cause I closed the php tags and opened it again after the session start code, seems to be ok now touch wood, I now have the following and seems to be working, I also changed $id to $memberid just in case it was that was getting confused with another one called id somewhere

<?php

session_start();

$title = "PayPal Success - Security Site";

include ( 'includes/header.php' );

include 'db-connect.php';

$memberid = $_SESSION["memberID"]; // store the user id into session
$name = $_SESSION["name"]; // store the user id into session
$email = $_SESSION["email"]; // store the user id into session

//Store transaction information from PayPal
$item_number = $_GET['item_number'];
$txn_id = $_GET['tx'];
$payment_gross = $_GET['amt'];
$currency_code = $_GET['cc'];
$payment_status = $_GET['st'];

//Get product price
$productResult = $db->query("SELECT price FROM products WHERE id = ".$item_number);

$productRow = $productResult->fetch_assoc();
$productPrice = $productRow['price'];

if(!empty($txn_id) && $payment_gross == $productPrice){
    //Insert tansaction data into the database
    $insert = $db->query("INSERT INTO payments(memberID,name,email,item_number,txn_id,payment_gross,currency_code,payment_status) VALUES('".$memberid."','".$name."','".$email."','".$item_number."','".$txn_id."','".$payment_gross."','".$currency_code."','".$payment_status."')");
    $last_insert_id = $db->insert_id;


echo "<h1>Your payment has been successful.</h1>";
echo "<h1>Your Payment ID - <?php echo $last_insert_id; ?>.</h1>";
}
// get the records from the database
if ($insert = $db->query("SELECT employers.memberID, employers.name, employers.email, payments.payment_id, payments.payment_status FROM employers 
	INNER JOIN payments ON employers.memberID = payments.memberID WHERE employers.memberID='$id'"));

if ($payment_status == Completed) {
header("Location: http://www.broadwaymediadesigns.co.uk/sites/security-site/payg-job-advert.php");
}else{
header("Location: http://www.broadwaymediadesigns.co.uk/sites/security-site/employer-profile.php");
}
?>

<?php include( 'includes/footer.php' ); ?>

Edited January 5, 2016 by ianhaney

[ginerjm]

I hate seeing repeated closing/opening php tags in a script. Demonstrates a lack of understanding and poor script structure IMHO. Also a lot of single /double quotes in the query string that can be simply avoided.

 

VALUES('$id','$name','$email','$item_number','$txn_id','$payment_gross','$currency_code','$payment_status')");

works just as well as the messy string you used. Also - for those values that are numbers you don't even need quotes.

Of course a better approach would be to use a prepared statement and make your query more secure.

 

As for your code in general:

 

1 - why expose all your data to the user via a GET method instead of POST one?

2 - WHY ARE YOU NOT SANITIZING your inputs - especially after using the GET? Tsk, tsk, tsk.

[ianhaney]

Regarding the first statement, is it as simple as changing $_GET to $_POST and changing the form method to post instead of get

 

Regarding statement 2, is the following good enough as sanitization

$item_number = filter_var($_GET['item_number'], FILTER_SANITIZE_ITEM_NUMBER);
$txn_id = filter_var($_GET['tx'], FILTER_SANITIZE_TX);
$payment_gross = filter_var($_GET['amt'], FILTER_SANITIZE_AMT);
$currency_code = filter_var($_GET['cc'], FILTER_SANITIZE_CC);
$payment_status = filter_var($_GET['st'], FILTER_SANITIZE_ST);

I also changed the values to be single quotes instead of the mess it was and does look neater with less opening and closing of php tags as is no need for them to be opening closing many times

[ianhaney]

Ignore that post it is wrong

[ianhaney]

Think this is ok, it works as is still adding the data to the database still as I tried other ways but was not adding the data to the db table but this way is, is it suitable enough for sanitizing

$item_number = $db->real_escape_string($_GET['item_number']);
$txn_id = $db->real_escape_string($_GET['tx']);
$payment_gross = $db->real_escape_string($_GET['amt']);
$currency_code = $db->real_escape_string($_GET['cc']);
$payment_status = $db->real_escape_string($_GET['st']);

[ginerjm]

You're still using GET instead of POST?

[ianhaney]

Yeah only for the moment cause when I used $_POST it was not adding no data to the db so will take a look tonight and see if I can work it out

[ginerjm]

Using POST or not is not the problem. The problem is you have to review your code when you switch from one thing to another and apparently you probably didn't. Don't blame it on POST - blame it on the programmer.