Improvement of the IPv4 address format check

[ka2]

For the current "is_ip_address()" in the function of out of range, such as "256.256.256.256" IP so will also be judged as IPv4, it is increases the usefulness of the function you have to return false for out of range of IP.

[ka2]

I fixed file.

[GaryJ]

Hello ka2, welcome to Trac. Thank you for your ticket and patch.

A couple of things about your patch to consider:

• Only the last hunk is important here - all the rest are un-related whitespace fixes. Try and keep to fixes just in the immediate function you're changing - in this case, adding the non-optional braces for the if that you're amending.
• This is going to need unit tests to show that 999.1.1.1 and other similarly broken IPv4 address potentials are no longer valid. If unit tests for correct IP addresses, these should be added too.

[johnbillion]

This method doesn't validate an IP address, it just checks whether the passed value looks like an IP address as opposed to a domain name. From the method's description:

This does not verify if the IP is a valid IP, only that it appears to be an IP address.

This method is used in WP_Http_Streams::verify_ssl_certificate() to determine whether the host is an IP address or a domain name. If an invalid IP address is passed to an HTTP request, then WP_Http::is_ip_address() must still return true otherwise the host will incorrectly be treated as a domain name. Note that either way, the HTTP request will fail due to the invalid IP address.

I think this is a wontfix.

[chriscct7]

I actually like this enhancement. It ensures that non-valid IP addresses won't be validated as such, so if plugins rely on this to check an IP address, it can be safely used as part of a validation routine. Refreshed patch from proper directory.

[swissspidy]

34538.patch​ still includes unneeded whitespace changes.

This method doesn't validate an IP address, it just checks whether the passed value looks like an IP address as opposed to a domain name.

Agree on this, though the second regex seems to check for real IPv6 addresses. See ​http://home.deds.nl/~aeron/regex/

If an invalid IP address is passed to an HTTP request, then WP_Http::is_ip_address() must still return true otherwise the host will incorrectly be treated as a domain name. Note that either way, the HTTP request will fail due to the invalid IP address.

What exactly would change for an HTTP request with this proposed patch?

[ka2]

Thank you for your consideration various about my patch.

As you say, I also think that there is no benefit to the existing HTTP request processing even if the verification of the IP address by my patch was stricter.
However, I think that this function is useable from a place other than the core, as for example plugins. Also I think that it will become very user-friendly feature if WP_Http::is_ip_address() will be able to strictly the IP validation.

As another approach, I thought that divides the verification processing by adding a toggle of strict verification to the second argument of the function. But it just would simply become a redundant function.

I suggested to the ticket because I thought that might be there are happy to some people in the case that became to be able to complete IP address verification in this function.

Thank you,

[ka2]

I've created a unit test.
Test code contents are as follows:

