[PHP]Code-Review

Mikee

Active Member
Jul 8, 2017
162
102
Since devbest's community has a lot of php programmers I'm interested in getting criticized; also this subforum is lacking activity imo.
I'm Learning how to pull POST Data for a register page and send it to a db. Let me know how I did.
Note, I've never developed for the web before.
PHP:
<?php
require_once("Template/index.php");
require("Engine/DBConnect");

$Database = DBConnect::EstablishConnect();


if (isset($_REQUEST["submit"]) && $_POST['username'] != null  && $_POST['password'] == $_POST['confirm_password']){
    $Username = $_POST['username'];
    $EncryptedPassword = password_hash($_POST['password'], PASSWORD_BCRYPT);

    $NewUser = $Database->prepare("INSERT INTO users (username, password) VALUES (?, ?)");
    $NewUser->execute([$Username,$EncryptedPassword]);

}else if(!isset($_REQUEST["submit"])){
    return;
}else{
    echo "Some fields are missing values";
}

Code:
<?php

class DBConnect{
    function  __construct(){

    }

    public static function EstablishConnect(){
        $PDO = new PDO("mysql:host=localhost;dbname=first_site", "root", "password");
        return $PDO;
    }
}

?>
Code:
<HTML>
    <head>
        <title>Mike's Website</title>
    </head>
    <body>
        <form name = "form" method = "POST">
        <center>
            Username:</br>
            <input type = "text" name="username"></br>

            Password:</br>
            <input type = "text" name="password"></br>
            
            Confirm Password:</br>
            <input type = "text" name="confirm_password"></br>
            <button type = "submit" name = "submit">Register</button>
        </center>


        </form>

    </body>


</HTML>
 

Ecko

23:37 [autobots] -!- eckostylez [[email protected]]
Nov 25, 2012
1,398
962
PHP:
$Username = $_POST['username'];

Please sanitize input:
PHP:
$Username = = filter_input(INPUT_POST, 'username', FILTER_SANITIZE_STRING);

Edit: Also never be using MySQL root password when it's not needed (ie - a script like this).
 

RastaLulz

fight teh power
Staff member
May 3, 2010
3,934
3,933
  1. Instead of sanitizing the value, validate it, and return an error if it doesn't fit your constraints.
  2. The password is hashed, not encrypted; there's a difference.
  3. For the sack of my eyes, please take a look at and .
  4. I don't know how your script is entirely setup, but you can check if the form has been submitted at the top, and return instead of doing needless checks to later determine that. You can also infer later in the script that it has been posted, reducing the need for unnecessary checks.
  5. Instead of checking $_REQUEST['submit'], you should only care if it were POST'd in this context.
  6. Instead of checking if the username is not null, use ; this will prevent warnings.
I'm sure there's more I could nitpick.
 
Last edited:

Users who are viewing this thread

Top