11
\$\begingroup\$

I am using PHP for detecting a user's IP address and displaying their details, and I use OOP (i.e. object-oriented programming).

Are there any problems in it or is it well-written?

index.php:

<?php 
require('UserInfo.php');
?>

<!DOCTYPE html>
<html>
<head>
    <title>UserInfo Demo</title>
    <style>
table {
    margin-top: 20px;
    font-family: arial, sans-serif;
    border-collapse: collapse;
    width: 100%;
}

td, th {
    border: 1px solid #dddddd;
    text-align: center;
    padding: 8px;
}

tr:nth-child(even) {
    background-color: #dddddd;
}
h2{font-family: sans-serif,'Helvetica';}
</style>

</head>
<body>
<center><h2>UserInfo demo</h2></center>


    <table>
        <tr>
            <th>Ip</th>
            <th>Device</th>
            <th>Os</th>
            <th>Browser</th>
        </tr>
        <tr>
            <td><?= UserInfo::get_ip();?></td>
            <td><?= UserInfo::get_device();?></td>
            <td><?= UserInfo::get_os();?></td>
            <td><?= UserInfo::get_browser();?></td>
        </tr>
    </table>

</body>
</html>

UserInfo.php

<?php
class UserInfo{



    private static function get_user_agent() {
        return  $_SERVER['HTTP_USER_AGENT'];
    }

    public static function get_ip() {
        $mainIp = '';
        if (getenv('HTTP_CLIENT_IP'))
            $mainIp = getenv('HTTP_CLIENT_IP');
        else if(getenv('HTTP_X_FORWARDED_FOR'))
            $mainIp = getenv('HTTP_X_FORWARDED_FOR');
        else if(getenv('HTTP_X_FORWARDED'))
            $mainIp = getenv('HTTP_X_FORWARDED');
        else if(getenv('HTTP_FORWARDED_FOR'))
            $mainIp = getenv('HTTP_FORWARDED_FOR');
        else if(getenv('HTTP_FORWARDED'))
            $mainIp = getenv('HTTP_FORWARDED');
        else if(getenv('REMOTE_ADDR'))
            $mainIp = getenv('REMOTE_ADDR');
        else
            $mainIp = 'UNKNOWN';
        return $mainIp;
    }

    public static function get_os() {

        $user_agent = self::get_user_agent();
        $os_platform    =   "Unknown OS Platform";
        $os_array       =   array(
            '/windows nt 10/i'      =>  'Windows 10',
            '/windows nt 6.3/i'     =>  'Windows 8.1',
            '/windows nt 6.2/i'     =>  'Windows 8',
            '/windows nt 6.1/i'     =>  'Windows 7',
            '/windows nt 6.0/i'     =>  'Windows Vista',
            '/windows nt 5.2/i'     =>  'Windows Server 2003/XP x64',
            '/windows nt 5.1/i'     =>  'Windows XP',
            '/windows xp/i'         =>  'Windows XP',
            '/windows nt 5.0/i'     =>  'Windows 2000',
            '/windows me/i'         =>  'Windows ME',
            '/win98/i'              =>  'Windows 98',
            '/win95/i'              =>  'Windows 95',
            '/win16/i'              =>  'Windows 3.11',
            '/macintosh|mac os x/i' =>  'Mac OS X',
            '/mac_powerpc/i'        =>  'Mac OS 9',
            '/linux/i'              =>  'Linux',
            '/ubuntu/i'             =>  'Ubuntu',
            '/iphone/i'             =>  'iPhone',
            '/ipod/i'               =>  'iPod',
            '/ipad/i'               =>  'iPad',
            '/android/i'            =>  'Android',
            '/blackberry/i'         =>  'BlackBerry',
            '/webos/i'              =>  'Mobile'
        );

        foreach ($os_array as $regex => $value) {
            if (preg_match($regex, $user_agent)) {
                $os_platform    =   $value;
            }
        }   
        return $os_platform;
    }

