HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Test if given ip is a public one?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
publiconegiventest

Problem

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;
}

Solution

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.

Code Snippets

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());
}

Context

StackExchange Code Review Q#65071, answer score: 15

Revisions (0)

No revisions yet.