Jump to content

Please review my code


WWW_9hub_Net

Recommended Posts

Could someone please look at the code below and leave your feedback.

 

// grab the connecting IP address. //
$connecting_ip = get_ipaddress();
if (empty($connecting_ip)) {
    echo "Could not determine IP address.";
    exit;

}

// Determine if connecting IP address is allowed to connect to timesheet


    if ($restrict_ips == "yes") {
        $size = count($allowed_networks);
for ($x = 0; $x < $size; $x++) {
            $is_allowed = ip_range($allowed_networks[$x],

$connecting_ip);
            if ($is_allowed) {
                break;
            }
        }


if (! $is_allowed) {
        echo "You are not authorized to view this page.";
        exit;

        }

        unset($x);
        unset($size);
        unset($is_allowed);
    }

function get_ipaddress() {
    if (! empty($_SERVER['REMOTE_ADDR'])) {
        $direct_ip  = $_SERVER['REMOTE_ADDR'];
    } else {
        $direct_ip  = '';
    }

    if (! empty($_SERVER['HTTP_X_FORWARDED'])) {
        $proxy_ip = $_SERVER['HTTP_X_FORWARDED'];
    } elseif (! empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
        $proxy_ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
    } elseif (! empty($_SERVER['HTTP_FORWARDED'])) {
        $proxy_ip = $_SERVER['HTTP_FORWARDED'];
    } elseif (! empty($_SERVER['HTTP_X_FORWARDED'])) {
        $proxy_ip = $_SERVER['HTTP_X_FORWARDED'];
    } else {
        $proxy_ip  = '';
    }



// Returns the true IP if it has been found, else false //
    if (! empty($proxy_ip)) {
        $is_ip = preg_match('/'.VALID_IP_ADDRESS_REGEX.'/', $proxy_ip, $regs);
        if ($is_ip and (count($regs) > 0)) {



// True IP behind a proxy
            return $regs[0];
        } else {
           

// Can't define IP: there is a proxy but we don't have information about the true IP
            return false;
        }
    }


// True IP without proxy
    return $direct_ip;
}

 

I had problem with the script when there is proxy server in place.

 

Thanks in advance

 

 

 

 

Link to comment
Share on other sites

I'd recommend looking at a Indent style and stick to it, and place your code within

PHP

tags.

 

As for your code, It looks fairly excessive regarding extracting the IP from X-FORWARDED*, but it looks functional. You should define your functions before they are used, you are calling it before it is ever defined (get_ipaddress())

Link to comment
Share on other sites

Thanks for the suggestion. I will make a note of it.

 

Is there any other simpler way to handle same situation? Extracting ip address when there is a proxy.

 

What sort of proxy are you wanting to get? Most (such as gov't/company/squid) proxy types may release an X-FORWARDED-FOR header, Nearly no web proxy in existance does that, and the *-TO headers won't be set as it isn't within it's own IP location table.

 

    $ipString=@getenv("HTTP_X_FORWARDED_FOR");

    $addr = explode(",",$ipString);

    return $addr[sizeof($addr)-1];

 

Should be sufficient ennough.

Link to comment
Share on other sites

As for your code, It looks fairly excessive regarding extracting the IP from X-FORWARDED*, but it looks functional. You should define your functions before they are used, you are calling it before it is ever defined (get_ipaddress())

 

I'm defining get_ipaddress in the functions.php and connecting_ip in header.php. You mean I have to define 'VALID_IP_ADDRESS_REGEX' ?

 

Yeah I did forget to define it in functions.php

// Input validating regular expressions

define('VALID_IP_ADDRESS_REGEX', '^((?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)(?:[.](?:25[0-5]|2[0-4]\d|1\d\d|[1-9]\d|\d)){3})|(\s*((([0-9A-Fa-f]{1,4}{7}
(([0-9A-Fa-f]{1,4})|)|(([0-9A-Fa-f]{1,4}{6}
(:|((25[0-5]|2[0-4]\d|[01]?\d{1,2})(\.(25[0-5]|2[0-4]\d|
[01]?\d{1,2})){3})|(:[0-9A-Fa-f]{1,4})))|
(([0-9A-Fa-f]{1,4}{5}(((25[0-5]|2[0-4]\d|[01]?\d{1,2})
(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|
((:[0-9A-Fa-f]{1,4}){1,2})))|(([0-9A-Fa-f]{1,4}{4}
(:[0-9A-Fa-f]{1,4}){0,1}(((25[0-5]|2[0-4]\d|[01]?\d{1,2})
(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|
((:[0-9A-Fa-f]{1,4}){1,2})))|(([0-9A-Fa-f]{1,4}{3}
(:[0-9A-Fa-f]{1,4}){0,2}(((25[0-5]|2[0-4]\d|[01]?\d{1,2})
(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|((:[0-9A-Fa-f]{1,4}){1,2})))|
(([0-9A-Fa-f]{1,4}{2}(:[0-9A-Fa-f]{1,4}){0,3}(((25[0-5]|2[0-4]\d|
[01]?\d{1,2})(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|
((:[0-9A-Fa-f]{1,4}){1,2})))|(([0-9A-Fa-f]{1,4}(:[0-9A-Fa-f]{1,4}){0,4}
(((25[0-5]|2[0-4]\d|[01]?\d{1,2})(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|
((:[0-9A-Fa-f]{1,4}){1,2})))|(:[0-9A-Fa-f]{1,4}){0,5}(((25[0-5]|2[0-4]\d|
[01]?\d{1,2})(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})?)|
((:[0-9A-Fa-f]{1,4}){1,2})))|(((25[0-5]|2[0-4]\d|[01]?\d{1,2})
(\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})){3})))(%.+)?\s*)$');

 

 

 

Link to comment
Share on other sites

I was aware of that, I just meant in the original code

$connecting_ip = get_ipaddress();

Came before:

function get_ipaddress() {

    if (! empty($_SERVER['REMOTE_ADDR'])) {

        $direct_ip  = $_SERVER['REMOTE_ADDR'];

    } else {

        $direct_ip  = '';

    }

 

Was the original code just combined or have you not spotted that mistake?

 

Link to comment
Share on other sites

Thats one of the strange things with PHP. You can refer to a function if its declared after its called..

For example.

woot('Buddski');

function woot($str) {
echo $str.' got Wooted..<br/>';
}

will return "Buddski got Wooted..";

Makes for some really confusing code..

 

Link to comment
Share on other sites

Thats one of the strange things with PHP. You can refer to a function if its declared after its called..

For example.

 

You know, I don't think I knew that, that's what you get when you come from a VB.NET background.. Assumed as variables couldn't be called before, functions couldn't either.

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • 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.