PHP Kodu temizleniyor

6 Cevap

Ben çok özensiz kodlayıcı duyuyorum ve sıradan şeyleri fark ettik.

Benim kod bir göz atın ve bana daha verimli kod bazı ipuçları verebilir misiniz? Ne geliştirmek için ne yapabilirim?

session_start();



/*check if the token is correct*/
if ($_SESSION['token'] == $_GET['custom1']){

    /*connect to db*/
    mysql_connect('localhost','x','x') or die(mysql_error());
    mysql_select_db('x');



    /*get data*/

    $orderid = mysql_real_escape_string($_GET['order_id']);
    $amount = mysql_real_escape_string($_GET['amount']);
    $product = mysql_real_escape_string($_GET['product1Name']);
    $cc = mysql_real_escape_string($_GET['Credit_Card_Number']);
    $length = strlen($cc);
    $last = 4;
    $start = $length - $last;
    $last4 = substr($cc, $start, $last);
    $ipaddress = mysql_real_escape_string($_GET['ipAddress']);
    $accountid = $_SESSION['user_id'];
    $credits = mysql_real_escape_string($_GET['custom3']);




    /*insert history into db*/
    mysql_query("INSERT into billinghistory (orderid, price, description, credits, last4, orderip, accountid) VALUES ('$orderid', '$amount', '$product', '$credits', '$last4', '$ipaddress', '$accountid')"); 
    /*add the credits to the users account*/
    mysql_query("UPDATE accounts SET credits = credits + $credits WHERE user_id = '$accountid'");

    /*redirect is successful*/
    header("location: index.php?x=1");
}else{

    /*something messed up*/
    header("location: error.php");
}

6 Cevap

Sizin kod oldukça standart görünüyor. MySQL sorguları yerine sadece sorgu ortasında değişkenleri içeren prepared statements kullanmak için muhtemelen çeşit girinti kullanmak ve gerekir. Görünümünde bir stil açıdan bakıldığında ama çoğunlukla gayet iyi

Sen bir çerçeve kullanarak içine bakmak istiyorum, ya da en azından bir nesne-ilişkisel mapper olabilir.

Eğer bir nesne yönelimli yaklaşım kullanarak eğer gerçekten SQL işlemleri çok kolay hale getirir.

Örneğin, benim bir çerçeve ile, sadece bir kayıt formu doldurduktan sonra yeni bir kullanıcı oluşturmak için kod birkaç satır alır. Tabii orada da eğlenceli bir iş ya da girişimleri önlemek kesmek için nesnenin arkasında kapsüllü kod ton, ama onunla çalışmak çok basit hale getirir.

$account = new Account();
$account->insert(array(
    'userid' => $id,
    'accountpass' => $passhash,
    'accountemail' => $emailhash
));

Ben herhangi bir iyi bilinen çerçeveler kullanılmış veya ilişkisel Mappers nesne, ama ben CodeIgniter, Kohana ve Doktrin birkaç iyi olanlar olduğunu duydum değil. Uygulama başka bir API öğrenme gerektirecek kadar büyük olup olmadığını önceden bilmeli, ancak onlar bir öğrenme eğrisi vardır.

GET aracılığıyla kredi kartı numarası ve IP adresi geçen GET dize içeren URL tarayıcı geçmişi muhafaza edilebilir, çok güvenli değildir. Nasıl atama $_GET['ipAddress']? Sen $_SERVER['REMOTE_ADDR'] yerine kullanmalısınız.

Bir kredi kartının son 4 rakamını elde etmek için, yaparak kod birkaç satır kaydedebilirsiniz:

$last4 = substr($cc,-4);

Eğer burada "temiz" demek ne bağlıdır. Her şey temel girinti derece yararlanabilir olur - oldukça fazla, daha üretken oluyor bir yerde için mevcut boşluk hareket. (Sizin satır sonları biri için, oldukça tutarsız dağıtılmış gibi görünüyor.) Girinti ve satır sonu tutarlılık büyük ölçüde kod okunabilirliği artırmak.

