Suppress Site Health ICU test if site or home URL is not an IDN (#4698)

* Suppress ICU test if site or home URL is not an IDN * Check if any domain labels start with `xn--` to determine if a domain is an IDN * Return true if IDN domain found * Update src/Admin/SiteHealth.php Make `is_intl_extension_needed()` private Co-authored-by: Weston Ruter <westonruter@google.com> Co-authored-by: Weston Ruter <westonruter@google.com>
ampproject · May 14, 2020 · 2da4b55 · 2da4b55
1 parent cc6cc71
commit 2da4b55
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 37 deletions.
diff --git a/src/Admin/SiteHealth.php b/src/Admin/SiteHealth.php
@@ -49,15 +49,19 @@ public function add_tests( $tests ) {
 			'label' => esc_html__( 'cURL multi functions', 'amp' ),
 			'test'  => [ $this, 'curl_multi_functions' ],
 		];
-		$tests['direct']['amp_icu_version']             = [
-			'label' => esc_html__( 'ICU version', 'amp' ),
-			'test'  => [ $this, 'icu_version' ],
-		];
-		$tests['direct']['amp_css_transient_caching']   = [
+
+		if ( $this->is_intl_extension_needed() ) {
+			$tests['direct']['amp_icu_version'] = [
+				'label' => esc_html__( 'ICU version', 'amp' ),
+				'test'  => [ $this, 'icu_version' ],
+			];
+		}
+
+		$tests['direct']['amp_css_transient_caching'] = [
 			'label' => esc_html__( 'Transient caching of stylesheets', 'amp' ),
 			'test'  => [ $this, 'css_transient_caching' ],
 		];
-		$tests['direct']['amp_xdebug_extension']        = [
+		$tests['direct']['amp_xdebug_extension']      = [
 			'label' => esc_html__( 'Xdebug extension', 'amp' ),
 			'test'  => [ $this, 'xdebug_extension' ],
 		];
@@ -512,33 +516,35 @@ private function get_css_transient_caching_sampling_range() {
 	/**
 	 * Adds suggested PHP extensions to those that Core depends on.
 	 *
-	 * @param array $extensions The existing extensions from Core.
+	 * @param array $core_extensions The existing extensions from Core.
 	 * @return array The extensions, including those for AMP.
 	 */
-	public function add_extensions( $extensions ) {
-		return array_merge(
-			$extensions,
-			[
-				'intl'     => [
-					'extension' => 'intl',
-					'function'  => 'idn_to_utf8',
-					'required'  => false,
-				],
-				'json'     => [
-					'extension' => 'json',
-					'function'  => 'json_encode',
-					'required'  => false,
-				],
-				'mbstring' => [
-					'extension' => 'mbstring',
-					'required'  => false,
-				],
-				'zip'      => [
-					'extension' => 'zip',
-					'required'  => false,
-				],
-			]
-		);
+	public function add_extensions( $core_extensions ) {
+		$extensions = [
+			'json'     => [
+				'extension' => 'json',
+				'function'  => 'json_encode',
+				'required'  => false,
+			],
+			'mbstring' => [
+				'extension' => 'mbstring',
+				'required'  => false,
+			],
+			'zip'      => [
+				'extension' => 'zip',
+				'required'  => false,
+			],
+		];
+
+		if ( $this->is_intl_extension_needed() ) {
+			$extensions['intl'] = [
+				'extension' => 'intl',
+				'function'  => 'idn_to_utf8',
+				'required'  => false,
+			];
+		}
+
+		return array_merge( $core_extensions, $extensions );
 	}
 
 	/**
@@ -577,4 +583,27 @@ public function add_styles() {
 			</style>
 		';
 	}
+
+	/**
+	 * Determine if the `intl` extension is needed.
+	 *
+	 * @return bool True if the `intl` extension is needed, otherwise false.
+	 */
+	private function is_intl_extension_needed() {
+		// Publisher's own origins.
+		$domains = array_unique(
+			[
+				wp_parse_url( site_url(), PHP_URL_HOST ),
+				wp_parse_url( home_url(), PHP_URL_HOST ),
+			]
+		);
+
+		foreach ( $domains as $domain ) {
+			if ( preg_match( '/(^|\.)xn--/', $domain ) ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
 }
diff --git a/tests/php/test-class-site-health.php b/tests/php/test-class-site-health.php
@@ -67,8 +67,16 @@ public function test_add_tests() {
 		$this->assertArrayHasKey( 'direct', $tests );
 		$this->assertArrayHasKey( 'amp_persistent_object_cache', $tests['direct'] );
 		$this->assertArrayHasKey( 'amp_curl_multi_functions', $tests['direct'] );
-		$this->assertArrayHasKey( 'amp_icu_version', $tests['direct'] );
+		$this->assertArrayNotHasKey( 'amp_icu_version', $tests['direct'] );
 		$this->assertArrayHasKey( 'amp_xdebug_extension', $tests['direct'] );
+
+		// Test that the the ICU version test is added only when site URL is an IDN.
+		add_filter( 'site_url', [ self::class, 'get_idn' ], 10, 4 );
+
+		$tests = $this->instance->add_tests( [] );
+		$this->assertArrayHasKey( 'amp_icu_version', $tests['direct'] );
+
+		remove_filter( 'site_url', [ self::class, 'get_idn' ] );
 	}
 
 	/**
@@ -370,11 +378,6 @@ public function test_get_serve_all_templates( $theme_support, $do_serve_all_temp
 	public function test_add_extensions() {
 		$this->assertEquals(
 			[
-				'intl'     => [
-					'extension' => 'intl',
-					'function'  => 'idn_to_utf8',
-					'required'  => false,
-				],
 				'json'     => [
 					'extension' => 'json',
 					'function'  => 'json_encode',
@@ -391,5 +394,30 @@ public function test_add_extensions() {
 			],
 			$this->instance->add_extensions( [] )
 		);
+
+		// Test that the the `intl` extension is added only when site URL is an IDN.
+		add_filter( 'site_url', [ self::class, 'get_idn' ], 10, 4 );
+
+		$extensions = $this->instance->add_extensions( [] );
+		$this->assertArrayHasKey( 'intl', $extensions );
+		$this->assertEquals(
+			[
+				'extension' => 'intl',
+				'function'  => 'idn_to_utf8',
+				'required'  => false,
+			],
+			$extensions['intl']
+		);
+
+		remove_filter( 'site_url', [ self::class, 'get_idn' ] );
+	}
+
+	/**
+	 * Get a an IDN for testing purposes.
+	 *
+	 * @return string
+	 */
+	public static function get_idn() {
+		return 'https://foo.xn--57h.bar.com';
 	}
 }