Skip to content
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

Merged
merged 1 commit into from Aug 8, 2014

Conversation

Projects
None yet
4 participants
@spinscale
Copy link
Member

spinscale commented Aug 4, 2014

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 private

Closes #7131

@@ -83,6 +84,9 @@ public static long ipToLong(String ip) throws ElasticsearchIllegalArgumentExcept
if (octets.length != 4) {

This comment has been minimized.

Copy link
@dadoonet

dadoonet Aug 4, 2014

Member

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

This comment has been minimized.

Copy link
@kimchy

kimchy Aug 5, 2014

Member

I think that if InetAddresses does it already, we might as well rely just on it

@dadoonet

This comment has been minimized.

Copy link
Member

dadoonet commented Aug 4, 2014

@spinscale left a comment

@dadoonet dadoonet removed the review label Aug 4, 2014

@kimchy

This comment has been minimized.

Copy link
Member

kimchy commented Aug 5, 2014

LGTM on my end, added a comment to @dadoonet

@spinscale

This comment has been minimized.

Copy link
Member Author

spinscale commented Aug 8, 2014

We need the additional check as well, as the IPAddresses.isInetAddress has a validation for IPv4 and IPv6 (which does not help us here). Thanks for reviewing

@dadoonet

This comment has been minimized.

Copy link
Member

dadoonet commented Aug 8, 2014

Ha! Thanks @spinscale

Mapping API: Improve IP address validation
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 #7131

@spinscale spinscale merged commit 724b14c into elastic:master Aug 8, 2014

@clintongormley clintongormley changed the title Mapping API: Improve IP address validation Mapping: Improve IP address validation Sep 11, 2014

@clintongormley clintongormley changed the title Mapping: Improve IP address validation Improve IP address validation Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.