diff --git a/.github/workflows/build-test-measure.yml b/.github/workflows/build-test-measure.yml index 5e09eace199..9464f0d898b 100644 --- a/.github/workflows/build-test-measure.yml +++ b/.github/workflows/build-test-measure.yml @@ -542,6 +542,10 @@ jobs: if: needs.pre-run.outputs.changed-php-count > 0 run: cp -r "$PWD" "$WP_CORE_DIR/src/wp-content/plugins/amp" + - name: Setup required WP constants + run: | + echo "WP_ENVIRONMENT_TYPE=local" >> $GITHUB_ENV + - name: Run tests if: ${{ matrix.coverage == false && needs.pre-run.outputs.changed-php-count > 0 }} run: vendor/bin/phpunit --verbose diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index cdd40a28278..1844bb0bf06 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1450,6 +1450,16 @@ function amp_is_dev_mode() { ( is_admin_bar_showing() && is_user_logged_in() ) || is_customize_preview() + || + ( + ! is_ssl() + && + function_exists( 'wp_get_environment_type' ) + && + 'local' === wp_get_environment_type() + && + 'localhost' === wp_parse_url( home_url(), PHP_URL_HOST ) + ) ) ); } @@ -1522,6 +1532,7 @@ function amp_get_content_sanitizers( $post = null ) { AMP_Theme_Support::TRANSITIONAL_MODE_SLUG === AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) ); + $is_dev_mode = amp_is_dev_mode(); $native_img_used = amp_is_native_img_used(); $sanitizers = [ @@ -1590,7 +1601,8 @@ function amp_get_content_sanitizers( $post = null ) { AMP_Accessibility_Sanitizer::class => [], // Note: This validating sanitizer must come at the end to clean up any remaining issues the other sanitizers didn't catch. AMP_Tag_And_Attribute_Sanitizer::class => [ - 'prefer_bento' => amp_is_bento_enabled(), + 'prefer_bento' => amp_is_bento_enabled(), + 'allow_localhost_http_protocol' => $is_dev_mode, ], ]; @@ -1651,7 +1663,7 @@ function amp_get_content_sanitizers( $post = null ) { */ $sanitizers = apply_filters( 'amp_content_sanitizers', $sanitizers, $post ); - if ( amp_is_dev_mode() ) { + if ( $is_dev_mode ) { /** * Filters the XPath queries for elements that should be enabled for dev mode. * @@ -1696,6 +1708,11 @@ function amp_get_content_sanitizers( $post = null ) { ); } + // Mark the manifest output by PWA plugin as being in dev mode. + if ( ! is_ssl() && 'localhost' === $parsed_home_url['host'] ) { + $dev_mode_xpaths[] = '//link[@rel="manifest" and contains(@href, "web-app-manifest")]'; + } + $sanitizers = array_merge( [ AMP_Dev_Mode_Sanitizer::class => [ diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php index d8ff0a9d0ce..47069101e68 100644 --- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php +++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php @@ -92,6 +92,13 @@ class AMP_Tag_And_Attribute_Sanitizer extends AMP_Base_Sanitizer { const SPECIFIED_LAYOUT_INVALID = 'SPECIFIED_LAYOUT_INVALID'; const WRONG_PARENT_TAG = 'WRONG_PARENT_TAG'; + /** + * Key for localhost. + * + * @var string + */ + const LOCALHOST = 'localhost'; + /** * Allowed tags. * @@ -184,6 +191,7 @@ public function __construct( $dom, $args = [] ) { 'amp_globally_allowed_attributes' => AMP_Allowed_Tags_Generated::get_allowed_attributes(), 'amp_layout_allowed_attributes' => AMP_Allowed_Tags_Generated::get_layout_attributes(), 'prefer_bento' => false, + 'allow_localhost_http_protocol' => false, ]; parent::__construct( $dom, $args ); @@ -1939,7 +1947,9 @@ private function check_attr_spec_rule_allowed_protocol( DOMElement $node, $attr_ if ( $node->hasAttribute( $attr_name ) ) { foreach ( $this->extract_attribute_urls( $node->getAttributeNode( $attr_name ) ) as $url ) { $url_scheme = $this->parse_protocol( $this->normalize_url_from_attribute_value( $url ) ); - if ( isset( $url_scheme ) && ! in_array( strtolower( $url_scheme ), $attr_spec_rule[ AMP_Rule_Spec::VALUE_URL ][ AMP_Rule_Spec::ALLOWED_PROTOCOL ], true ) ) { + if ( $this->args['allow_localhost_http_protocol'] && self::LOCALHOST === wp_parse_url( $url, PHP_URL_HOST ) ) { + return AMP_Rule_Spec::PASS; + } elseif ( isset( $url_scheme ) && ! in_array( strtolower( $url_scheme ), $attr_spec_rule[ AMP_Rule_Spec::VALUE_URL ][ AMP_Rule_Spec::ALLOWED_PROTOCOL ], true ) ) { return AMP_Rule_Spec::FAIL; } } @@ -1951,7 +1961,9 @@ private function check_attr_spec_rule_allowed_protocol( DOMElement $node, $attr_ if ( $node->hasAttribute( $alternative_name ) ) { foreach ( $this->extract_attribute_urls( $node->getAttributeNode( $alternative_name ), $attr_name ) as $url ) { $url_scheme = $this->parse_protocol( $this->normalize_url_from_attribute_value( $url ) ); - if ( isset( $url_scheme ) && ! in_array( strtolower( $url_scheme ), $attr_spec_rule[ AMP_Rule_Spec::VALUE_URL ][ AMP_Rule_Spec::ALLOWED_PROTOCOL ], true ) ) { + if ( $this->args['allow_localhost_http_protocol'] && self::LOCALHOST === wp_parse_url( $url, PHP_URL_HOST ) ) { + return AMP_Rule_Spec::PASS; + } elseif ( isset( $url_scheme ) && ! in_array( strtolower( $url_scheme ), $attr_spec_rule[ AMP_Rule_Spec::VALUE_URL ][ AMP_Rule_Spec::ALLOWED_PROTOCOL ], true ) ) { return AMP_Rule_Spec::FAIL; } } diff --git a/package.json b/package.json index 7da3f9d2b89..cb3e8717097 100644 --- a/package.json +++ b/package.json @@ -144,6 +144,7 @@ "test:js:update-snapshots": "npm run test:js -- --updateSnapshot", "test:js:watch": "npm run test:js -- --watch", "test:php": "wp-env run phpunit 'env WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php WORDPRESS_TABLE_PREFIX=wptests_ WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", + "test:php:coverage": "wp-env run tests-wordpress 'env WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist --coverage-html /var/www/html/wp-content/plugins/amp/coverage'", "test:php:multisite": "wp-env run phpunit 'env WP_MULTISITE=1 WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php WORDPRESS_TABLE_PREFIX=wptests_ WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", "test:php:xdebug": "wp-env run tests-wordpress 'env PHP_IDE_CONFIG=serverName=localhost WORDPRESS_TABLE_PREFIX=wptests_ WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", "test:php:help": "npm run test:php --args='--help'", @@ -159,4 +160,4 @@ } }, "title": "AMP for WordPress" -} +} \ No newline at end of file diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 4e21a562bde..5e8e2a56f53 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1897,6 +1897,10 @@ public function test_amp_get_content_embed_handlers() { public function test_amp_is_dev_mode() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG ); + // Enabale HTTPS to short circuit `amp_is_dev_mode()` in multiste tests which depends on wp-env. + // We require it because wp-env already meets the prerequisites for local development. + $_SERVER['HTTPS'] = 'on'; + $this->assertFalse( amp_is_dev_mode() ); add_filter( 'amp_dev_mode_enabled', '__return_true' ); $this->assertTrue( amp_is_dev_mode() ); @@ -1916,6 +1920,26 @@ public function test_amp_is_dev_mode() { $this->assertFalse( is_user_logged_in() ); $this->assertTrue( is_admin_bar_showing() ); $this->assertFalse( amp_is_dev_mode() ); + + if ( function_exists( 'wp_get_environment_type' ) ) { + // Just to ensure is_ssl() is false. + $_SERVER['HTTPS'] = false; + + add_filter( + 'home_url', + static function () { + return 'http://localhost'; + } + ); + + $this->assertFalse( is_ssl() ); + $this->assertTrue( amp_is_dev_mode() ); + $this->assertEquals( 'local', wp_get_environment_type() ); + $this->assertTrue( 'localhost' === wp_parse_url( home_url(), PHP_URL_HOST ) ); + } else { + $this->assertFalse( amp_is_dev_mode() ); + $this->assertFalse( function_exists( 'wp_get_environment_type' ) ); + } } /** @@ -1958,6 +1982,10 @@ public function test_amp_get_content_sanitizers() { $post = self::factory()->post->create_and_get(); add_filter( 'amp_content_sanitizers', [ $this, 'capture_filter_call' ], 10, 2 ); + // Enabale HTTPS to short circuit `amp_is_dev_mode()` in multiste tests which depends on wp-env. + // We require it because wp-env already meets the prerequisites for local development. + $_SERVER['HTTPS'] = 'on'; + $this->last_filter_call = null; AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG ); $handlers = amp_get_content_sanitizers(); @@ -2034,6 +2062,10 @@ function ( $xpaths ) use ( $element_xpaths ) { } ); + // Enabale HTTPS to short circuit `amp_is_dev_mode()` in multiste tests which depends on wp-env. + // We require it because wp-env already meets the prerequisites for local development. + $_SERVER['HTTPS'] = 'on'; + // Check that AMP_Dev_Mode_Sanitizer is not registered if not in dev mode. $sanitizers = amp_get_content_sanitizers(); $this->assertFalse( amp_is_dev_mode() ); @@ -2091,6 +2123,13 @@ public function test_amp_get_content_sanitizers_with_post_preview() { ); $this->go_to( get_preview_post_link( $post ) ); + add_filter( + 'home_url', + static function () { + return 'http://localhost'; + } + ); + $sanitizers = amp_get_content_sanitizers(); $this->assertTrue( is_admin_bar_showing() ); $this->assertTrue( amp_is_dev_mode() ); @@ -2102,6 +2141,7 @@ public function test_amp_get_content_sanitizers_with_post_preview() { '//*[ @id = "wpadminbar" ]//*', '//style[ @id = "admin-bar-inline-css" ]', '//script[ not( @src ) and contains( text(), "document.location.search" ) and contains( text(), "preview=true" ) and contains( text(), "unload" ) and contains( text(), "window.name" ) and contains( text(), "wp-preview-' . $post . '" ) ]', + '//link[@rel="manifest" and contains(@href, "web-app-manifest")]', ], $sanitizers[ AMP_Dev_Mode_Sanitizer::class ]['element_xpaths'] ); diff --git a/tests/php/test-amp-tag-and-attribute-sanitizer-private-methods.php b/tests/php/test-amp-tag-and-attribute-sanitizer-private-methods.php index a67a78f4995..3b3d853ddd5 100644 --- a/tests/php/test-amp-tag-and-attribute-sanitizer-private-methods.php +++ b/tests/php/test-amp-tag-and-attribute-sanitizer-private-methods.php @@ -5,6 +5,8 @@ use AmpProject\AmpWP\Tests\Helpers\PrivateAccess; use AmpProject\AmpWP\Tests\TestCase; +use AmpProject\Html\Attribute; +use AmpProject\Validator\Spec\Tag\Html; class AMP_Tag_And_Attribute_Sanitizer_Attr_Spec_Rules_Test extends TestCase { @@ -1788,6 +1790,183 @@ public function test_check_attr_spec_rule_allowed_protocol( $data, $expected ) { $this->assertEquals( $expected, $got, sprintf( "using source: %s\n%s", $data['source'], wp_json_encode( $data ) ) ); } + /** @return array */ + public function get_check_attr_spec_rule_allowed_protocol_with_allow_localhost_http_protocol() { + return [ + 'protocol_pass_and_allow_localhost_http_protocol' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::PASS, + true, + ], + 'protocol_alternative_pass_and_allow_localhost_http_protocol' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'alternative_names' => [ + 'attribute1_alternative1', + ], + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::PASS, + true, + ], + 'protocol_pass_and_allow_localhost_http_protocol_with_cross_origin_url' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + true, + ], + 'protocol_alternative_pass_and_allow_localhost_http_protocol_with_cross_origin_url' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'alternative_names' => [ + 'attribute1_alternative1', + ], + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + true, + ], + 'protocol_pass_and_disallow_localhost_http_protocol' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + false, + ], + 'protocol_alternative_pass_and_disallow_localhost_http_protocol' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'alternative_names' => [ + 'attribute1_alternative1', + ], + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + false, + ], + 'protocol_pass_and_disallow_localhost_http_protocol_with_cross_origin_url' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + false, + ], + 'protocol_alternative_pass_and_disallow_localhost_http_protocol_with_cross_origin_url' => [ + [ + 'source' => '
', + 'node_tag_name' => 'div', + 'attr_name' => 'attribute1', + 'attr_spec_rule' => [ + 'alternative_names' => [ + 'attribute1_alternative1', + ], + 'value_url' => [ + 'protocol' => [ + 'https', + ], + ], + ], + ], + AMP_Rule_Spec::FAIL, + false, + ], + ]; + } + + /** + * Test `check_attr_spec_rule_allowed_protocol()` with `allow_localhost_http_protocol` arg. + * + * @dataProvider get_check_attr_spec_rule_allowed_protocol_with_allow_localhost_http_protocol + * + * @covers AMP_Tag_And_Attribute_Sanitizer::check_attr_spec_rule_allowed_protocol() + * @group allowed-tags-private-methods + * + * @param array $data The data. + * @param int $expected The expected result. + * @param bool $allow_localhost_http_protocol Whether to allow localhost http protocol. + */ + public function test_check_attr_spec_rule_allowed_protocol_with_allow_localhost_http_protocol( + $data, + $expected, + $allow_localhost_http_protocol + ) { + $dom = AMP_DOM_Utils::get_dom_from_content( $data['source'] ); + $node = $dom->getElementsByTagName( $data['node_tag_name'] )->item( 0 ); + $sanitizer = new AMP_Tag_And_Attribute_Sanitizer( + $dom, + [ + 'allow_localhost_http_protocol' => $allow_localhost_http_protocol, + ] + ); + + $got = $this->call_private_method( $sanitizer, 'check_attr_spec_rule_allowed_protocol', [ $node, $data['attr_name'], $data['attr_spec_rule'] ] ); + + $this->assertEquals( $expected, $got, sprintf( "using source: %s\n%s", $data['source'], wp_json_encode( $data ) ) ); + } + public function get_check_attr_spec_rule_disallowed_relative() { return [ 'no_attributes' => [