New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve IP address validation #7141
Improve IP address validation #7141
Conversation
@@ -83,6 +84,9 @@ public static long ipToLong(String ip) throws ElasticsearchIllegalArgumentExcept | |||
if (octets.length != 4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering why we need to keep that test. InetAddresses.isInetAddress()
does not do it?
Another way for catching specifically your issue could be to control all octets size?
if (octets.length != 4 || octets[0].length() > 3 || octets[1].length() > 3 || octets[2].length() > 3 || octets[3].length() > 3) {
throw new ElasticsearchIllegalArgumentException("failed to parse ip [" + ip + "], not full ip address (4 dots)");
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that if InetAddresses does it already, we might as well rely just on it
@spinscale left a comment |
LGTM on my end, added a comment to @dadoonet |
We need the additional check as well, as the |
Ha! Thanks @spinscale |
Until now, IP addresses were only checked for four dots, which allowed invalid values like 127.0.0.111111 This adds an additional check for validation. Closes elastic#7131
Until now, IP addresses were only checked for four dots, which
allowed invalid values like 127.0.0.111111
This adds an additional check for validation.
Note: This does have a performance impact in the log file indexing case as it adds an additional parsing step. Maybe this was the reason, why it had not been implemented in the first case? We could potentially just reuse the code from guavas
InetAddresses.textToNumericFormatV4()
which is unfortunately privateCloses #7131