WWW_9hub_Net Posted January 17, 2010 Share Posted January 17, 2010 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 Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/ Share on other sites More sharing options...
redarrow Posted January 17, 2010 Share Posted January 17, 2010 is this doing it? function get_ipaddress() { if (! empty($_SERVER['REMOTE_ADDR'])) { $direct_ip = $_SERVER['REMOTE_ADDR']; } else { $direct_ip = ''; } Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996506 Share on other sites More sharing options...
WWW_9hub_Net Posted January 17, 2010 Author Share Posted January 17, 2010 I don't have access to the server until monday. So i did get a chance to test :-( Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996507 Share on other sites More sharing options...
oni-kun Posted January 17, 2010 Share Posted January 17, 2010 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()) Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996510 Share on other sites More sharing options...
WWW_9hub_Net Posted January 17, 2010 Author Share Posted January 17, 2010 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. Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996533 Share on other sites More sharing options...
oni-kun Posted January 17, 2010 Share Posted January 17, 2010 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. Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996536 Share on other sites More sharing options...
WWW_9hub_Net Posted January 17, 2010 Author Share Posted January 17, 2010 I'm trying to get through our company proxy Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996553 Share on other sites More sharing options...
WWW_9hub_Net Posted January 17, 2010 Author Share Posted January 17, 2010 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*)$'); Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996601 Share on other sites More sharing options...
oni-kun Posted January 17, 2010 Share Posted January 17, 2010 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? Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996603 Share on other sites More sharing options...
Buddski Posted January 17, 2010 Share Posted January 17, 2010 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.. Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996605 Share on other sites More sharing options...
oni-kun Posted January 17, 2010 Share Posted January 17, 2010 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. Quote Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996606 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.