I've developed my shopping cart further. This is related to my earlier post: Securing PHP shopping cartIn addition to those functionalities, I've proceeded to make integration to Paytrail payment gateway. This post will add option to handle customer's personal data and to send that data to payment gateway. All credit card info/payments are handled by Paytrail on their servers.Merchant id/secret are Paytrail's test credentials, if anyone thinks, I'm revealing those accidentally.My questions are the following:Is my output data being visible in pure HTML considered insecure? This includes an MD5 hash, which is calculated based on shopping cart items/user input data (output.html).If this is insecure, then how should I proceed with changing my code, such that it'll be less likely to be abused?Are there any vulnerabilities on handling customer input data (addresses, names etc.)?Also, please state if your improvements/changes are purely an alternate way of doing things or are they crucial changes.This script is free to use for anyone who wishes to make their own integration with Paytrail gateway.test_inputfunction test_input($data){   $data = trim($data);   $data = htmlspecialchars($data);   return $data;}order.php<?php if ($_SERVER[REQUEST_METHOD] == POST) {$error = false;// CUSTOMER ADDRESS VARIABLES$firstname = $lastname = $address = $postnumber = $city = $country = $company = $homenumber = $worknumber = $email = ;// PAYMENT DATA VARIABLES$merchant_secret = '6pKF4jkv97zmqBJ3ZL8gUw5DfT2NMQ';$merchant_id = '13466';$order_number = '123456';$reference_number = '';$order_description = 'Testitilaus';$currency = 'EUR';$return_address = 'http://www.esimerkki.fi/success';$cancel_address = 'http://www.esimerkki.fi/cancel';$pending_address = '';$notify_address = 'http://www.esimerkki.fi/notify';$type = 'E1';$culture = 'fi_FI';$preselected_method = '';$mode = '1';$visible_methods = '';$group = '';// CART ITEMS DATA  $vat = 0;$cart_tax = 0;$cart_discount = 0;$cart_type = 1;$items_data = '';$count = 0;foreach($_SESSION['cart']['id'] as $key => $value) {        $count = count($_SESSION['cart']['id']);}## FORM VALIDATION BEGING ##// VALIDATE FIRSTNAME           if (empty($_POST['firstname'])) {$error = true;}        $firstname = test_input($_POST['firstname']);        if(!preg_match(/[a-zA-Z]/u, $firstname)) {$error = true;}// VALIDATE LASTNAME        if (empty($_POST['lastname'])) {$error = true;}        $lastname = test_input($_POST['lastname']);        if(!preg_match(/[a-zA-Z]/u, $lastname)) {$error = true;}// VALIDATE ADDRESS        if (empty($_POST['address'])) {$error = true;}        $address = test_input($_POST['address']);        if(!preg_match(/[a-zA-Z]/u, $address)) {$error = true;}// VALIDATE POSTNUMBER        $postnumber = filter_input(INPUT_POST, 'postnumber', FILTER_SANITIZE_NUMBER_INT);        if (empty($_POST['postnumber'])) { $error = true; }// VALIDATE CITY        if (empty($_POST['city'])) {$error = true;}        $city = test_input($_POST['city']);        if(!preg_match(/[a-zA-Z]/u, $city)) {$error = true;}// VALIDATE COUNTRY        if (empty($_POST['country'])) {$error = true;}        $country = test_input($_POST['country']);        if(!preg_match(/[a-zA-Z]/u, $country)) {$error = true;}// VALIDATE COMPANY (OPTIONAL)        if(isset($_POST['company']) && !empty($_POST['company'])) {            if(empty($_POST['company'])) {$error = true;}            $company = test_input($_POST['company']);            if(!preg_match(/[a-zA-Z]/u, $company)) {$error = true;}        }// VALIDATE HOMENUMBER (OPTIONAL)        if(isset($_POST['homenumber']) && !empty($_POST['homenumber'])) {            if (empty($_POST['homenumber'])) {$error = true;}            $homenumber = test_input($_POST['homenumber']);            if(!preg_match(/^[\+0-9\-\(\)\s]*$/, $homenumber)) {$error = true;}        }// VALIDATE WORKNUMBER (OPTIONAL)        if(isset($_POST['worknumber']) && !empty($_POST['worknumber'])) {            if (empty($_POST['worknumber'])) {$error = true;}            $worknumber = test_input($_POST['worknumber']);            if(!preg_match(/^[\+0-9\-\(\)\s]*$/, $worknumber)) {$error = true;}        }// VALIDATE EMAIL            if (empty($_POST[email])) {$error = true;}            $email = test_input($_POST[email]);            if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {$error = true;}    }if(!empty($error)) {echo Something fishy is going on..; exit;}## FORM VALIDATION ENDS ##//CREATE STRINGS FROM CUSTOMER AND PAYMENT DATA$customer = $homenumber. '|' .$worknumber. '|' .$email. '|' .$firstname.   '|' .$lastname. '|' .$company. '|' .$address. '|' .$postnumber. '|' .$city. '|' .$country. '|' .$vat. '|' .$count. '|';$payment_data = $merchant_secret. '|' .$merchant_id. '|' .$order_number. '|' .$reference_number. '|' .$order_description. '|' .$currency. '|' .$return_address. '|' .$cancel_address. '|' .$pending_address. '|' .$notify_address. '|' .$type. '|' .$culture. '|' .$preselected_method. '|' .$mode. '|' .$visible_methods. '|' .$group. '|' ;   // PAYMENT DATAecho <form action=\https://payment.paytrail.com/\ method=\post\ id=\payment\>;echo <input name=\MERCHANT_ID\ type=\hidden\ value=\{$merchant_id}\>;echo <input name=\ORDER_NUMBER\ type=\hidden\ value=\{$order_number}\>;echo <input name=\REFERENCE_NUMBER\ type=\hidden\ value=\{$reference_number}\>;echo <input name=\ORDER_DESCRIPTION\ type=\hidden\ value=\{$order_description}\>;echo <input name=\CURRENCY\ type=\hidden\ value=\{$currency}\>;echo <input name=\RETURN_ADDRESS\ type=\hidden\ value=\{$return_address}\>;echo <input name=\CANCEL_ADDRESS\ type=\hidden\ value=\{$cancel_address}\>;echo <input name=\PENDING_ADDRESS\ type=\hidden\ value=\{$pending_address}\>;echo <input name=\NOTIFY_ADDRESS\ type=\hidden\ value=\{$notify_address}\>;echo <input name=\TYPE\ type=\hidden\ value=\{$type}\>;echo <input name=\CULTURE\ type=\hidden\ value=\{$culture}\>;echo <input name=\PRESELECTED_METHOD\ type=\hidden\ value=\{$preselected_method}\>;echo <input name=\MODE\ type=\hidden\ value=\{$mode}\>;echo <input name=\VISIBLE_METHODS\ type=\hidden\ value=\{$visible_methods}\>;echo <input name=\GROUP\ type=\hidden\ value=\{$group}\>;// CUSTOMER ADDRESSecho <input name=\CONTACT_TELNO\ type=\hidden\ value=\{$homenumber}\>;echo <input name=\CONTACT_CELLNO\ type=\hidden\ value=\{$worknumber}\>;echo <input name=\CONTACT_EMAIL\ type=\hidden\ value=\{$email}\>;echo <input name=\CONTACT_FIRSTNAME\ type=\hidden\ value=\{$firstname}\>;echo <input name=\CONTACT_LASTNAME\ type=\hidden\ value=\{$lastname}\>;echo <input name=\CONTACT_COMPANY\ type=\hidden\ value=\{$company}\>;echo <input name=\CONTACT_ADDR_STREET\ type=\hidden\ value=\{$address}\>;echo <input name=\CONTACT_ADDR_ZIP\ type=\hidden\ value=\{$postnumber}\>;echo <input name=\CONTACT_ADDR_CITY\ type=\hidden\ value=\{$city}\>;echo <input name=\CONTACT_ADDR_COUNTRY\ type=\hidden\ value=\{$country}\>;// CART ITEMS DATAecho <input name=\INCLUDE_VAT\ type=\hidden\ value=\{$vat}\ />;echo <input name=\ITEMS\ type=\hidden\ value=\{$count}\>;$i = -1;foreach ($_SESSION['cart']['id'] as $key => $value) {    $i++;    $cart_title = $_SESSION['cart']['name'][$key];    $cart_id = $i;    $cart_quantity = $_SESSION['cart']['quantity'][$key];    $cart_price = $_SESSION['cart']['price'][$key];    $cart_pricedot = str_replace(',' , '.' , $cart_price);    $cart_pricedot_trim = trim($cart_pricedot);    echo <input name=\ITEM_TITLE[$i]\ type=\hidden\ value=\{$cart_title}\>;             echo <input name=\ITEM_NO[$i]\ type=\hidden\ value=\{$i}\>;     echo <input name=\ITEM_AMOUNT[$i]\ type=\hidden\ value=\{$cart_quantity}\>;     echo <input name=\ITEM_PRICE[$i]\ type=\hidden\ value=\{$cart_pricedot_trim}\>;     echo <input name=\ITEM_TAX[$i]\ type=\hidden\ value=\{$cart_tax}\>;     echo <input name=\ITEM_DISCOUNT[$i]\ type=\hidden\ value=\{$cart_discount}\>;       echo <input name=\ITEM_TYPE[$i]\ type=\hidden\ value=\{$cart_type}\>;    $items_data.=$cart_title|$i|$cart_quantity|$cart_pricedot_trim|$cart_tax|$cart_discount|$cart_type|;}// START CALCULATING PAYMENT MD5 HASH$combined = $payment_data. '' .$customer. '' .$items_data;$combinedsub = substr($combined, 0, -1);$code = strtoupper(md5($combinedsub));$trimmeddata = trim($code);echo <input name=\AUTHCODE\ type=\hidden\ value=\{$trimmeddata}\>;echo <input type=\submit\ value=\Siirry maksamaan\>;echo </form>;?>output.html    <form action=https://payment.paytrail.com/ method=post id=payment>    <input name=MERCHANT_ID type=hidden value=13466>    <input name=ORDER_NUMBER type=hidden value=123456>    <input name=REFERENCE_NUMBER type=hidden value=>    <input name=ORDER_DESCRIPTION type=hidden value=Testitilaus>    <input name=CURRENCY type=hidden value=EUR>    <input name=RETURN_ADDRESS type=hidden value=http://www.esimerkki.fi/success>    <input name=CANCEL_ADDRESS type=hidden value=http://www.esimerkki.fi/cancel>    <input name=PENDING_ADDRESS type=hidden value=>    <input name=NOTIFY_ADDRESS type=hidden value=http://www.esimerkki.fi/notify>    <input name=TYPE type=hidden value=E1>    <input name=CULTURE type=hidden value=fi_FI>    <input name=PRESELECTED_METHOD type=hidden value=>    <input name=MODE type=hidden value=1>    <input name=VISIBLE_METHODS type=hidden value=>    <input name=GROUP type=hidden value=>    <input name=CONTACT_TELNO type=hidden value=+5747 5884 7574543>    <input name=CONTACT_CELLNO type=hidden value=0060 55574645>    <input name=CONTACT_EMAIL type=hidden value=esupridf@oooooo.com>    <input name=CONTACT_FIRSTNAME type=hidden value=zil>    <input name=CONTACT_LASTNAME type=hidden value=lgebr>    <input name=CONTACT_COMPANY type=hidden value=Company Ot>    <input name=CONTACT_ADDR_STREET type=hidden value=Krkel 34>    <input name=CONTACT_ADDR_ZIP type=hidden value=00000>    <input name=CONTACT_ADDR_CITY type=hidden value=lbm>    <input name=CONTACT_ADDR_COUNTRY type=hidden value=FI>    <input name=INCLUDE_VAT type=hidden value=0 />    <input name=ITEMS type=hidden value=2>    <input name=ITEM_TITLE[0] type=hidden value=Lasikengt>    <input name=ITEM_NO[0] type=hidden value=0>    <input name=ITEM_AMOUNT[0] type=hidden value=45>    <input name=ITEM_PRICE[0] type=hidden value=23.43>    <input name=ITEM_TAX[0] type=hidden value=0>    <input name=ITEM_DISCOUNT[0] type=hidden value=0>    <input name=ITEM_TYPE[0] type=hidden value=1>    <input name=ITEM_TITLE[1] type=hidden value=Nahkakengt>    <input name=ITEM_NO[1] type=hidden value=1>    <input name=ITEM_AMOUNT[1] type=hidden value=23>    <input name=ITEM_PRICE[1] type=hidden value=564.44>    <input name=ITEM_TAX[1] type=hidden value=0>    <input name=ITEM_DISCOUNT[1] type=hidden value=0>    <input name=ITEM_TYPE[1] type=hidden value=1>    <input name=AUTHCODE type=hidden value=958C104FA7522E0319214C3AE1147351>    <input type=submit value=Siirry maksamaan></form>
Securing PHP shopping cart (Paytrail integration) - follow-up
php;html;security;e commerce
As one of your previous answer-ers, I can say this has definitely improved!I'll work through by the type this time.Variables:Your beginning (block) variables are defined like $firstname, but the secondary block variables are defined like $merchant_secret, with an underline  instead of a no-space. I, personally, would suggest using underlines for readability, but, if you do choose the first, keep the coding style the same.It seems like the rest of your script fluctuates too between underline(s) and no space(s), so please do keep the coding style the same.Below, you initialise $count as zero, and proceed to go through every item in the cart, overriding $count with the same value everytime.$count = 0;foreach($_SESSION['cart']['id'] as $key => $value) {        $count = count($_SESSION['cart']['id']);}Just use: $count = count($_SESSION['cart']['id']).This line here:if(!empty($error)) {echo Something fishy is going on..; exit;}While empty() works, it's pointless, because they're booleans. So, you can remove empty() and it'll work the exact same.You then proceed to 'CREATE STRINGS [sic] FROM CUSTOMER AND PAYMENT DATA', and just like the fact I won't comment on why sentences should not become SQL or your usage of plurality, I won't comment on why you concatenate a bunch of strings to make a secure MD5 hash. (It'd certainly be a question I'd vote up on Security.SE, though!)My beef is (again) with your coding style:$customer = $homenumber. '|' .$worknumber. '|' .$email. '|' .$firstname.   '|' .$lastname. '|' .$company. '|' .$address. '|' .$postnumber. '|' .$city. '|' .$country. '|' .$vat. '|' .$count. '|';$payment_data = $merchant_secret. '|' .$merchant_id. '|' .$order_number. '|' .$reference_number. '|' .$order_description. '|' .$currency. '|' .$return_address. '|' .$cancel_address. '|' .$pending_address. '|' .$notify_address. '|' .$type. '|' .$culture. '|' .$preselected_method. '|' .$mode. '|' .$visible_methods. '|' .$group. '|' ;   compared to here:$items_data.=$cart_title|$i|$cart_quantity|$cart_pricedot_trim|$cart_tax|$cart_discount|$cart_type|;If you choose to join each string with a . instead of just placing them in a string like $items_data, then do so, but please (again) keep your coding style the same.Your foreach loop, has similar to a for loop style coding attached.However, normally in a for loop, you increment the value once you've executed the code inside the loop, not before.Instead of:$i = -1;foreach ($array as $key => value){    $i++;    some_code_here();}Change it to:$i = 0;foreach ($array as $key => $value){    some_code_here()    $i++;}Moving on, you initialise $cart_id as $i, but then never use it, and if you were, it'd be best to replace it with $i seeing as they're the same. So, that line can be removed.These lines are clutter:$cart_price = $_SESSION['cart']['price'][$key];$cart_pricedot = str_replace(',' , '.' , $cart_price);$cart_pricedot_trim = trim($cart_pricedot);You can just use:$cart_price = trim(str_replace(',' , '.' ,$_SESSION['cart']['price'][$key]));or$cart_price = $_SESSION['cart']['price'][$key];$cart_price_formatted = trim(str_replace(',', '.', $cart_price));Only the last is used later anyway, so, if you like, you can replace them.Finally (on variables):$combined = $payment_data. '' .$customer. '' .$items_data;$combinedsub = substr($combined, 0, -1);$code = strtoupper(md5($combinedsub));$trimmeddata = trim($code);In $combined, you add strings together with empty strings in between. That can get removed.$combinedsub could be better named to $combined_substring, Nbdy lks abbrevs, yh?In $code, whilst being misleading, you perform strtoupper() and md5 transforms on it. You then proceed to trim() it.First, trim()ing after you've MD5 hashed it, literally reduces the chances of having characters trimmed to zero. So, you can put the trim() in front of the md5() instead.Secondly, if you're performing two transforms in one line, adding a third and condensing is nothing.  While $combinedsub has a misleading name, $trimmeddata has a worse name. While having incorrect grammar and spelling, you manage to double both m & d, making the variable name look even less readable.String Building:Here's the most helpful part.During the course of execution, you echo out quite a few hidden inputs.Replacing that with a function would greatly be of assistance.function build_hidden_inputs($type, $value){  return <input name=\$type\ type=\hidden\ value=\$value\>;}Then, build arrays with the content as keys and values, like so:$first_content = array(                     MERCHANT_ID => $merchant_id,                     ORDER_NUMBER => $order_number,                     // So on                     );Then, foreach them into the function.foreach ($first_content as $key => $value){    echo build_hidden_inputs($key, $value);}To answer your questions:Is it somehow unsecure, that my output data will be visible in pure html? including md5 hash, which is calculated based on shopping cart items/user input data. (output.html)The visible data will be the data visible in your output.html file, if you don't want to output it, and it's not necessary, then just remove it from the echo statements.As for md5, you are outputting it, but, cracking hashes works by matching hashes of strings.Say, for example, if we both md5 hashed 'mypasswordissecure', we'd recieve 'efaafd259c656221c88b22471ac0d61e'.However, even if I had your hash, I would need to match it, and with a data input as large as yours, would probably1 be pretty hard.If 1. is unsecure, how should i proceed changing my code, that it'll be less likely to be abused?Use a more secure encryption than MD5, add a salt coming in from the Database.1Are there any vulnerabilities on handling customer input data? (addresses, names etc.)I wouldn't go outputting the tax, price, and the more sensitive information into a HTML field, not necessarily for hackers, but for cheeky users trying to buy things for free.The AUTHCODE though, probably shouldn't be outputted because of both.You can always use session variables instead.Also, please state, if your improvements/changes are purely an alternate way of doing things or are they crucial changes.Well, if you remove the foreach loop at the beginning that keeps doing the same thing, you'll get faster execution times.However, these suggestions are closer to improvements than alternate ways of proceeding.None of these are crucial as such.1: I'm not an expert on security, unfortunately, so if you're worried, take it to Security.SE, and see what they think.