    public static function  get_browser() {

        $user_agent= self::get_user_agent();

        $browser        =   "Unknown Browser";

        $browser_array  =   array(
            '/msie/i'       =>  'Internet Explorer',
            '/Trident/i'    =>  'Internet Explorer',
            '/firefox/i'    =>  'Firefox',
            '/safari/i'     =>  'Safari',
            '/chrome/i'     =>  'Chrome',
            '/edge/i'       =>  'Edge',
            '/opera/i'      =>  'Opera',
            '/netscape/i'   =>  'Netscape',
            '/maxthon/i'    =>  'Maxthon',
            '/konqueror/i'  =>  'Konqueror',
            '/ubrowser/i'   =>  'UC Browser',
            '/mobile/i'     =>  'Handheld Browser'
        );

        foreach ($browser_array as $regex => $value) {

            if (preg_match($regex, $user_agent)) {
                $browser    =   $value;
            }

        }

        return $browser;

    }

    public static function  get_device(){

        $tablet_browser = 0;
        $mobile_browser = 0;

        if (preg_match('/(tablet|ipad|playbook)|(android(?!.*(mobi|opera mini)))/i', strtolower($_SERVER['HTTP_USER_AGENT']))) {
            $tablet_browser++;
        }

        if (preg_match('/(up.browser|up.link|mmp|symbian|smartphone|midp|wap|phone|android|iemobile)/i', strtolower($_SERVER['HTTP_USER_AGENT']))) {
            $mobile_browser++;
        }

        if ((strpos(strtolower($_SERVER['HTTP_ACCEPT']),'application/vnd.wap.xhtml+xml') > 0) or ((isset($_SERVER['HTTP_X_WAP_PROFILE']) or isset($_SERVER['HTTP_PROFILE'])))) {
            $mobile_browser++;
        }

        $mobile_ua = strtolower(substr(self::get_user_agent(), 0, 4));
        $mobile_agents = array(
            'w3c ','acs-','alav','alca','amoi','audi','avan','benq','bird','blac',
            'blaz','brew','cell','cldc','cmd-','dang','doco','eric','hipt','inno',
            'ipaq','java','jigs','kddi','keji','leno','lg-c','lg-d','lg-g','lge-',
            'maui','maxo','midp','mits','mmef','mobi','mot-','moto','mwbp','nec-',
            'newt','noki','palm','pana','pant','phil','play','port','prox',
            'qwap','sage','sams','sany','sch-','sec-','send','seri','sgh-','shar',
            'sie-','siem','smal','smar','sony','sph-','symb','t-mo','teli','tim-',
            'tosh','tsm-','upg1','upsi','vk-v','voda','wap-','wapa','wapi','wapp',
            'wapr','webc','winw','winw','xda ','xda-');

        if (in_array($mobile_ua,$mobile_agents)) {
            $mobile_browser++;
        }

        if (strpos(strtolower(self::get_user_agent()),'opera mini') > 0) {
            $mobile_browser++;
                //Check for tablets on opera mini alternative headers
            $stock_ua = strtolower(isset($_SERVER['HTTP_X_OPERAMINI_PHONE_UA'])?$_SERVER['HTTP_X_OPERAMINI_PHONE_UA']:(isset($_SERVER['HTTP_DEVICE_STOCK_UA'])?$_SERVER['HTTP_DEVICE_STOCK_UA']:''));
            if (preg_match('/(tablet|ipad|playbook)|(android(?!.*mobile))/i', $stock_ua)) {
                $tablet_browser++;
            }
        }

        if ($tablet_browser > 0) {
               // do something for tablet devices
            return 'Tablet';
        }
        else if ($mobile_browser > 0) {
               // do something for mobile devices
            return 'Mobile';
        }
        else {
               // do something for everything else
            return 'Computer';
        }   
    }

}
\$\endgroup\$
0
15
\$\begingroup\$

IP Detection.

