PHP - code works in 2 files, but doesn't work in one file

Question

Finally got my code to work, sort of. I'm not sure what I can do to define variable $username/$password without the else in the if/else statement executing. When the page loads it display the message "Oops something went wrong try again!" I realize it's doing this because I was declaring $username/$password to be NULL. I took that part out of the code, but than I get the error message "Undefined variable: username" and now I'm not sure what I can do to avoid both of these errors. Does anyone have any ideas? Thanks in advance.

<html>
<body>
<?php
echo "<form action='oneFile.php' method='post'>";
echo "<table border='0'>";
echo "<tr bgcolor='#cccccc'>";
echo "<td width='150'>Username: </td>";
echo "<td width='20'><input type='text' name='username' size='15'
maxlength='15'></td>";
echo "</tr><tr><tr bgcolor='#cccccc'>";
echo "<td width='150'>Password: </td>";
echo "<td width='20'><input type='password' name='password' size='15'
maxlength='15'></td>";
echo "</tr></tr><tr><td colspan='2' align='center'>
<input type='submit' value='Login'></td>
</tr>
</table>
</form>
</body>
</html>";
?>
<?php
$usernameArr = array( 1 => "elliez",
2 => "greatGuy",
3 => "blogger",
4 => "bob",
5 => "mike",
6 => "jane",
7 => "joe",
8 => "rachel",
9 => "james",
10 => "pizzaman2000");
$passwordArr = array( 1 => "tr789ial",
2 => "abc123",
3 => "23seventeen23",
4 => "12345",
5 => "Password123",
6 => "P@ssword123",
7 => "pass123",
8 => "123pass",
9 => "123P@ssword",
10 => "54321");

$varBool = false;


if(isset($_POST['submit'])){
$username = htmlspecialchars(trim($_POST['username']));
$password = htmlspecialchars(trim($_POST['password']));
trim($username);
trim($password);
}

for ( $i = 1; $i <= 10; $i++){
if ($username == $usernameArr[$i] && $password === $passwordArr[$i]){
$varBool = true;
}
}
If ($varBool){
echo "You have successfully logged in!";
}
else{
echo "Oops something went wrong try again!";
}

?>

Glitch Desire · Answer 1 · 2017-10-25T00:30:00.507

Please do not do this in production. Hard-coding passwords is incredibly dangerous and illegal in many jurisdictions if the site contains sensitive data.

Having got that out of the way, the answer to why your code isn't working is simple. You need to put this code:

for ( $i = 1; $i <= 10; $i++){
if ($username == $usernameArr[$i] && $password === $passwordArr[$i]){
$varBool = true;
}
}
If ($varBool){
echo "You have successfully logged in!";
}
else{
echo "Oops something went wrong try again!";
}

Inside this block, at the bottom:

if(isset($_POST['submit'])){
$username = htmlspecialchars(trim($_POST['username']));
$password = htmlspecialchars(trim($_POST['password']));
trim($username);
trim($password);
}

Because you only want to do this check if the user has submitted the form. The reason it's working in 2 files is because the 2nd file is only ever being called upon submission, so username & password aren't null.

Also please look into code indentation, foreach loops & password hashing. There's a lot more wrong here (why echoing the HTML instead of just writing HTML, for example?) but these should help you improve your code readability.

You could end up with something like:

<!DOCTYPE html>
<html>
  <body>
    <form action='<?= $_SERVER['PHP_SELF']; ?>' method='POST'>
      <table border='0'>
        <tr bgcolor='#cccccc'>
          <td width='150'>Username: </td>
          <td width='20'><input type='text' name='username' size='15'
maxlength='15'></td>
        </tr>
        <tr bgcolor='#cccccc'>
          <td width='150'>Password: </td>
          <td width='20'><input type='password' name='password' size='15'
maxlength='15'></td>
        </tr>
        <tr>
          <td colspan='2' align='center'>
            <input type='submit' value='Login' name='login'>
          </td>
        </tr>
      </table>
    </form>
  </body>
</html>

