From d11e0e08af557e368216b3d9410a8827c8ccf4b9 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Tue, 8 Mar 2022 23:04:20 +0100 Subject: [PATCH 1/8] Replace `img` with `amp-pixel` when dealing with Facebook tracking pixel --- .../sanitizers/class-amp-img-sanitizer.php | 30 +++++++++++ tests/php/test-amp-img-sanitizer.php | 53 ++++++++++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 77666faa747..f0b629bc100 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -81,6 +81,7 @@ public function get_selector_conversion_mapping() { Tag::IMG => [ 'amp-img', 'amp-anim', + 'amp-pixel', ], ]; } @@ -183,6 +184,20 @@ public function sanitize() { continue; } + // Replace img with amp-pixel when dealing with tracking pixels. + if ( self::is_tracking_pixel_url( $node->getAttribute( Attribute::SRC ) ) ) { + $amp_pixel_node = AMP_DOM_Utils::create_node( + $this->dom, + 'amp-pixel', + [ + Attribute::SRC => $node->getAttribute( Attribute::SRC ), + Attribute::LAYOUT => Layout::NODISPLAY, + ] + ); + $node->parentNode->replaceChild( $amp_pixel_node, $node ); + continue; + } + // Short-circuit emoji images from needing to make requests out to https://s.w.org/. if ( 'wp-smiley' === $node->getAttribute( Attribute::CLASS_ ) ) { $node->setAttribute( Attribute::WIDTH, '72' ); @@ -552,4 +567,19 @@ private function is_gif_url( $url ) { $path = wp_parse_url( $url, PHP_URL_PATH ); return substr( $path, -strlen( $ext ) ) === $ext; } + + /** + * Determines if a URL is a known tracking pixel URL. + * + * Currently, only Facebook tracking pixel URL is detected. + * + * @since 2.2.2 + * + * @param string $url URL to inspect. + * + * @return bool Returns true if $url is a tracking pixel URL. + */ + private static function is_tracking_pixel_url( $url ) { + return (bool) preg_match( '/http(?>s)?:\/\/(?>www\.)?facebook\.com\/tr\?/i', $url ); + } } diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index c2d083ac4b2..59a35e376c0 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -48,13 +48,13 @@ public function test_get_selector_conversion_mapping() { $with_defaults = new AMP_Img_Sanitizer( $dom ); $this->assertEquals( - [ 'img' => [ 'amp-img', 'amp-anim' ] ], + [ 'img' => [ 'amp-img', 'amp-anim', 'amp-pixel' ] ], $with_defaults->get_selector_conversion_mapping() ); $with_false_native_used = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => false ] ); $this->assertEquals( - [ 'img' => [ 'amp-img', 'amp-anim' ] ], + [ 'img' => [ 'amp-img', 'amp-anim', 'amp-pixel' ] ], $with_false_native_used->get_selector_conversion_mapping() ); @@ -575,6 +575,11 @@ public function get_data() { AMP_Tag_And_Attribute_Sanitizer::WRONG_PARENT_TAG, ], ], + + 'facebook_pixel_img_to_amp_pixel' => [ + 'fbpx', + '', + ], ]; } @@ -923,4 +928,48 @@ public function test_process_picture_elements( $input, $args, $expected ) { $this->assertEqualMarkup( $expected, $actual, "Actual content:\n$actual" ); } + + /** + * @return array + */ + public function get_data_for_test_is_tracking_pixel_url() { + return [ + 'facebook_pixel_url_no_ssl_no_www' => [ + 'url' => 'http://facebook.com/tr?id=123456789012345', + 'expected' => true, + ], + 'facebook_pixel_url_no_ssl_www' => [ + 'url' => 'http://www.facebook.com/tr?id=123456789012345', + 'expected' => true, + ], + 'facebook_pixel_url_ssl_no_www' => [ + 'url' => 'https://facebook.com/tr?id=123456789012345', + 'expected' => true, + ], + 'facebook_pixel_url_ssl_www' => [ + 'url' => 'https://www.facebook.com/tr?id=123456789012345', + 'expected' => true, + ], + 'facebook_page_url' => [ + 'url' => 'https://www.facebook.com/traffic?id=123456789012345', + 'expected' => false, + ], + 'relative_url' => [ + 'url' => '/tr?id=123456789012345', + 'expected' => false, + ], + ]; + } + + /** + * @dataProvider get_data_for_test_is_tracking_pixel_url() + * + * @covers ::is_tracking_pixel_url() + */ + public function test_is_tracking_pixel_url( $url, $expected ) { + $this->assertEquals( + $expected, + $this->call_private_static_method( 'AMP_Img_Sanitizer', 'is_tracking_pixel_url', [ $url ] ) + ); + } } From 69df5bf68666dc6492f7637f2daae890e99e8b02 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:24:24 +0100 Subject: [PATCH 2/8] Remove `amp-pixel` from selector conversion mapping In practice a Facebook pixel should really never be styled by authors. It's supposed to be invisible after all. Co-authored-by: Weston Ruter --- includes/sanitizers/class-amp-img-sanitizer.php | 1 - tests/php/test-amp-img-sanitizer.php | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index f0b629bc100..1d3c06da6ec 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -81,7 +81,6 @@ public function get_selector_conversion_mapping() { Tag::IMG => [ 'amp-img', 'amp-anim', - 'amp-pixel', ], ]; } diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index 59a35e376c0..6edf03c87dd 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -48,13 +48,13 @@ public function test_get_selector_conversion_mapping() { $with_defaults = new AMP_Img_Sanitizer( $dom ); $this->assertEquals( - [ 'img' => [ 'amp-img', 'amp-anim', 'amp-pixel' ] ], + [ 'img' => [ 'amp-img', 'amp-anim' ] ], $with_defaults->get_selector_conversion_mapping() ); $with_false_native_used = new AMP_Img_Sanitizer( $dom, [ 'native_img_used' => false ] ); $this->assertEquals( - [ 'img' => [ 'amp-img', 'amp-anim', 'amp-pixel' ] ], + [ 'img' => [ 'amp-img', 'amp-anim' ] ], $with_false_native_used->get_selector_conversion_mapping() ); From 4fccf4e8ec4f96d4e59a0bddcedcf04f69f34809 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:25:35 +0100 Subject: [PATCH 3/8] Examine parsed URL instead of using regex Co-authored-by: Weston Ruter --- includes/sanitizers/class-amp-img-sanitizer.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 1d3c06da6ec..7a311089218 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -579,6 +579,7 @@ private function is_gif_url( $url ) { * @return bool Returns true if $url is a tracking pixel URL. */ private static function is_tracking_pixel_url( $url ) { - return (bool) preg_match( '/http(?>s)?:\/\/(?>www\.)?facebook\.com\/tr\?/i', $url ); + $parsed_url = wp_parse_url( $url ); + return isset( $parsed_url['host'], $parsed_url['path'] ) && 'facebook.com' === str_replace( 'www.', $parsed_url['host'] ) && '/tr' === $parsed_url['path']; } } From 0b6f47429388333edcb76e35c1f6a419ee7629b4 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:26:24 +0100 Subject: [PATCH 4/8] Use `::class` constant instead of hardcoded class name Co-authored-by: Weston Ruter --- tests/php/test-amp-img-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index 6edf03c87dd..f10d5a4f3e9 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -969,7 +969,7 @@ public function get_data_for_test_is_tracking_pixel_url() { public function test_is_tracking_pixel_url( $url, $expected ) { $this->assertEquals( $expected, - $this->call_private_static_method( 'AMP_Img_Sanitizer', 'is_tracking_pixel_url', [ $url ] ) + $this->call_private_static_method( AMP_Img_Sanitizer::class, 'is_tracking_pixel_url', [ $url ] ) ); } } From 9d923e3b6fe10af8aa752f9a77c0a5dd58d93ce7 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:28:29 +0100 Subject: [PATCH 5/8] Add missing replacement string to `str_replace` and reformat --- includes/sanitizers/class-amp-img-sanitizer.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 7a311089218..47a68cd8752 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -580,6 +580,13 @@ private function is_gif_url( $url ) { */ private static function is_tracking_pixel_url( $url ) { $parsed_url = wp_parse_url( $url ); - return isset( $parsed_url['host'], $parsed_url['path'] ) && 'facebook.com' === str_replace( 'www.', $parsed_url['host'] ) && '/tr' === $parsed_url['path']; + + return ( + isset( $parsed_url['host'], $parsed_url['path'] ) + && + 'facebook.com' === str_replace( 'www.', '', $parsed_url['host'] ) + && + '/tr' === $parsed_url['path'] + ); } } From 0d66f3752f63fded68a729a87b425cd058a7aa57 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:29:51 +0100 Subject: [PATCH 6/8] Use `Extension` class constant for getting `amp-pixel` tag name --- includes/sanitizers/class-amp-img-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index 47a68cd8752..b0055898e6b 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -187,7 +187,7 @@ public function sanitize() { if ( self::is_tracking_pixel_url( $node->getAttribute( Attribute::SRC ) ) ) { $amp_pixel_node = AMP_DOM_Utils::create_node( $this->dom, - 'amp-pixel', + Extension::PIXEL, [ Attribute::SRC => $node->getAttribute( Attribute::SRC ), Attribute::LAYOUT => Layout::NODISPLAY, From 2c74649d423ad47c1674d20617adbc28e9551631 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 9 Mar 2022 10:54:46 +0100 Subject: [PATCH 7/8] Remove redundant space --- includes/sanitizers/class-amp-img-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index b0055898e6b..bbf0ab12fb3 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -584,7 +584,7 @@ private static function is_tracking_pixel_url( $url ) { return ( isset( $parsed_url['host'], $parsed_url['path'] ) && - 'facebook.com' === str_replace( 'www.', '', $parsed_url['host'] ) + 'facebook.com' === str_replace( 'www.', '', $parsed_url['host'] ) && '/tr' === $parsed_url['path'] ); From e40d370cd03e06b0f86835a92822c21b979930d1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 9 Mar 2022 09:25:53 -0800 Subject: [PATCH 8/8] Copy referrerpolicy attribute from img to amp-pixel --- includes/sanitizers/class-amp-img-sanitizer.php | 14 ++++++++++---- tests/php/test-amp-img-sanitizer.php | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/includes/sanitizers/class-amp-img-sanitizer.php b/includes/sanitizers/class-amp-img-sanitizer.php index bbf0ab12fb3..9188ac2f05e 100644 --- a/includes/sanitizers/class-amp-img-sanitizer.php +++ b/includes/sanitizers/class-amp-img-sanitizer.php @@ -185,13 +185,19 @@ public function sanitize() { // Replace img with amp-pixel when dealing with tracking pixels. if ( self::is_tracking_pixel_url( $node->getAttribute( Attribute::SRC ) ) ) { + $attributes = [ + Attribute::SRC => $node->getAttribute( Attribute::SRC ), + Attribute::LAYOUT => Layout::NODISPLAY, + ]; + foreach ( [ Attribute::REFERRERPOLICY ] as $allowed_attribute ) { + if ( $node->hasAttribute( $allowed_attribute ) ) { + $attributes[ $allowed_attribute ] = $node->getAttribute( $allowed_attribute ); + } + } $amp_pixel_node = AMP_DOM_Utils::create_node( $this->dom, Extension::PIXEL, - [ - Attribute::SRC => $node->getAttribute( Attribute::SRC ), - Attribute::LAYOUT => Layout::NODISPLAY, - ] + $attributes ); $node->parentNode->replaceChild( $amp_pixel_node, $node ); continue; diff --git a/tests/php/test-amp-img-sanitizer.php b/tests/php/test-amp-img-sanitizer.php index f10d5a4f3e9..9742e32fbfb 100644 --- a/tests/php/test-amp-img-sanitizer.php +++ b/tests/php/test-amp-img-sanitizer.php @@ -580,6 +580,11 @@ public function get_data() { 'fbpx', '', ], + + 'facebook_pixel_img_to_amp_pixel_with_referrer' => [ + 'fbpx', + '', + ], ]; }