You are confusing an IP address with an HTTP header, the latter being utterly unreliable. It is ridiculously simple to fake an HTTP header. There is an old cautionary tale by Anthony Ferrara about how he unintentionally hacked Stack Overflow by manipulating headers. You may want to read about it. Not to mention that even if you catch one of these headers, it is quite possible that it would belong to a private network, whicih would make it essentially useless.

In short, just use REMOTE_ADDR if you need an IP address and you have no good reason to use something else. For example, some web-servers are using a local proxy, and in this case REMOTE_ADDR always contains the proxy's IP, not the outside IP, while the latter being stored in some of the headers. In this particular case you can write a similar translator, but of course not a code that picks a random HTTP header out of the pile, but a certain header that is deliberately set by the proxy.

OOP

Sorry, there is no OOP. It's just a collection of functions, which are, for some reason, combined into a single class. You'd save yourself a few keystrokes if just remove the class and keep these methods as functions.

If you want to learn OOP, maybe you could create a class to parse the user agent. It could accept the user agent string in the constructor and then give you various details through methods.

\$\endgroup\$
3
  • 3
    \$\begingroup\$ "may be you could create a class to parse the user agent" - Although be aware that they too are unreliable and some browsers are even beginning to phase them out, so this should only really be done as a learning experience. \$\endgroup\$ – Nick Jun 20 '20 at 12:10
  • \$\begingroup\$ And the inconsistent indentation and use of empty lines? \$\endgroup\$ – Peter Mortensen Jun 20 '20 at 15:12
  • \$\begingroup\$ Thanks @Cody. I had something similar in my mind in making the link more descriptive but failed formulating. And thanks for the other edits. \$\endgroup\$ – Your Common Sense Jun 21 '20 at 5:27
10
\$\begingroup\$

Are there any problems in it or is it well-written?

Well, it is not really written in a simple manner - it is very redundant. A commonly accepted principle is the Don't Repeat Yourself. principle. For example, the following block:

    if (getenv('HTTP_CLIENT_IP'))
        $mainIp = getenv('HTTP_CLIENT_IP');
    else if(getenv('HTTP_X_FORWARDED_FOR'))
        $mainIp = getenv('HTTP_X_FORWARDED_FOR');
    else if(getenv('HTTP_X_FORWARDED'))
        $mainIp = getenv('HTTP_X_FORWARDED');
    else if(getenv('HTTP_FORWARDED_FOR'))
        $mainIp = getenv('HTTP_FORWARDED_FOR');
    else if(getenv('HTTP_FORWARDED'))
        $mainIp = getenv('HTTP_FORWARDED');
    else if(getenv('REMOTE_ADDR'))
        $mainIp = getenv('REMOTE_ADDR');
    else
        $mainIp = 'UNKNOWN';
    return $mainIp;

Could be simplified by putting each key into an array:

$ipKeys = ['HTTP_CLIENT_IP', 'HTTP_X_FORWARDED_FOR', ...];

or that could be stored in a constant:

const IP_KEYS = ['HTTP_CLIENT_IP', 'HTTP_X_FORWARDED_FOR', ...];

Then iterate over those keys to set the value once a value is found:

foreach ($ipKeys as $key) {
    if (getenv($key)) {
        return getenv($key);
    }
}
return 'UNKNOWN';

As Gwyn Evans suggests the return value of the call to getenv() can be stored in a variable and if that value doesn't evaluate to false then the variable can be returned, though this may go against an accepted coding standard/style guide so use with caution:

foreach ($ipKeys as $key) {
    if ($value = getenv($key)) {
        return $value;
    }
}
return 'UNKNOWN';

Also in the end of the method get_device() there are multiple return statements, which is fine, but each one is in a conditional block:

    if ($tablet_browser > 0) {
           // do something for tablet devices
        return 'Tablet';
    }
    else if ($mobile_browser > 0) {
           // do something for mobile devices
        return 'Mobile';
    }
    else {
           // do something for everything else
        return 'Computer';
    }

The else keyword is not needed in the second and third cases, since the return in the previous cases would exit the method.

