From: Thalia <thalia.e.chan@googlemail.com>
Date: Mon, 27 May 2019 21:53:23 +0000 (+0100)
Subject: Make improvements to BlockManagerTest
X-Git-Tag: 1.34.0-rc.0~1578^2
X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=e9714e6a8db766010f545a5b5fda430e72596cf4;p=lhc%2Fweb%2Fwiklou.git

Make improvements to BlockManagerTest

Remove reliance on real domain and add more test cases.

Change-Id: Icd67fe8c1c9223a92a3775d34d0453328442d89e
---

diff --git a/includes/block/BlockManager.php b/includes/block/BlockManager.php
index 7b665ae35b..9823b3c950 100644
--- a/includes/block/BlockManager.php
+++ b/includes/block/BlockManager.php
@@ -345,29 +345,42 @@ class BlockManager {
 				if ( is_array( $base ) ) {
 					if ( count( $base ) >= 2 ) {
 						// Access key is 1, base URL is 0
-						$host = "{$base[1]}.$ipReversed.{$base[0]}";
+						$hostname = "{$base[1]}.$ipReversed.{$base[0]}";
 					} else {
-						$host = "$ipReversed.{$base[0]}";
+						$hostname = "$ipReversed.{$base[0]}";
 					}
 					$basename = $base[0];
 				} else {
-					$host = "$ipReversed.$base";
+					$hostname = "$ipReversed.$base";
 				}
 
 				// Send query
-				$ipList = gethostbynamel( $host );
+				$ipList = $this->checkHost( $hostname );
 
 				if ( $ipList ) {
-					wfDebugLog( 'dnsblacklist', "Hostname $host is {$ipList[0]}, it's a proxy says $basename!" );
+					wfDebugLog(
+						'dnsblacklist',
+						"Hostname $hostname is {$ipList[0]}, it's a proxy says $basename!"
+					);
 					$found = true;
 					break;
 				}
 
-				wfDebugLog( 'dnsblacklist', "Requested $host, not found in $basename." );
+				wfDebugLog( 'dnsblacklist', "Requested $hostname, not found in $basename." );
 			}
 		}
 
 		return $found;
 	}
 
+	/**
+	 * Wrapper for mocking in tests.
+	 *
+	 * @param string $hostname DNSBL query
+	 * @return string[]|bool IPv4 array, or false if the IP is not blacklisted
+	 */
+	protected function checkHost( $hostname ) {
+		return gethostbynamel( $hostname );
+	}
+
 }
