2
\$\begingroup\$

I have a form on my page. I am using jQuery validation for client-side and PHP for server-side validation. I am using PDO. I don't have any issues with client-side and server-side validation. My form is working. I want to know that is this code is the best code with serverside validation with logic?

Client-side validation

<script type = "text/javascript" >
  $("#userregister").validate({
    rules: {
      user_name: {
        required: true,
        minlength: 3
      },
      user_profile_pic: {
        required: true,
        extension: "jpg,jpeg,png",
        filesize: 1000000, // <- 1 MB
      },
      user_email: {
        required: true,
        email: true,
        emailExt: true
      },
      user_mobileno: {
        required: true,
        minlength: 10,
        maxlength: 10,
        number: true,
        mobileValidation: true
      },
      user_city: {
        required: true,
        minlength: 3
      },
      terms: {
        required: true
      },
    },
    messages: {
      user_profile_pic: {
        filesize: "File size must be less than 1MB.",
        extension: "Sorry, only jpg, jpeg & png files are allowed."
      }
    },
    submitHandler: function(form) {
      // alert('hello');
      var formData = new FormData(form);
      $.ajax({
        url: "process.php",
        type: "POST",
        dataType: "json",
        data: formData,
        cache: false,
        contentType: false,
        processData: false,
        success: function(response) {
          // alert("success");

          if (response.error_no === '1') {
            $('#formerros').html(response.error);
          } else if (response.error_no === '2') {
            $('#erroremail').html(response.error);
          } else if (response.error_no === '3') {
            $('#errorphone').html(response.error);
          } else if (response.error_no === '4') {
            $('#errorfile').html(response.error);
          } else {
            window.location = "success";
          }
        },
      }); // AJAX Get Jquery statment

    }
  });

Server-side validation with logic

function userregister($pdo){
    $rv_user_name=validate_data($_POST['user_name']);
    $rv_user_email=validate_data($_POST['user_email']);
    $rv_user_mobileno=validate_data($_POST['user_mobileno']);
    $rv_user_city=validate_data($_POST['user_city']);
    
    $terms=validate_data($_POST['terms']);
    
      $user_profile_pic =" ";
      $target_dir = "assets/images/uploads/user_profile_pic/";
      $filename  = basename($_FILES['user_profile_pic']['name']);
      $extension = pathinfo($filename, PATHINFO_EXTENSION);
      $new1        = mt_rand();// random number
      $new_img_name       = $new1.'.'.$extension;
      $target_file = $target_dir . $new_img_name;
      $uploadOk = 1;
      $FileType = pathinfo($target_file,PATHINFO_EXTENSION);
      $filename = pathinfo($target_file,PATHINFO_FILENAME);
      
    
        if(($rv_user_name =="") || ($rv_user_name =="") || ($rv_user_email =="") || ($rv_user_mobileno =="") || ($rv_user_city =="") )  {
        $errorMsg[]= "All fields must be completed before you submit the form.";
        $code= "1" ;
        }
        elseif(!preg_match("/^[_\.0-9a-zA-Z-]+@([0-9a-zA-Z][0-9a-zA-Z-]+\.)+[a-zA-Z]{2,6}$/i", $rv_user_email)){
         $errorMsg[]= "You did not enter a valid email.";
          $code="2";
        }
      elseif($rv_user_mobileno == "") {
        $errorMsg[]="Please enter mobile number.";
       $code= "3" ;
      }
      elseif(is_numeric(trim($rv_user_mobileno)) == false){
        $errorMsg[]="Please enter numeric value.";
        $code= "3" ;
      }elseif(strlen($rv_user_mobileno)<10){
        $errorMsg[]="Number should be 10 digits.";
        $code= "3" ;
      }
      else if ($_FILES['reviewer_user_profile_pic']['name'] == ""){
        $errorMsg[]="Please upload profile pic.";
         $code="4";
            }
    else if($FileType != "jpg" && $FileType != "jpeg" && $FileType != "png" ) {
        $errorMsg[]="Sorry, only jpg, jpeg & png files are allowed.";
                    $code="4";
                    $uploadOk = 0;
                }
    else if ($_FILES["reviewer_user_profile_pic"]["size"] > 1000000) {
        $errorMsg[]="Sorry, your file is too large.";
                     $code="4";
                     $uploadOk = 0;
                }
        else{
                // Check if $uploadOk is set to 0 by an error
                if ($uploadOk == 0) {
                    $errorMsg[]="Sorry, your file was not uploaded.";
                    $code="4";
                // if everything is ok, try to upload file
                } else {
                    if (move_uploaded_file($_FILES["reviewer_user_profile_pic"]["tmp_name"], $target_file)) {
                       $imagename= $new_img_name;
                    } else {
                        $errorMsg[]="Sorry, there was an error uploading your file.";
                        $code="4";
                    }
                }
    
            try{
                
            $arrayname=array(
                'pd_name'=>$rv_user_name,
                'pd_pic'=>$imagename,
                'pd_email'=>$rv_user_email,
                'pd_contact'=>$rv_user_mobileno,
                'pd_city'=>$rv_user_city,
                'terms'=>$terms
                );
    
$sql="INSERT INTO `tbl_details`(`pd_name`, `pd_pic`, `pd_email`, `pd_contact`, `pd_city`, `pb_name`,`terms`)VALUES (:pd_name,:pd_pic,:pd_email,:pd_contact,:pd_city,:pb_name,:terms)";
    $stmt= $pdo->prepare($sql);
    $stmt->execute($arrayname);
    $code =5;
    $errorMsg=  "Success";
    $response['error'] = "true";
}
  catch(PDOExecption $e) { 
        $dbh->rollback(); 
        print "Error!: " . $e->getMessage() . "</br>"; 
        $response['error'] = "false";
    } 
}//else 
    echo json_encode($response);
}

