Validating an IP address and returning the reason it's invalid

I have a method that validates a user-written IP address in an Android application. If it's invalid, I need to know why and notify the user using a Toast.

I created an enum for this called Status that has a list of reasons for an IP address to be invalid and a value for when it's valid. The method isValid will return a Status value depending on whether or not it's invalid.

Android code

switch(IP.isValid(ip.getText().toString())) {
     case INVALID_NUMBER:
         Toast.makeText(MainActivity.this, "You have entered an invalid octect(s) value", Toast.LENGTH_SHORT).show();
         break;
     case INVALID_LENGTH:
         Toast.makeText(MainActivity.this, "You have entered an IP with an invalid length", Toast.LENGTH_SHORT).show();
         break;
     case NON_NUMERIC:
         Toast.makeText(MainActivity.this, "You have entered a non-numeric IP", Toast.LENGTH_SHORT).show();
         break;
}

• ip is an EditText field.

Status enum

public enum Status {
    INVALID_NUMBER,
    INVALID_LENGTH,
    NON_NUMERIC,
    VALID
}

isValid() method

public static Status isValid(String ip) {
    String[] octets = ip.split("\\.");
    if(octets.length != 4) {
        return Status.INVALID_LENGTH;
    }

    for(String octet : octets) {
        if(StringUtils.isNumeric(octet)) {
            int val = Integer.parseInt(octet);
            if(val > 255 || val < 0) {
                return Status.INVALID_NUMBER;
            }
        } else {
            return Status.NON_NUMERIC;
        }
    }

    return Status.VALID;
}

StringUtils.isNumeric()

(StringUtils is a class I created, it's not from Apache)
https://stackoverflow.com/a/237204/4355066

```
public static boolean isNumeric(String str) {
if (str == null) {
return false;
}
int length = str.length();
if (length == 0) {
return false;
}
int i = 0;
if (str.charAt(0) == '-') {
if (length == 1) {
return false;
}

Your code has three major errors that all the previous reviewers failed to spot. If I were a lecturer, I'd show your code to the students as an example why they should always use libraries, even doing something seemingly simple as validating the IPv4 address.

-
It accepts 127.0.0.1..... as a valid IP address, which it is not. Pass -1 as a second argument to the split function.

-
Because of your faulty isNumeric function, the address 0.00123.0000034.12 is considered valid, too.

-
Similar to above, the address 127.-0.-0.1 is accepted too.

Consider using a library for validating the IP addresses

As described above, you can never be too careful. Besides that, we are starting to make the migration to the IPv6 standard, which is harder to validate.

So, if you can rely on a well tested library for IP validation that does not bring an additional huge dependency to your code, use that instead.

It is not resilient against crafted inputs

Depending on the source of the input, the validation function might pose a huge performance risk and cause OM (out of memory) errors.

See, even really short strings of just dots ".........x" create one String object per character. String object and a pointer overhead in Java is rather in the neighborhood of 44 bytes. Therefore, someone sending you a 10 MiB string is typically going to overflow the heap space.

Code locality (tie error messages to enum values)

Because error strings are tied to enum values, it is a good thing to specify them in the actual enum construct.

public enum IpValidation {
    INVALID_NUMBER("You have entered an invalid octect(s) value"),
    INVALID_LENGTH("You have entered an IP with an invalid length"),
    NON_NUMERIC("You have entered an IP with an invalid length"),
    VALID("This IP address is valid");

    private final String error;

    private IpValidation(final String error) {
        this.error = error;
    }

    public String getError() {
        return error;
    }
}

Give the enum a more meaningful name

Strings is just a bad name to give an enum. Pick something else.

Nits

• Do put a space after the switch, if, and for keywords.

• Using enums on Android might have performance consequences. Depending on the use case, you might want to avoid it.

StackExchange Code Review Q#139280, answer score: 6