\$\endgroup\$
4
  • \$\begingroup\$ Isn't it possible to avoid the explicit loop by predicates, array filters, or something like that? \$\endgroup\$ – Peter Mortensen Jun 20 '20 at 16:07
  • 4
    \$\begingroup\$ Surely DRY suggests that the two calls to getenv() should be replaced with a single call with a saved result that's used for the test and return? \$\endgroup\$ – Gwyn Evans Jun 21 '20 at 11:55
  • \$\begingroup\$ @GwynEvans - good call. I have incorporated that advice. Many thanks! \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jun 22 '20 at 18:28
  • \$\begingroup\$ The single-line condition with assignment can be split into two lines and maintain the same logic without risking any conflict with coding styles/standards. \$\endgroup\$ – mickmackusa Jun 22 '20 at 21:20
2
\$\begingroup\$
  • I don't see any value in writing a private method that fetches $_SERVER data. I mean, all you have to do is nominate the key and the value is accessed and I can't really foresee the need to change the source of the data -- so avoid the unnecessary overhead.

  • Running with YCS's advice to only rely on REMOTE_ADDR, that condition block can be reduced to a single line levering a ternary operator (i.e. ?:).

  • Regex is great and powerful, but it should only be used when it offers a valuable advantage over simpler/faster methods. With the exception of Mac OS X (which employs an alternative/pipe), the $os_array is merely checking for the case-insensitive existence of a substring in the string -- this is performed by stripos() without rolling out the regex engine. Unless you are planning on finding multiple matches and returning the last match, you should write a conditional break/return for best efficiency in your loop.

  • Same advice again for get_browser().

  • In get_device(), you only care if there is a generated value greater than 0 before your conditional expressions are satisfied -- so why bother counting higher than 1? Don't, of course, unless there is something in your script that you are not telling us (e.g. // do something for tablet devices). As soon as you have what you want, short circuit the flow and take the desired action(s).

  • Your first $tablet_browser regex pattern has unnecessary parentheses which can be removed without damaging the pattern logic. It is pointless to call strtolower() if your pattern has a case-insensitive flag on it. Omit the unneeded function call.

  • You didn't escape the dots in your regex pattern (up.browser|up.link), this makes your pattern potentially less accurate.

  • As a general piece of advice, pay closer attention to spacing and tabbing. Keeping your script in good spacing makes it easier to read and maintain.

  • Writing else if as two words in PHP is a violation of PSR-12 guidelines. It is effectively using two separate control structures -- the former of which is not enjoying the expected curly brace syntax.

  • It is good practice to define the return data type from each method.

Code:

class UserInfo
{        
    public static function get_ip(): string
    {
        return getenv('REMOTE_ADDR') ?: 'UNKNOWN';
    }

    public static function get_os(): string
    {
        $os_array = [
            'windows nt 10'  => 'Windows 10',
            'windows nt 6.3' => 'Windows 8.1',
            'windows nt 6.2' => 'Windows 8',
            'windows nt 6.1' => 'Windows 7',
            'windows nt 6.0' => 'Windows Vista',
            'windows nt 5.2' => 'Windows Server 2003/XP x64',
            'windows nt 5.1' => 'Windows XP',
            'windows xp'     => 'Windows XP',
            'windows nt 5.0' => 'Windows 2000',
            'windows me'     => 'Windows ME',
            'win98'          => 'Windows 98',
            'win95'          => 'Windows 95',
            'win16'          => 'Windows 3.11',
            'macintosh'      => 'Mac OS X',
            'mac os x'       => 'Mac OS X',
            'mac_powerpc'    => 'Mac OS 9',
            'linux'          => 'Linux',
            'ubuntu'         => 'Ubuntu',
            'iphone'         => 'iPhone',
            'ipod'           => 'iPod',
            'ipad'           => 'iPad',
            'android'        => 'Android',
            'blackberry'     => 'BlackBerry',
            'webos'          => 'Mobile'
        ];

        foreach ($os_array as $substring => $value) {
            if (stripos($_SERVER['HTTP_USER_AGENT'], $substring) !== false) {
                return $value;
            }
        }   
        return "Unknown OS Platform";
    }

