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 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 = ''; } 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 :-( 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()) 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. 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. 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 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*)$'); 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? 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.. 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. Link to comment https://forums.phpfreaks.com/topic/188764-please-review-my-code/#findComment-996606 Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.