7
\$\begingroup\$

I'd like to check if the user submitted adress ip is a valid public IPv4 (non loopback, non local, non reserved).

For that purpose, I came up with that method, but I was wondering if this was enough or if I was missing some tests.

private boolean isValidPublicIp(String ip) {
    try
    {
        Inet4Address address = (Inet4Address) InetAddress.getByAddress(ip.getBytes());
        if (!address.isSiteLocalAddress() && !address.isAnyLocalAddress() && !address.isLinkLocalAddress() && !address.isLoopbackAddress() && !address.isMulticastAddress()) {
            return true;
        }
    }
    catch (UnknownHostException exception) {}

    return false;
}
\$\endgroup\$
  • \$\begingroup\$ I will assume your user only inputs valid ipv4 addresses, and not ipv6, which makes this use-case slightly outdated ... \$\endgroup\$ – Vogel612 Oct 8 '14 at 9:57
  • \$\begingroup\$ Yes, I forgot to mention but I'm only targetting, for now, IPv4. \$\endgroup\$ – Cyril N. Oct 8 '14 at 10:02
12
\$\begingroup\$

One-shot answer:

private boolean isValidPublicIp(String ip) {
    Inet4Address address;
    try {
        address = (Inet4Address) InetAddress.getByName(ip);
    } catch (UnknownHostException exception) {
        return false; // assuming no logging, exception handling required
    }
    return !(address.isSiteLocalAddress() || 
             address.isAnyLocalAddress()  || 
             address.isLinkLocalAddress() || 
             address.isLoopbackAddress() || 
             address.isMulticastAddress());
}

Reduce your try scope, and you do not need to do something like if (boolean) { return true; }. Just return boolean;. :)

edit: Your usage of ip.getBytes() for getByAddress() is actually wrong. The most straight-forward way is just to do getByName(ip), as shown here and here.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the optimisation, but do you know if every kind of ip is checked. I mean, is this method will return true only for valid public ip address or did I missed some checkings ? \$\endgroup\$ – Cyril N. Oct 8 '14 at 10:03
  • \$\begingroup\$ Perhaps a slightly larger context as to why you need a "public" IP then? Unfortunately this is not really in my domain, so let's hope someone else can answer this question specifically. \$\endgroup\$ – h.j.k. Oct 8 '14 at 10:12
  • \$\begingroup\$ I indicated it in my question; it's a data my user submit. It should a valid, accessible ip adrress. \$\endgroup\$ – Cyril N. Oct 8 '14 at 10:15
  • \$\begingroup\$ To give a larger context, they submit a list of IP addresses that will be allowed to access a specific part of the website. By default, I take their IP address, but they can add others (friends for example). It seems counter intuitive but the website in question is dedicated to technical IT users, no worries ;) \$\endgroup\$ – Cyril N. Oct 8 '14 at 10:28
  • 1
    \$\begingroup\$ This seems like a self correcting problem. If they enter an IP address that is not public and not the address they are accessing the site from then they'll be locked out and have to re-enter. After doing the basic checks outlined above perhaps you could redirect the user to a test page behind the same whitelist and assert that they can access it. Will the IP the user enters be the same as the IP they are coming from or a different IP? Would something like sending an email verification code then validate the code then note the address that validation request came from be better? Dynamic IPs? \$\endgroup\$ – Freiheit Oct 8 '14 at 13:32

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.