function validate_data($data)
 {
  $data = trim($data);
  $data = stripslashes($data);
  $data = strip_tags($data);
  $data = htmlspecialchars($data);
  return $data;    
 }
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Welcome to Code Review. While it's great that a review enabled you to improve your code, please do not update the code in your question to incorporate feedback from answers. Doing so goes against the Question + Answer style of Code Review, as it unfortunately invalidates the existing review(s). This is not a forum where you should keep the most updated version in your question, so I rolled your changes back to the previous version. Please see see what you may and may not do after receiving answers for ways to announce your new code. \$\endgroup\$
    – Zeta
    Jun 22, 2020 at 20:06
  • \$\begingroup\$ @Zeta, Noted. Thanks for the information. \$\endgroup\$ Jun 22, 2020 at 21:21

1 Answer 1

2
\$\begingroup\$
  • Client-side validation can improve the UX and clarify the expected field values for well-behaved users, but the naughty folks that want to interact with your application will have no trouble bypassing these barriers to submission. All of the restrictions that you impose in the jquery validate() call must be duplicated in the php script if you want to truly filter submission data.

  • I don't see any reason to call validate_data() (honestly, the function should be named sanitize_data()) on $_POST['user_email'], a valid email will not allow any of the things that you are sanitizing. If you want to sanitize the other values, okay.

  • You are using a battery of if-elseif-else statements (by the way, use elseif versus else if in php -- they are technically different but not meaningfully so), so it is impossible to collection more than one error in a submission. For this reason, an array is an inappropriate data type. Just save the error message as a string.

  • Use consistent spacing on either side of the assigment operator when declaring variables. Use 4-spaces or tabs (depending on your preference) so that your script is easier to read and maintain.

  • Remove the redundant check for emptiness on $rv_user_name (specifically ($rv_user_name =="") || ($rv_user_name ==""). You also don't need to wrap any of the individual conditions in parentheses to maintain the disqualifying logic.

  • Pass a consistent set of response data back to your ajax call. It needn't be anything more than

    $response = ['target' => 'formerros', 'message' => 'All fields must be completed before you submit the form.'];
    

    or

    $response = ['target' => '', 'message' => 'Success'];
    

    This way there are no magical error_no numbers to translate and you can simply check if !response.target (which means Success), otherwise show the response.message string at the targeted field by its id.

  • Don't use regex to validate an email, the more accurate it is the more convoluted it looks. Just use filter_var() How to validate an Email in PHP?.

  • Any place where you are making a loose comparison on an empty string, you might as well just use ! for example: !$rv_user_mobileno.

  • Condense your file extension check by using in_array() with an array of whitelisted extensions.

  • Don't bother with the try-catch block once you get this into production, you are going to want those errors to go to the logs. Read this advice: https://codereview.stackexchange.com/a/243749/141885 Definitely don't pass back the generated $e->getMessage() string.

\$\endgroup\$
8
  • \$\begingroup\$ Give me some time to understand your answer. \$\endgroup\$ Jun 22, 2020 at 8:46
  • \$\begingroup\$ I'm happy to explain anything that is unclear -- that is my responsibility as a reviewer. If you don't understand something then I have failed. \$\endgroup\$ Jun 22, 2020 at 10:54
  • \$\begingroup\$ Yes, I'll update you here. Please allow me some hours. \$\endgroup\$ Jun 22, 2020 at 10:58
  • \$\begingroup\$ I appreciate your help and support. This is great information for me to improve my skills. I have some doubts. Point no 6 and Point no 10-> Do you have any link where i can check the example and learn? Point no 8->It's confused me. Some of the correcton I done in the question. \$\endgroup\$ Jun 22, 2020 at 20:13
  • \$\begingroup\$ Regarding #6, I am trying to streamline the information that php is sending back to javascript. Your javascript response handling code wants to know 1. If there was success, or if any errors then 2. What needs to be said and 3. where should the message be written on the html document. By having php pass back an array containing two pieces of data as a json encoded string, the exhaustive condition block can be avoided in the javascript. If the target value is empty, then the php script was successful, otherwise $('#' + response.target).html(response.message);. \$\endgroup\$ Jun 23, 2020 at 8:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.