Please review my code tell me if there are anything
Results 1 to 4 of 4

Thread: Please review my code tell me if there are anything

  1. #1
    Member
    Join Date
    Sep 2011
    Posts
    93

    Please review my code tell me if there are anything

    In my system there are 3 main functionalists.

    01. Add users to database
    02. Delete users from database
    03. Updater users

    This is the screenshot in my UI

    table.jpg

    When clicking on the button of add new user, jquery modal form dialog is appearing and there is a form to add user details to database.

    When clicking on the images under delete column ( see above image) , jquery conformation dialog is appearing and if conformation is true user should be deleted from the database and in the meantime particular table row which belong to deleted user should be disappear from the table with fadeout effect.

    Here I have use Jqeury and Ajax with PHP and Mysql for the project first time. So I need to make sure how is my jquery - ajax code going on and Is there and security issues? or Is there any refactoring method in my code.

    So please review my code and let me know about my code.

    NOTE: User adding and deleting functionality is working perfectly with below posted code.

    This is my jquery code :

    PHP Code:
        $(function(){            
                    
        $( 
    "input[type=submit], button" )
            .
    button()
            .
    click(function( event ) {
            
    event.preventDefault();
        });
        
            
        
    // form validation
        
    var name     = $( "#name" ),
             
    address = $( "#address" ),
             
    city     = $( "#city" ),
             
    all         = $( [] ).addname ).addaddress ).addcity ),
             
    tips     = $( ".validateTips" );    
                        
        function 
    updateTips) {
            
    tips
                
    .text)
                .
    addClass"ui-state-highlight" );
                
    setTimeout(function() {
                    
    tips.removeClass"ui-state-highlight"1500 );    
                }, 
    1500 );
                    
    tips.css("color""red");    
        }            
                    
        function 
    checkLengthonminmax ) {
            if ( 
    o.val().length max || o.val().length min ) {
                
    o.addClass"ui-state-error" );
                
    updateTips"Length of " " must be between " min " and " max "." );
                return 
    false;
            } else {
                return 
    true;
            }
        }            
        
        function 
    checkRegexporegexp) {
            if ( !( 
    regexp.testo.val() ) ) ) {
                
    o.addClass"ui-state-error" );
                
    updateTips);
                return 
    false;
            } else {
                return 
    true;
            }
        }    
        
        $(
    "#dialog").dialog({
            
    autoOpenfalse,
            
    height'auto',
            
    width350,
            
    modaltrue,
            
    buttons: {
                
    "Add New User": function() {
                    var 
    bValid true;
                    
    all.removeClass"ui-state-error" );        
        
                    
    bValid bValid && checkLengthname"Your Name"360 );
                    
    bValid bValid && checkLengthaddress"Your Address"350 );
                    
    bValid bValid && checkLengthcity"Your City"340 );    
        
                    
    bValid bValid && checkRegexpname, /^[a--']+$/i, "Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );
                    bValid = bValid && checkRegexp( address, /^[a-z -'
    ]+$/i"Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );
                    
    bValid bValid && checkRegexpcity, /^[a--']+$/i, "Your name may consist of A-Z, a-z, 0-9, underscores, begin with a letter." );
                    
                    if ( bValid) {         
                        $.ajax({
                            type: "POST", // HTTP method POST or GET
                            url: "process.php", //Where to make Ajax calls
                            //dataType:"text", // Data type, HTML, json etc.
                            dataType: '
    html',
                            data: { 
                                name: $('
    #name').val(), 
                                
    address: $('#address').val(), 
                                
    city: $('#city').val() 
                            },
                            
    beforeSend: function(){
                                $(
    '#loading').dialog({
                                    
    height57,
                                    
    width400,
                                    
    modaltrue,
                                    
    position: { my"center top+20"at"center top+20"ofwindow }, 
                                    
    resizablefalse,
                                    
    draggablefalse,                            
                                    
    dialogClass'no-close loading-dialog',
                                    
    hide: {
                                        
    effect"fade",
                                        
    duration500
                                    
    }
                                });
                            },
                            
    success:function(data){
                                $(
    '#manage_user table > tbody:last').find('tr:first').before(data);
        
                            },
                            
    error:function (xhrajaxOptionsthrownError){
                                
    //On error, we alert user
                                
    alert(thrownError);
                            }, 
                            
    complete: function(){
                                
    //alert('update success'); 
                                //$('#loading').hide(); 
                                
    setTimeout("$('#loading').dialog('close');"300);
                                $(
    '#success').dialog({
                                    
    height57,
                                    
    width400,
                                    
    modaltrue,
                                    
    position: { my"center top+20"at"center top+20"ofwindow }, 
                                    
    resizablefalse,
                                    
    draggablefalse,                            
                                    
    dialogClass'no-close success-dialog',
                                    
    hide: {
                                        
    effect"fade",
                                        
    duration1000
                                    
    }
                                });
                                
    setTimeout("$('#success').fadeOut('slow').dialog('close');"3000);
                            }
                        });
                        $(
    this).dialog("close");                    
                    } 
                },                    
                
    Cancel: function() {
                    $( 
    this ).dialog"close" );
                }
            }, 
            
    close: function() {
                
    all.val"" ).removeClass"ui-state-error" );
            }
        });
        
        $( 
    "#FormSubmit" )
            .
    button()
            .
    click(function() {
            $( 
    "#dialog" ).dialog"open" );
            });   
        
        
    //##### Send delete Ajax request to process.php #########
        
    $("body").on("click""#response table td a.del_button", function(e) {
            
    e.returnValue false;
            var 
    clickedID this.id.split('-'); //Split string (Split works as PHP explode)
            
    var DbNumberID clickedID[1]; //and get number from array
            
    var myData 'recordToDelete='DbNumberID//build a post data structure    
            //console.log(myData); 
            
            
    var $tr = $(this).closest('tr'); //here we hold a reference to the clicked tr which will be later used to delete the row
        
            
            
    $("#delete_this_user").dialog({
              
    resizablefalse,
              
    height:140,
              
    modaltrue,
              
    buttons: {
                 
    "Yes": function() {
                    
    //$row.remove();
                    
    $(this).dialog"close" );
                        
                        $.
    ajax({
                            
    type"POST"// HTTP method POST or GET
                            
    url"process.php"//Where to make Ajax calls
                            
    dataType:"text"// Data type, HTML, json etc.
                            
    data:myData//Form variables
                            
    beforeSend: function(){
                                $(
    '#loading').dialog({
                                    
    height57,
                                    
    width400,
                                    
    modaltrue,
                                    
    position: { my"center top+20"at"center top+20"ofwindow }, 
                                    
    resizablefalse,
                                    
    draggablefalse,                            
                                    
    dialogClass'no-close loading-dialog',
                                    
    hide: {
                                        
    effect"fade",
                                        
    duration500
                                    
    }
                                });
                            },
                            
    success:function(response){
                                
    //on success, hide  element user wants to delete.
                          
    $tr.find('td').fadeOut(1000,function(){ 
                             
    $tr.remove();                    
                          });
                            },
                            
    error:function (xhrajaxOptionsthrownError){
                                
    alert(thrownError);
                            },
                            
    complete: function(){
                                
    setTimeout("$('#loading').dialog('close');"300);
                                $(
    '#success').dialog({
                                    
    height57,
                                    
    width400,
                                    
    modaltrue,
                                    
    position: { my"center top+20"at"center top+20"ofwindow }, 
                                    
    resizablefalse,
                                    
    draggablefalse,                            
                                    
    dialogClass'no-close success-dialog',
                                    
    hide: {
                                        
    effect"fade",
                                        
    duration1000
                                    
    }
                                });
                                
    setTimeout("$('#success').fadeOut('slow').dialog('close');"3000);
                            }
                        });
                 },
                 
    "no": function() {
                    $(
    this).dialog"close" );
                 }
              },
                
    position: { 
                    
    my'top center',
                    
    at'top center',
                    
    of: $('#response table')
                }
           });         
                 
        
        });
                    
        }); 
    Thank you.

  2. #2
    Pna lbh ernq guvf¿
    Join Date
    Jul 2004
    Location
    Kansas City area
    Posts
    19,393
    For one, you've got a parse error in the code you posted.

    For another, it really doesn't make sense to talk about the "security" of client-side scripting at all; any and all client-side code you use can easily be 1) bypassed altogether, or 2) modified as desired before it is executed. This is why every security-related aspect (data validation, sanitization, etc.) must always be done on at least the server side of things.

  3. #3
    Member
    Join Date
    Sep 2011
    Posts
    93
    This is my PHP code for process.php page

    PHP Code:
    <?php

    //include db configuration file
    include_once("../test.php");

    if ( (isset(
    $_POST["name"]) && strlen($_POST["name"]) >= && strlen($_POST["name"]) <= 60) &&
        (isset(
    $_POST["address"]) && strlen($_POST["address"]) >= && strlen($_POST["address"]) <= 50) &&
        (isset(
    $_POST["city"]) && strlen($_POST["city"]) >= && strlen($_POST["city"]) <= 40) ) 
    {    
    //check $_POST["name"] and $_POST["address"] and $_POST["city"] are not empty

        
    $name     $_POST["name"];
        
    $address $_POST["address"];
        
    $city     $_POST["city"];
        
        
    $q "INSERT INTO users ( name, address, city) VALUES 
                ('"
    .$name."','".$address."','".$city."')";
        
    $r mysqli_query($dbc$q); 
        
        if ( 
    $r ) {
            
    // make the table row 
            
    $output  "<tr>\n";
            
    $output .= "  <td><input type='checkbox' name='' value='' class='' />&nbsp;&nbsp;$name</td>\n";
            
    $output .= "  <td>$address</td>\n";
            
    $output .= "  <td>$city</td>\n";
            
    $output .= "  <td><span class='edit_ico'></span></td>\n";
            
    $output .= "  <td><span class='delete_ico'></span></td>\n";
            
    $output .= "</tr>\n";    
              
              
    //$output  = "  <td><input type='checkbox' name='' value='' class='' />&nbsp;&nbsp;$name</td>\n";
            //$output .= "  <td>$address</td>\n";
            //$output .= "  <td>$city</td>\n";
            //$output .= "  <td><span class='edit_ico'></span></td>\n";
            //$output .= "  <td><span class='delete_ico'></span></td>\n";
            
            
    echo $output;
        
        } else {
            echo 
    'query error';
        }
    } elseif( isset(
    $_POST["recordToDelete"]) && 
                 
    strlen($_POST["recordToDelete"]) > && 
                 
    is_numeric($_POST["recordToDelete"])) {    //do we have a delete request? $_POST["recordToDelete"]

        //sanitize post value, PHP filter FILTER_SANITIZE_NUMBER_INT removes all characters except digits, plus and minus sign.
        
    $idToDelete filter_var($_POST["recordToDelete"],FILTER_SANITIZE_NUMBER_INT); 
        
        
    //try deleting record using the record ID we received from POST
        
    $q "DELETE FROM users WHERE id=$idToDelete";
        
    $r mysqli_query($dbc$q); 
        
        if(
    mysqli_affected_rows($dbc) > ) {    
            
    //If mysql delete query was unsuccessful, output error 
            
    header('HTTP/1.1 500 Could not delete record!');
            exit();
        }
        
    mysqli_close($dbc); //close db connection

    } else {             
        echo 
    "error in post array";
    }

    ?>

  4. #4
    Pna lbh ernq guvf¿
    Join Date
    Jul 2004
    Location
    Kansas City area
    Posts
    19,393
    Your code is vulnerable to SQL injection attacks (and/or just plain SQL errors); see security.database.sql-injection for more info.

    In addition, none of the validations you do in your Javascript code (e.g. the checkRegexp() calls) are replicated in the server-side code, thus you can't assume that any of them have actually been enforced.

Thread Information

Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)

Tags for this Thread

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •