From 3922ccedca02db216917016a0cddb17a972313f3 Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Wed, 13 May 2020 13:40:53 -0400 Subject: [PATCH 1/4] Suppress ICU test if site or home URL is not an IDN --- src/Admin/SiteHealth.php | 39 +++++++++++++++++++++++----- tests/php/test-class-site-health.php | 15 ++++++++++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/Admin/SiteHealth.php b/src/Admin/SiteHealth.php index 10a75bd157f..d6fb1204985 100644 --- a/src/Admin/SiteHealth.php +++ b/src/Admin/SiteHealth.php @@ -49,15 +49,42 @@ 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 ( function_exists( 'idn_to_utf8' ) ) { + $has_idn = false; + + // 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 ) { + // The third parameter is set explicitly to prevent issues with newer PHP versions compiled with an old ICU version. + // phpcs:ignore PHPCompatibility.Constants.RemovedConstants.intl_idna_variant_2003Deprecated + $unicode_domain = idn_to_utf8( $domain, IDNA_DEFAULT, defined( 'INTL_IDNA_VARIANT_UTS46' ) ? INTL_IDNA_VARIANT_UTS46 : INTL_IDNA_VARIANT_2003 ); + + if ( $unicode_domain && $domain !== $unicode_domain ) { + $has_idn = true; + break; + } + } + + if ( $has_idn ) { + $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' ], ]; diff --git a/tests/php/test-class-site-health.php b/tests/php/test-class-site-health.php index 30d37a341ed..18c346f6649 100644 --- a/tests/php/test-class-site-health.php +++ b/tests/php/test-class-site-health.php @@ -67,8 +67,21 @@ 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' ] ); + } + + public static function get_idn() { + return 'https://xn--e28h.com'; } /** From b97582b56d53792c22caee39a4de3bf31dca2587 Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Wed, 13 May 2020 17:22:50 -0400 Subject: [PATCH 2/4] Check if any domain labels start with `xn--` to determine if a domain is an IDN --- src/Admin/SiteHealth.php | 111 ++++++++++++++------------- tests/php/test-class-site-health.php | 35 ++++++--- 2 files changed, 83 insertions(+), 63 deletions(-) diff --git a/src/Admin/SiteHealth.php b/src/Admin/SiteHealth.php index d6fb1204985..3d368e41f5b 100644 --- a/src/Admin/SiteHealth.php +++ b/src/Admin/SiteHealth.php @@ -50,34 +50,11 @@ public function add_tests( $tests ) { 'test' => [ $this, 'curl_multi_functions' ], ]; - if ( function_exists( 'idn_to_utf8' ) ) { - $has_idn = false; - - // 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 ) { - // The third parameter is set explicitly to prevent issues with newer PHP versions compiled with an old ICU version. - // phpcs:ignore PHPCompatibility.Constants.RemovedConstants.intl_idna_variant_2003Deprecated - $unicode_domain = idn_to_utf8( $domain, IDNA_DEFAULT, defined( 'INTL_IDNA_VARIANT_UTS46' ) ? INTL_IDNA_VARIANT_UTS46 : INTL_IDNA_VARIANT_2003 ); - - if ( $unicode_domain && $domain !== $unicode_domain ) { - $has_idn = true; - break; - } - } - - if ( $has_idn ) { - $tests['direct']['amp_icu_version'] = [ - 'label' => esc_html__( 'ICU version', 'amp' ), - 'test' => [ $this, 'icu_version' ], - ]; - } + 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'] = [ @@ -539,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 ); } /** @@ -604,4 +583,30 @@ public function add_styles() { '; } + + /** + * Determine if the `intl` extension is needed. + * + * @return bool True if the `intl` extension is needed, otherwise false. + */ + public function is_intl_extension_needed() { + $has_idn = false; + + // 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 ) ) { + $has_idn = true; + break; + } + } + + return $has_idn; + } } diff --git a/tests/php/test-class-site-health.php b/tests/php/test-class-site-health.php index 18c346f6649..4e79b234ed0 100644 --- a/tests/php/test-class-site-health.php +++ b/tests/php/test-class-site-health.php @@ -70,7 +70,6 @@ public function test_add_tests() { $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 ); @@ -80,10 +79,6 @@ public function test_add_tests() { remove_filter( 'site_url', [ self::class, 'get_idn' ] ); } - public static function get_idn() { - return 'https://xn--e28h.com'; - } - /** * Test persistent_object_cache. * @@ -383,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', @@ -404,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'; } } From da6bd14b20544ed15c6dba52eb97005f0726869e Mon Sep 17 00:00:00 2001 From: Pierre Gordon Date: Wed, 13 May 2020 17:47:54 -0400 Subject: [PATCH 3/4] Return true if IDN domain found --- src/Admin/SiteHealth.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Admin/SiteHealth.php b/src/Admin/SiteHealth.php index 3d368e41f5b..ed9f19624cc 100644 --- a/src/Admin/SiteHealth.php +++ b/src/Admin/SiteHealth.php @@ -590,8 +590,6 @@ public function add_styles() { * @return bool True if the `intl` extension is needed, otherwise false. */ public function is_intl_extension_needed() { - $has_idn = false; - // Publisher's own origins. $domains = array_unique( [ @@ -602,11 +600,10 @@ public function is_intl_extension_needed() { foreach ( $domains as $domain ) { if ( preg_match( '/(^|\.)xn--/', $domain ) ) { - $has_idn = true; - break; + return true; } } - return $has_idn; + return false; } } From 3422885107ea029e0734090fbfcb2f9c29f5c7a7 Mon Sep 17 00:00:00 2001 From: Pierre Gordon <16200219+pierlon@users.noreply.github.com> Date: Wed, 13 May 2020 17:53:17 -0400 Subject: [PATCH 4/4] Update src/Admin/SiteHealth.php Make `is_intl_extension_needed()` private Co-authored-by: Weston Ruter --- src/Admin/SiteHealth.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Admin/SiteHealth.php b/src/Admin/SiteHealth.php index ed9f19624cc..987ec3076d6 100644 --- a/src/Admin/SiteHealth.php +++ b/src/Admin/SiteHealth.php @@ -589,7 +589,7 @@ public function add_styles() { * * @return bool True if the `intl` extension is needed, otherwise false. */ - public function is_intl_extension_needed() { + private function is_intl_extension_needed() { // Publisher's own origins. $domains = array_unique( [