diff --git a/tests/phpunit/includes/block/BlockManagerTest.php b/tests/phpunit/includes/block/BlockManagerTest.php
index 414566503e..ec6eea99c9 100644
--- a/tests/phpunit/includes/block/BlockManagerTest.php
+++ b/tests/phpunit/includes/block/BlockManagerTest.php
@@ -20,10 +20,7 @@ class BlockManagerTest extends MediaWikiTestCase {
 
 		$this->user = $this->getTestUser()->getUser();
 		$this->sysopId = $this->getTestSysop()->getUser()->getId();
-	}
-
-	private function getBlockManager( $overrideConfig ) {
-		$blockManagerConfig = array_merge( [
+		$this->blockManagerConfig = [
 			'wgApplyIpBlocksToXff' => true,
 			'wgCookieSetOnAutoblock' => true,
 			'wgCookieSetOnIpBlock' => true,
@@ -32,7 +29,11 @@ class BlockManagerTest extends MediaWikiTestCase {
 			'wgProxyList' => [],
 			'wgProxyWhitelist' => [],
 			'wgSoftBlockRanges' => [],
-		], $overrideConfig );
+		];
+	}
+
+	private function getBlockManager( $overrideConfig ) {
+		$blockManagerConfig = array_merge( $this->blockManagerConfig, $overrideConfig );
 		return new BlockManager(
 			$this->user,
 			$this->user->getRequest(),
@@ -174,50 +175,112 @@ class BlockManagerTest extends MediaWikiTestCase {
 	 * @covers ::inDnsBlacklist
 	 */
 	public function testIsDnsBlacklisted( $options, $expected ) {
-		$blockManager = $this->getBlockManager( [
+		$blockManagerConfig = array_merge( $this->blockManagerConfig, [
 			'wgEnableDnsBlacklist' => true,
-			'wgDnsBlacklistUrls' => $options[ 'inBlacklist' ] ? [ 'local.wmftest.net' ] : [],
-			'wgProxyWhitelist' => $options[ 'inWhitelist' ] ? [ '127.0.0.1' ] : [],
+			'wgDnsBlacklistUrls' => $options['blacklist'],
+			'wgProxyWhitelist' => $options['whitelist'],
 		] );
 
-		$ip = '127.0.0.1';
+		$blockManager = $this->getMockBuilder( BlockManager::class )
+			->setConstructorArgs(
+				array_merge( [
+					$this->user,
+					$this->user->getRequest(),
+				], $blockManagerConfig ) )
+			->setMethods( [ 'checkHost' ] )
+			->getMock();
+
+		$blockManager->expects( $this->any() )
+			->method( 'checkHost' )
+			->will( $this->returnValueMap( [ [
+				$options['dnsblQuery'],
+				$options['dnsblResponse'],
+			] ] ) );
+
 		$this->assertSame(
 			$expected,
-			$blockManager->isDnsBlacklisted( $ip, $options[ 'check' ] )
+			$blockManager->isDnsBlacklisted( $options['ip'], $options['checkWhitelist'] )
 		);
 	}
 
 	public static function provideIsDnsBlacklisted() {
+		$dnsblFound = [ '127.0.0.2' ];
+		$dnsblNotFound = false;
 		return [
 			'IP is blacklisted' => [
 				[
-					'inBlacklist' => true,
-					'inWhitelist' => false,
-					'check' => false,
+					'blacklist' => [ 'dnsbl.test' ],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => '1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [],
+					'checkWhitelist' => false,
+				],
+				true,
+			],
+			'IP is blacklisted; blacklist has key' => [
+				[
+					'blacklist' => [ [ 'dnsbl.test', 'key' ] ],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => 'key.1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [],
+					'checkWhitelist' => false,
+				],
+				true,
+			],
+			'IP is blacklisted; blacklist is array' => [
+				[
+					'blacklist' => [ [ 'dnsbl.test' ] ],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => '1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [],
+					'checkWhitelist' => false,
 				],
 				true,
 			],
 			'IP is not blacklisted' => [
 				[
-					'inBlacklist' => false,
-					'inWhitelist' => false,
-					'check' => false,
+					'blacklist' => [ 'dnsbl.test' ],
+					'ip' => '1.2.3.4',
+					'dnsblQuery' => '4.3.2.1.dnsbl.test',
+					'dnsblResponse' => $dnsblNotFound,
+					'whitelist' => [],
+					'checkWhitelist' => false,
 				],
 				false,
 			],
-			'IP is blacklisted and whitelisted; whitelist is checked' => [
+			'Blacklist is empty' => [
 				[
-					'inBlacklist' => true,
-					'inWhitelist' => true,
-					'check' => false,
+					'blacklist' => [],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => '1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [],
+					'checkWhitelist' => false,
 				],
-				true,
+				false,
 			],
 			'IP is blacklisted and whitelisted; whitelist is not checked' => [
 				[
-					'inBlacklist' => true,
-					'inWhitelist' => true,
-					'check' => true,
+					'blacklist' => [ 'dnsbl.test' ],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => '1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [ '127.0.0.1' ],
+					'checkWhitelist' => false,
+				],
+				true,
+			],
+			'IP is blacklisted and whitelisted; whitelist is checked' => [
+				[
+					'blacklist' => [ 'dnsbl.test' ],
+					'ip' => '127.0.0.1',
+					'dnsblQuery' => '1.0.0.127.dnsbl.test',
+					'dnsblResponse' => $dnsblFound,
+					'whitelist' => [ '127.0.0.1' ],
+					'checkWhitelist' => true,
 				],
 				false,
 			],