Jump to content

benanamen

Members
  • Posts

    2,134
  • Joined

  • Last visited

  • Days Won

    42

Posts posted by benanamen

  1. Your example is ridiculous! The multiple try catch's you show is not needed and you know full well that is not how it is used. You only need one. As far as query errors, It will tell you exactly what the problem is and where it is.  And what part of your solution will notify you as soon as there is an error when you are not the user? The exact purpose of a try catch block is to catch the errors and handle it as you see fit. Since I am not the user of the app, I want to know as soon as it breaks and not hope a user is going to be able to contact me and tell me the details I need.

     

    The OP's example uses try/catch executing a query which is exactly where I use it. Not with this random action or that random action which is why the "Obsession" with PDO exceptions.  For a database driven application query exceptions are everything.

     

    As an include, the code is hardly cluttered in the file it is used in. Write once, use everywhere. My code is an even more controlled manner. It is not going to display error messages to the user except "Fatal Error! Admin has been notified".

  2. You need to get rid of the try statement, though. It's completely useless, it will suppress very important debug information (line number, stack trace etc.),

     

    That is completely wrong. The catch part of the "try" will give you all that information if you do it right. You cannot have the catch without the try. The code below is included in my catch. It will tell you all you want to know and more. (filename, line number, stack trace, error message)

     

    There is also additional code in the include to optionally log and or email the exception data. I get notified automatically if something breaks, whether error details are displayed to user or not.

     

    try{
    //some bad action here
    }
    catch (PDOException $e)
        {
        include_once('./config/pdo_catch_error.php');
        }

     

    pdo_catch_error.php

    $error_msg = $e->getMessage() . ' in ' . $e->getFile() . ' on line ' . $e->getLine();
    
    if (DEBUG == 1)
        {
        echo '<div class="error_custom">Error Message:<br>'.$error_msg.'</div>';
    
        echo '<div class="error_custom">SQL Statement:<br>';
        echo ''.$stmt->debugDumpParams().'';
        echo '</div>';
    
        echo '<div class="error_custom">Stack Trace:<br>';
            foreach ($e->getTrace() as $trace) {
           echo $trace['file'] . ' Line #' . $trace['line'] . '<br>';
           }
       echo '</div>';
        } //DEBUG == 1
    
    
    //Email Error - suppressed error message to user with @ if no mail server.
    if (EMAIL_ERROR == 1)
        {
        @error_log("ERROR: $error_msg\n", 1, "$email_admin", "From: $email_to");
        } //EMAIL_ERROR == 1
    
    
    // Write error to log
    if (LOG_ERROR == 1)
        {
        error_log("$mysql_datetime|$error_msg\r\n", 3, "$errorlog_path");
        } //LOG_ERROR == 1
    
    
    die('<div class="error_custom">Fatal Error! Admin has been notified</div>');
  3. You have a few server security issues. 

     

     

    Your site is vulnerable to Click Jacking.

     

    You are advertising your PHP version (PHP/5.3.29)

     

    Your PHP version is out of date. Current Stable PHP 5.6.13

     

    You are vulnerable to cross-domain Javascript inclusion (Host your JS on your server instead of linking to someone else's server.)

     

    Auto Complete is not disabled for your registration fields.

     

    Additionally, you are repeating ID's. You can only use the same ID once in a page.

    (newlife-search-form-block, searchform, s)

     

    You have many ending </p> tags with no opening <p> tag

  4. You have a few server security issues. 

     

    You are advertising your PHP version (PHP/5.4.44)

     

    Your PHP version is out of date. Current Stable PHP 5.6.13

     

    You are vulnerable to cross-domain Javascript inclusion (Host your JS on your server instead of linking to someone else's server.)

     

    Auto Complete is not disabled for your login and registration fields.

  5. You have a few server security issues. 

     

    1. Your site is vulnerable to Click Jacking.

    2. You are advertising your PHP version (PHP/5.3.29)

    3. Your PHP version is out of date. Current Stable PHP 5.6.13

    4. You allow directory browsing. http://weightroom.uk/css/ http://weightroom.uk/img/

    5. You are vulnerable to cross-domain Javascript inclusion (Host your JS on your server instead of linking to someone else's server.)

    6. Auto Complete is not disabled for your login fields.

  6. It would look something like the following:

    * There are many PDO tutorials available. Google is your friend.

            try
                {
                $sql  = "SELECT * FROM admin WHERE username=? and password=?";
                $stmt = $pdo->prepare($sql);
                $stmt->execute(array(
                    $username,
                    $password
                ));
                }
            catch (PDOException $e)
                {
                // Handle error here.
                }
    
  7. The Mysql code you are using is obsolete and insecure. It has also been removed from Php ver 7. You need to be using PDO with prepared statements or at the least Mysqli.

     

    Your connection code should only be in one place and included where you need it. As is, you need to do updates in three different places if anything changes.

     

    Your code is vulnerable to SQL injection. You are sending user supplied data directly to the database. You should not be using this code at all.

     

    There is also no need to create extra variables on your insert. Additionally, there is no need to close the connection. It is automatically closed after the script runs.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.