<?php
// IF THIS IS A PRODUCTION SITE USE PASSWORD HASHING AND A DATABASE.
$credentials = [
  'username' => 'password',
  ...
];

if (isset($_POST['login'])) {
  $loginSuccess = false;

  foreach ($credentials as $username => $password) {
    if ($_POST['username'] == $username && $_POST['password'] = $password) {
      $loginSuccess = true;
  }

  echo $loginSuccess ? "Successfully logged in." : "Failed to log in.";
}

Which is still not perfect, but I hope you'd agree, much more readable than what you have here.

i know, i would learn end 2 end encryption before doing something like this on production. This is just for learning experience :) — Pachuca, Oct 25 '17 at 07:05
@Pachuca That's fine, I just have to include the big warning in order to (hopefully) dissuade people from copy/pasting the working code into production sites, as happens all the time on SO. Also, [hashing and encryption are different](https://stackoverflow.com/questions/4948322/fundamental-difference-between-hashing-and-encryption-algorithms) and you should use both (HTTPS to encrypt traffic, hashing to prevent recovery of passwords with encryption key). — Glitch Desire, Oct 25 '17 at 09:55

Funk Forty Niner · Accepted Answer · 2017-10-25T00:26:50.443

There are a few things wrong here.

First, the brace should encapsulate your entire code with the isset() conditional statement and your submit button isn't named so that action will never take place.

See comments in code.

HTML sticklers: You also had a stray </tr> in </tr></tr><tr><td colspan='2' align='center'>. It won't break anything, but having proper HTML markup, is a good thing.

Using <!DOCTYPE> instead of <html> is also good practice, since some browsers will throw a warning such as: Start tag seen without seeing a doctype first. Expected "<!DOCTYPE html>".

<html>
    <body>
        <?php

            echo "<form action='oneFile.php' method='post'>";
            echo "<table border='0'>";
            echo "<tr bgcolor='#cccccc'>";
            echo "<td width='150'>Username: </td>";
            echo "<td width='20'><input type='text' name='username' size='15'
            maxlength='15'></td>";
            echo "</tr><tr><tr bgcolor='#cccccc'>";
            echo "<td width='150'>Password: </td>";
            echo "<td width='20'><input type='password' name='password' size='15'
            maxlength='15'></td>";
            echo "</tr><tr><td colspan='2' align='center'>
            <input type='submit' name='submit' value='Login'></td>
            </tr>
            </table>
            </form>
            </body>
            </html>";
        ?>
        <?php
            $usernameArr = array( 1 => "elliez",
            2 => "greatGuy",
            3 => "blogger",
            4 => "bob",
            5 => "mike",
            6 => "jane",
            7 => "joe",
            8 => "rachel",
            9 => "james",
            10 => "pizzaman2000");
            $passwordArr = array( 1 => "tr789ial",
            2 => "abc123",
            3 => "23seventeen23",
            4 => "12345",
            5 => "Password123",
            6 => "P@ssword123",
            7 => "pass123",
            8 => "123pass",
            9 => "123P@ssword",
            10 => "54321");

            $varBool = false;


            if(isset($_POST['submit'])){
                $username = htmlspecialchars(trim($_POST['username']));
                $password = htmlspecialchars(trim($_POST['password']));
                trim($username);
                trim($password);
                // } // this brace is misplaced

                for ( $i = 1; $i <= 10; $i++){
                    if ($username == $usernameArr[$i] && $password === $passwordArr[$i]){
                        $varBool = true;
                    }
                }
                If ($varBool){
                    echo "You have successfully logged in!";
                }
                else{
                    echo "Oops something went wrong try again!";
                }

            } // brace moved here

        ?>

Sidenote: There's no need to use trim() twice.

appreciate the thorough analysis. Will get back to you on it. — Pachuca, Oct 25 '17 at 00:26

PHP - code works in 2 files, but doesn't work in one file

2 Answers2

Please do not do this in production. Hard-coding passwords is incredibly dangerous and illegal in many jurisdictions if the site contains sensitive data.