/** * @dataProvider ip_address_testcases */	function test_is_ip_address( $maybe_ip, $expected ) {	$actual = WP_Http::is_ip_address( $maybe_ip );	$this->assertEquals( $expected, $actual );	}	function ip_address_testcases() {	// 0: The IP Address, 1: The expected resulting	return array(	// Valid IPv4 address	// See: http://tools.ietf.org/html/rfc5737	array( '0.0.0.0', 4 ),	array( '127.0.0.1', 4 ), // The loopback address	array( '192.0.2.0', 4 ), // RFC 5737 - IPv4 Address Blocks Reserved for Documentation	array( '198.51.100.0', 4 ), // RFC 5737 - IPv4 Address Blocks Reserved for Documentation	array( '203.0.113.0', 4 ), // RFC 5737 - IPv4 Address Blocks Reserved for Documentation	array( '255.255.255.255', 4 ),	// Invalid IPv4 address	array( '256.255.255.255', false ), // The first octet is out of range	array( '255.256.255.255', false ), // The second octet is out of range	array( '255.255.256.255', false ), // The third octet is out of range	array( '255.255.255.256', false ), // The fourth octet is out of range	array( '999.999.999.999', false ), // All octet is out of range	array( '2000.1.1.1', false ),	array( '1.2000.1.1', false ),	array( '1.1.2000.1', false ),	array( '1.1.1.2000', false ),	array( '2000.2000.2000.2000', false ),	// Valid IPv6 address	// See: http://tools.ietf.org/html/rfc4291	array( '0:0:0:0:0:0:0:0', 6 ), // The unspecified address	array( '::', 6 ), // The unspecified address (in compressed form)	array( '0:0:0:0:0:0:0:1', 6 ), // The loopback address	array( '::1', 6 ), // The loopback address (in compressed form)	array( '2001:db8::', 6 ), // RFC 3849 - IPv6 Address Prefix Reserved for Documentation	array( '2001:0db8:0000:0000:0000:0000:dead:beaf', 6 ),	array( '2001:db8:0:0:0:0:dead:beaf', 6 ),	array( '2001:db8::dead:beaf', 6 ),	array( 'ABCD:EF01:2345:6789:ABCD:EF01:2345:6789', 6 ), // RFC 4291 - Example	array( '2001:DB8:0:0:8:800:200C:417A', 6 ), // RFC 4291 - Example of unicast address	array( '2001:DB8::8:800:200C:417A', 6 ), // RFC 4291 - Example of unicast address (in compressed form)	array( 'FF01:0:0:0:0:0:0:101', 6 ), // RFC 4291 - Example of multicast address	array( 'FF01::101', 6 ), // RFC 4291 - Example of multicast address (in compressed form)	array( '0:0:0:0:0:0:13.1.68.3', 6 ), // RFC 4291 - Example of an alternative form with a mixed environment of IPv4 and IPv6 nodes	array( '::13.1.68.3', 6 ), // RFC 4291 - Example of an alternative form with a mixed environment of IPv4 and IPv6 nodes (in compressed form)	array( '0:0:0:0:0:FFFF:129.144.52.38', 6 ), // RFC 4291 - Example of an alternative form with a mixed environment of IPv4 and IPv6 nodes	array( '::FFFF:129.144.52.38', 6 ), // RFC 4291 - Example of an alternative form with a mixed environment of IPv4 and IPv6 nodes (in compressed form)	// Invalid IPv6 address	// See: https://github.com/richb-intermapper/IPv6-Regex	array( '2001:DB8:0:0:8:800:200C:417A:221', false ), // unicast full	array( 'FF01::101::2', false ), // multicast compressed	array( '02001:0000:1234:0000:0000:C1C0:ABCD:0876', false ), // extra 0 not allowed	array( '2001:0000:1234:0000:00001:C1C0:ABCD:0876', false ), // extra 0 not allowed	array( '2001:0000:1234:0000:0000:C1C0:ABCD:0876 0', false ), // junk after valid address	array( '2001:0000:1234: 0000:0000:C1C0:ABCD:0876', false ), // internal space	array( '3ffe:0b00:0000:0001:0000:0000:000a', false ), // Segments is less than 8	array( 'FF02:0000:0000:0000:0000:0000:0000:0000:0001', false ), // Segments is over 8	array( '3ffe:b00::1::a', false ), // Compressed double	array( '::1111:2222:3333:4444:5555:6666::', false ), // Compressed double	array( '1::5:400.2.3.4', false ), // Embedded ipv4 address is out of range	array( '1::5:192.168.0.256', false ), // Embedded ipv4 address is out of range	array( '2001:1:1:1:1:1:255Z255X255Y255', false ), // garbage instead of "." in IPv4	array( '::ffff:192x168.1.26', false ), // ditto	array( '::ffff:2.3.4', false ), // has ipv4 octet lost	array( '1.2.3.4:1111:2222:3333:4444::5555', false ), // Aeron	array( ':', false ),	array( ':::', false ),	// other	array( '123', false ),	array( 'ldkfj', false ),	array( '.', false ),	array( '..', false ),	array( '...', false ),	);	} 

Then, results of unit test execution are as follows:

# phpunit --group="http" Installing... Running as single site... To run multisite, use -c tests/phpunit/multisite.xml Not running ajax tests. To execute these, use --group ajax. Not running ms-files tests. To execute these, use --group ms-files. Not running external-http tests. To execute these, use --group external-http. PHPUnit 5.0.8 by Sebastian Bergmann and contributors. ................................................................ 64 / 89 ( 71%) ......................... 89 / 89 (100%) Time: 16.41 seconds, Memory: 74.75Mb OK (89 tests, 89 assertions) 

I will attach a patch file that unit testing is included.

In addition, I have been corresponding of parts leakage in white space refactoring.

Thank you,

[ka2]

It is a patch that contains the unit test.

[chriscct7]

The new patch is good but the whitespace changes at the top of it need to be removed from it for it to be merged into core.

[ka2]

I recreated a patch with removed of the whitespace changes.

Thank you,