Eğer çiğ MySQL fonksiyonları ile çalışıyoruz verilen, orada değişkenlerin bir ton atamak ihtiyacı var neden tarzı bir sonraki düzeyde, ben görüyorum. O sistemi ile sopa planlıyorsanız, en azından birkaç diğer değişiklikleri yapılmış ve aşağıdaki notları söz değil, mantıklı bir sırayla değişkenleri atamak.

// Let's just strip all non-numerics from the card number so that we can validate it.
$cc = preg_replace('/[^0-9]/', '', $_GET['Credit_Card_Number']);
if(strlen($cc) != 12) {
    // The card number is invalid; go back to form and try again.
}
$last4 = substr($cc, -4); // A negative start value is cleaner.

$accountid = $_SESSION['user_id'];

$amount = mysql_real_escape_string($_GET['amount']);
$credits = mysql_real_escape_string($_GET['custom3']);
// note: don't trust that users won't change the ipAddress parameter before submitting
$ipaddress = mysql_real_escape_string($_GET['ipAddress']);
$orderid = (int) $_GET['order_id'];
$product = mysql_real_escape_string($_GET['product1Name']);

Ben üç blok halinde değişken atamaları kırdı. İlk veri dönüşüm için, olmayan ikinci GET kaynaklardan veri alma için olduğunu, ve üçüncü alfabetik olarak sıralanır $_GET değişkenleri, bazik atamadır. Atamaları artık çok daha yapılandırılmış, ve herhangi bir satırı bulmak kolaydır. (Bu herhangi bir şey kesiliyor standart bir yolu anlamına gelir tarafından değil, tabii ki değil, burada mantıklı olan şey.)

Bu konuda iyi kod bütün mesele bu - bu korumak için kolay olduğunu. Sizin için en uygun yap, ama ben uzun vadede, kod okunabilirliği ve organize değişken atamaları iyi çalışmak, sanıyorum.

Ayrıca, diğerleri de söylediğim gibi, siz hazır deyimleri kullanabilir ve tüm bu kaçış fonksiyonlarını önlemek için verir veritabanı işlemleri için PDO gibi bir şey, kullanmayı düşünün. Ayrıca Doctrine gibi bir ORM içine bakmak istiyorum, ya da belki tam teşekküllü framework olabilir. Daha güçlü araçları kullanarak daha yüksek bir öğrenme eğrisi vardır, ama aynı zamanda OOP kez yetkin fazla verimlilik yol açar.

Bana dışarı fırlayıp ilk şey kodunda oldukça büyük bir güvenlik açığı var olmasıdır. Doğrudan SQL sorguları içine $ _GET değişkenleri koyarak, saldırıların bir yelpazede kendinizi açık bırakın. Fatura için olsun verileri kullanıyor gibi görünen Buna ek olarak, bir olsun değişkenler ile uğraşırken kendi hesap dengesini manipüle olabilir.

Ben veritabanına damping önce gelen GET veriler üzerinde bazı veri kontrolleri öneriyoruz.

Ben kendinizi aynı eylemi tekrarlayan görmek her zaman, bir işlev oluşturmak isteyebilirsiniz olduğunu eklemek istiyorum.

Eğer anına biçimini değiştirmek istiyorsanız yarın, size örneğin mysql_real_escape_string kullandığınız tüm hatları düzenlemek zorunda kalacak.

neden böyle gibi bir fonksiyonu ekleyerek değil

function escape_data($data) {
    $escaped_data = mysql_real_escape_string($data);
    // room for more actions ...
    return ($escaped_data);
}

Bir dizi (burada $ _GET) tüm üyelerine aynı işlevi uyguluyorsanız da, size array_map() yararlı olabilir:

$_GET = array_map('escape_data', $_GET);