    public static function get_browser(): string
    {
        $browser_array = [
            'msie'      =>  'Internet Explorer',
            'Trident'   =>  'Internet Explorer',
            'firefox'   =>  'Firefox',
            'safari'    =>  'Safari',
            'chrome'    =>  'Chrome',
            'edge'      =>  'Edge',
            'opera'     =>  'Opera',
            'netscape'  =>  'Netscape',
            'maxthon'   =>  'Maxthon',
            'konqueror' =>  'Konqueror',
            'ubrowser'  =>  'UC Browser',
            'mobile'    =>  'Handheld Browser'
        ];

        foreach ($browser_array as $substring => $value) {
            if (stripos($_SERVER['HTTP_USER_AGENT'], $substring) !== false) {
                return $value;
            }
        }
        return "Unknown Browser";
    }

    public static function  get_device(): string
    {
        if (preg_match('/tablet|ipad|playbook|android(?!.*(mobi|opera mini))/i', $_SERVER['HTTP_USER_AGENT'])) {
            // do something for tablet devices
            return 'Tablet';
        }
        if (stripos($_SERVER['HTTP_USER_AGENT'], 'opera mini') !== false) {
            $stock_ua = $_SERVER['HTTP_X_OPERAMINI_PHONE_UA'] ?? $_SERVER['HTTP_DEVICE_STOCK_UA'] ?? '';
            if (preg_match('/tablet|ipad|playbook|android(?!.*mobile)/i', $stock_ua)) {
                // do something for tablet devices
                return 'Tablet';
            }
            // do something for mobile devices
            return 'Mobile';
       }

       $mobile_keywords = ['up.browser', 'up.link', 'mmp', 'symbian', 'smartphone', 'midp', 'wap', 'phone', 'android', 'iemobile'];
       foreach ($mobile_keywords as $keyword) {
           if (stripos($_SERVER['HTTP_USER_AGENT'], $keyword) !== false) {
               // do something for mobile devices
               return 'Mobile';
           }
       }

       if (stripos($_SERVER['HTTP_ACCEPT']), 'application/vnd.wap.xhtml+xml') > 0 || isset($_SERVER['HTTP_X_WAP_PROFILE']) || isset($_SERVER['HTTP_PROFILE'])) {
           // do something for mobile devices
           return 'Mobile';
        }

        $mobile_agents = [
            'w3c ', 'acs-', 'alav', 'alca', 'amoi', 'audi', 'avan', 'benq', 'bird', 'blac',
            'blaz', 'brew', 'cell', 'cldc', 'cmd-', 'dang', 'doco', 'eric', 'hipt', 'inno',
            'ipaq', 'java', 'jigs', 'kddi', 'keji', 'leno', 'lg-c', 'lg-d', 'lg-g', 'lge-',
            'maui', 'maxo', 'midp', 'mits', 'mmef', 'mobi', 'mot-', 'moto', 'mwbp', 'nec-',
            'newt', 'noki', 'palm', 'pana', 'pant', 'phil', 'play', 'port', 'prox', 'qwap',
            'sage', 'sams', 'sany', 'sch-', 'sec-', 'send', 'seri', 'sgh-', 'shar', 'sie-',
            'siem', 'smal', 'smar', 'sony', 'sph-', 'symb', 't-mo', 'teli', 'tim-', 'tosh',
            'tsm-', 'upg1', 'upsi', 'vk-v', 'voda', 'wap-', 'wapa', 'wapi', 'wapp', 'wapr',
            'webc', 'winw', 'winw', 'xda ', 'xda-'
        ];
        if (in_array(strtolower(substr($_SERVER['HTTP_USER_AGENT'], 0, 4)), $mobile_agents)) {
            // do something for mobile devices
            return 'Mobile';
        }

        // do something for everything else
        return 'Computer';
    }
}
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.