Help with userclass, form checking, and header

I seem to be having some trouble with my PHP code. Was wondering if someone could help me sort them out...

One issue is that my session variables don't seem to be registering. I place a value inside a session variable to hold errors, but when the user is taken back to the index page the error isn't displayed in the error handle.

A second issue is even though I've check a session variable to not be equal to something the header fires the redirect anyway...

hmm... I've also got this section of code that searches form fields for special characters...it certainly works, but it's a bit of an over kill... I wasn't sure how to work the ereg function...it's quite confusing to me in the books...

Maybe someone can help me make a better function?

index.php
this is at the very top of the document.

<?php

    include('/system/scripts/php/systemclass.php');
    $system = new system;
    session_start();
    $_SESSION['sessionStarted'] = "yes";
    $_SESSION['registrationError'] = "";
?>

PHP:

this is used to display CSS dynamically. I just removed an instantiation of a new system, lol, let me know if that fixes one of the problems above. Not sure if that conflicted with object instantiated at the top of the document. (at library, so time is limited)

<?php
$system->cssDisplay();
?>

PHP:

This is the code used to display the error handle handed to the session variable from within my system class.

<?php echo "<p><font color=\"red\">". $_SESSION['registrationError'] . "</font></p>" ?>

PHP:

registration.php

This is the code at the top of my registration document. This is where the header that constantly fires is..

<?php

include('../system/scripts/php/systemclass.php');
$system = new system;

if($_SESSION['sessionStarted'] != "yes"){
   
    header("Location: ../index.php");
    exit;
   
}else{
$system->formFieldCheck();
}
?>

PHP:

Here's my user class. My lack of knowledge as I mentioned above left me coding functions that aren't very reusable usable in some cases...mostly the user check forum....I think I should split the function up into smaller functions to make it more reusable? Hopefully I can get some pointers

<?php

class system{
     
   public $fireFoxStylePath = "\"admin/css/ffhomestyle.css\"";
   private $webAgent;
   private $errorRegistration;
   
   function __construct()
   
   {
     $this->webAgent = getenv("HTTP_USER_AGENT");
     
   }
   
       
   function cssDisplay()
   
   {
     
     if(preg_match("/Mozilla/i", "$this->webAgent")){

       echo "<link href=\"admin/css/ffhomestyle.css\" rel=\"stylesheet\" type=\"text/css\"/>";
     }

   }
   
   
   function formFieldCheck()
   
   {
         
         
     foreach($_POST as $val){
       if($val == ""){
       
       $_SESSION['registrationError'] = "Don't forget to fill in the form feilds.";
       header("Location: ../index.php");
       exit;

       }       
     }
     
     foreach($_POST as $val){
       if($val != ""){
       
         $tempVal = $val;
       
         htmlentities($tempVal);
         strip_tags($tempVal);
         nl2br($tempVal);
         stripslashes($tempVal);
               
         $tempVal = str_replace("!", "NULL", $tempVal);
         $tempVal = str_replace("`", "NULL", $tempVal);
         $tempVal = str_replace("~", "NULL", $tempVal);
         $tempVal = str_replace("#", "NULL", $tempVal);
         $tempVal = str_replace("$", "NULL", $tempVal);
         $tempVal = str_replace("%", "NULL", $tempVal);
         $tempVal = str_replace("^", "NULL", $tempVal);
         $tempVal = str_replace("&", "NULL", $tempVal);
         $tempVal = str_replace("*", "NULL", $tempVal);
         $tempVal = str_replace("(", "NULL", $tempVal);
         $tempVal = str_replace( ")","NULL", $tempVal);
         $tempVal = str_replace("+", "NULL", $tempVal);
         $tempVal = str_replace("=", "NULL", $tempVal);
         $tempVal = str_replace("[", "NULL", $tempVal);
         $tempVal = str_replace("]", "NULL", $tempVal);
         $tempVal = str_replace("{", "NULL", $tempVal);
         $tempVal = str_replace("}", "NULL", $tempVal);
         $tempVal = str_replace(";", "NULL", $tempVal);
         $tempVal = str_replace(":", "NULL", $tempVal);       
         $tempVal = str_replace("'", "NULL", $tempVal);
         $tempVal = str_replace("\"","NULL", $tempVal);
         $tempVal = str_replace("|", "NULL", $tempVal);
         $tempVal = str_replace("<", "NULL", $tempVal);
         $tempVal = str_replace(">", "NULL", $tempVal);
         $tempVal = str_replace(",", "NULL", $tempVal);
         $tempVal = str_replace("?", "NULL", $tempVal);
         $tempVal = str_replace("/", "NULL", $tempVal);           
                 
         if($tempVal == "petNULLs nameNULL"){
         
           $tempVal = "pet's name?";
         
         }elseif($tempVal == "motherNULLs maiden nameNULL"){
         
           $tempVal = "mother's maiden name?";           
         
         }elseif($tempVal == "favorite yearNULL"){
           
          $tempVal = "favorite year?";
         }   
         
         if($_POST['password'] != $_POST['password2']){
         
           $_SESSION['registrationError'] = "Your passwords don't match.";
           header("Location: ../index.php");
           exit;

         }
         
         if(!strstr($_POST['email'],"@") && !strstr($_POST['email'],"@")){
         
           $_SESSION['registrationError'] = "The e-mail isn't valid.";
           header("Location: ../index.php");
           exit;
         
         }
         
         if($_POST['password'] == $_POST['userName']){
         
           $_SESSION['registrationError'] = "Password and Username can't match.";
           header("Location: ../index.php");
           exit;
   
         
         }
         
                   
         if($tempVal != $val){
           
           $_SESSION['registrationError'] = "Form fields can't contain speacial characters.";
           header("Location: ../index.php");
           exit;
           
         }
         elseif($val == $tempVal){
         
           $this->registerNewUser();
         }             
       
       }
     
     }
   
   }
   
   function registerNewUser()
   
   {
  
   }
   
}

?>

PHP:

 

First, I don't see you starting the session in registration.php.

But then again, if you ask me, errors don't belong in sessions to begin with. I would have the form submit the data to the same page it's on. This way you don't have to carry around unnecessary data, and you don't lose the data the user inserted into the form fields. If that's not an option, you could redirect the user to the form and append an error variable to it.

Like so:

if (/* error */)
{
    header('Location: index.php?error=username');
    exit;
}

PHP:

And then in index.php

$errors = [
    'username' => 'Username can\'t be empty',
    'password' => '...'
];

if (!empty($_GET['error']) AND in_array($_GET['error'], $errors, true))
{
    echo $errors[$_GET['error']];
}

PHP:

... or something along those lines. But that's just a dirty hack around something that can be much simpler.

I would do it like this:

<?php

$errors = [];

if ($_SERVER['REQUEST_METHOD'] === 'POST')
{
    // Form has been submitted. Validate stuff here

    if (/* error */)
    {
        $errors[] = 'Empty username';
    }

    // ...

    if (empty($errors))
    {
        // Save user
    }
}

if (!empty($errors))
{
    // Display errors here
}

?>
<form action="registration.php" method="post">
<!-- You can fill the form fields with the values from the $_POST array
    so users don't have to refill them -->
</form>

PHP:

 

Oh, and note that these lines serve no purpose, since you're not overwriting the existing variables.

         htmlentities($tempVal);
         strip_tags($tempVal);
         nl2br($tempVal);
         stripslashes($tempVal);

PHP:

 

hmm, since those functions aren't doing anything...is the rest of the function working the way I'd like it to? I'm kind of worried about that function not doing what I wanted it to, lol.

just wondering you said you didn't see me start a session in registration... I had started the session in index.php because I wanted to take the user out of registration.php and back into index.php if a session wasn't started... that's why I was storing errors in session variable, but you're right about not doing that because of the extra data..so I'll try and store the errors in any array in the system class and and display them via the array

ugh, hold on... trying to understand your code here...

<?php

$errors = [];

if ($_SERVER['REQUEST_METHOD'] === 'POST')
{
    // Form has been submitted. Validate stuff here

    if (/* error */)
    {
        $errors[] = 'Empty username';
    }

    // ...

    if (empty($errors))
    {
        // Save user
    }
}

if (!empty($errors))
{
    // Display errors here
}

?>

PHP:

That should be on the form page...just not sure what that does to my systemclass lol....or should my system class be for smaller purposes, like css and the likes?

sorry for the questions, always struggle a bit with code, and I find that I have a horrible time understanding another's code, even if it's kind of easy...it's like if I don't write it myself I don't know what's going on, lol.

 

maybe I should have mentioned that users start on index...fills out a form... go to registration.php and fill out a second form...that's why the validation of input isn't happening on the same page as the form at the moment...sorry really confused, lol

 

You need to do a session_start() on every page where you use or try to set sessions - else they won't work.

 

It's not that the function doesn't do what you want it to do, it's that you're not using it correctly. It should be:

$tempval = htmlentities($tempVal);
// ... etc

PHP:

As for the rest of your code, I don't know. I haven't tried it, but without digging too deep into it, it looks okay.

Sounds good!

Well what your class does and/or how you use it is up to you. I was merely suggesting to place the form, and the code that processes it on the same page. You can still redirect the user to the new page if everything in the first form was filled out correctly. That's how I would do it anyway.

It's not unusual, I suppose. But you'll eventually get around it.

 

Hey, thanks for all the replies. I got the code working, but have to transfer some of it back to relying on the class...especially the code that checks empty input, and special chars...also gonna break that down into ereg, or another method...just need to watch part video series on it, because the books are too dry on it....sometime someone teaching makes all the difference..

I have some new issues now that I've gotten farther, lol...always something..

I've got to the part where I want to connect to the database I have in wamp...just using phpmyadmin.

I have a sql class for making a connection, and plan to run some queries but it's trying to say that I have at least on undefined variable in my sql class, and cannot access empty property on line 18. That's the function call to connect.

You'll see I decided to take some of your advice and do same page validation, made things easier...

ahh... I have a commented out include file which has the sql password in it, and thought that's where the errors were so I added the variables in the class file until later, but found out that wasn't the case, lol.

index.php

<?php

    session_start();
    include('/system/scripts/php/systemclass.php');
    require('system/scripts/php/sqlclass.php');   
    $system = new system;
    $sqlsystem = new sqlsystem;
    $error = [];
    $formIsEmpty = false;
    $formDataSafe = false;
   

    if($_SERVER['REQUEST_METHOD'] === 'POST'){
       
        if(!IsSet($_POST['userAgreement']) || !IsSet($_POST['AUP'])){
       
            $error[] = "Must commit to agreements.";
        }

            foreach($_POST as $val){
               
                if($val == ""){
               
                    $formIsEmpty = true;
                   
           
                }           
            }
           
        if($formIsEmpty==true){
               
                $error[] = "Empty fields.";
               
        }   
       
                if($_POST['password'] != $_POST['password2']){
               
                        $error[] = "Passwords don't match. ";
                   
                    }
               
                        if(!strstr($_POST['email'],"@") && !strstr($_POST['email'],"@")){
                   
                            $error[] = "E-mail isn't valid. ";
               
                    }
               
                        if($_POST['password'] === $_POST['userName']){
                   
                            $error[] = "Password and Username match. ";
                    }
       
            foreach($_POST as $val){
                if($val != ""){
           
                    $tempVal = $val;
           
                    $tempVal = str_replace("!", "NULL", $tempVal);
                    $tempVal = str_replace("`", "NULL", $tempVal);
                    $tempVal = str_replace("~", "NULL", $tempVal);
                    $tempVal = str_replace("#", "NULL", $tempVal);
                    $tempVal = str_replace("$", "NULL", $tempVal);
                    $tempVal = str_replace("%", "NULL", $tempVal);
                    $tempVal = str_replace("^", "NULL", $tempVal);
                    $tempVal = str_replace("&", "NULL", $tempVal);
                    $tempVal = str_replace("*", "NULL", $tempVal);
                    $tempVal = str_replace("(", "NULL", $tempVal);
                    $tempVal = str_replace( ")","NULL", $tempVal);
                    $tempVal = str_replace("+", "NULL", $tempVal);
                    $tempVal = str_replace("=", "NULL", $tempVal);
                    $tempVal = str_replace("[", "NULL", $tempVal);
                    $tempVal = str_replace("]", "NULL", $tempVal);
                    $tempVal = str_replace("{", "NULL", $tempVal);
                    $tempVal = str_replace("}", "NULL", $tempVal);
                    $tempVal = str_replace(";", "NULL", $tempVal);
                    $tempVal = str_replace(":", "NULL", $tempVal);           
                    $tempVal = str_replace("'", "NULL", $tempVal);
                    $tempVal = str_replace("\"","NULL", $tempVal);
                    $tempVal = str_replace("|", "NULL", $tempVal);
                    $tempVal = str_replace("<", "NULL", $tempVal);
                    $tempVal = str_replace(">", "NULL", $tempVal);
                    $tempVal = str_replace(",", "NULL", $tempVal);
                    $tempVal = str_replace("?", "NULL", $tempVal);
                    $tempVal = str_replace("/", "NULL", $tempVal);       
               
                               
                if($tempVal == "petNULLs nameNULL"){
               
                    $tempVal = "pet's name?";
               
                }elseif($tempVal == "motherNULLs maiden nameNULL"){
               
                    $tempVal = "mother's maiden name?";                   
               
                }elseif($tempVal == "favorite yearNULL"){
                   
                    $tempVal = "favorite year?";
                }   
               
               
                if($tempVal != $val){
                       
                    $error[] = "Special characters. ";                   
               
                }elseif($tempVal == $val && IsSet($_POST['userAgreement']) && IsSet($_POST['AUP'])){
                   
                    $formDataSafe = true;
                                   
                                               
                }

            }
        }
    }       
   
            if($formDataSafe == true){
           
                $sqlsystem->sqlConnect();
                       
            }
           
   
?>

PHP:

sql.class

<?php
//Remember to change the folder location of this file when site is launched.
//Remember to make use of required file with variable. Couldn't make it
//work.
//require('/system/prime_system_data/sqlpassword.php');
class sqlsystem{
   
    private $host = "localhost";
    private $userName = "root";
    private $password = "";
    private $dbName = "famous4";
    //variable host, userName, password, and database name.
   
    function sqlConnect()
   
    {
   
        mysql_connect($this->$host, $this->$userName, $this->$password);
        mysql_select_db($this->$dbName);
       
    }

}



?>

PHP:

 

If I can make a suggestion here, go with "another method", namely preg_match(). The ereg_* extension is deprecated and will be removed in the future.

You got it almost right. It should be $this->dbName instead of $this->$dbName, etc...

On a further note, the mysql_* extension is deprecated too. Not sure where you're learning from, but it sure must be an old book/site. Take a look at PDO instead. It also comes with an object orientated interface, so you don't have to create your own wrappers around existing APIs.

One more thing. The foreach loop only checks if one of the submitted fields is empty. But nothing prevents me from removing a field before submitting the form, so I would be able to submit empty fields that you may consider "required".

EDIT:

For the past 10 years, since the release of the first PHP 5 RC, class constructors should be called __construct(), and not have the same name as the class itself. It will still work for backwards compatibility, but who knows for how much longer.

http://us3.php.net/__construct

I suggest you learn from another source, as the one you're currently using is horribly outdated.

 

Since you're learning, a few more suggestions:

strstr($_POST['email'],"@")

PHP:

... this is a very bad way of validating email addresses. It merely checks if it contains an @. This is a much better approach:

if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL))
{
    // invalid
}

PHP:

Calling str_replace() a million times in a row is slow (and not very elegant). Note that you can pass arrays to it, like so:

$replace = [')', '(', '#', '!', /* ... */];
$tempVal = str_replace($replace, 'NULL', $tempVal);

PHP:

This:

if($tempVal == "petNULLs nameNULL"){
              
                    $tempVal = "pet's name?";
              
                }elseif($tempVal == "motherNULLs maiden nameNULL"){
              
                    $tempVal = "mother's maiden name?";                 
              
                }elseif($tempVal == "favorite yearNULL"){
                  
                    $tempVal = "favorite year?";
                } 

PHP:

... is a dirty, dirty hack. If you don't want some fields to be filtered, you can do this:

$dontFilter = ['field1', 'field2', /* ... */];

foreach ($_POST AS $key => $val)
{
    // $key is the name of the form field.

    if (in_array($key, $dontFilter, true))
    {
         // Don't filter value
    }
    else
    {
         // Filter value
    }
}

PHP:

But in general, I'm not a big fan of the loop. If you don't have hundreds of fields, I suggest you validate them manually. It's less hacky, and probably more secure.

 

hey, I'm sure the books are probably a bit outdated. Your help has been amazing, and the section of the code I'm definitely learning a lot here. I'm gonna go home, and start re-doing the code to your suggestions. I'm sure I'll have a ton more questions, as PDO is a bit foreign this point, but I did find a good forum tut. Also found a tut for preg_match which cleared a lot up, so I should be able to validate each expression in a lot less code.

Thanks again