From 4652612d4d61039bb129d199bc62240804901648 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 13:11:31 +0530 Subject: [PATCH 1/7] fix: harden profiler, replace, widget --- inc/admin.php | 2 +- inc/dashboard_widget.php | 24 +- inc/media_rename/attachment_replace.php | 6 + inc/v2/BgOptimizer/Lazyload.php | 47 +++- .../media_rename/test-attachment-replace.php | 55 +++++ tests/test-bg-selector.php | 208 ++++++++++++++++++ tests/test-dashboard-widget.php | 101 +++++++++ 7 files changed, 432 insertions(+), 11 deletions(-) create mode 100644 tests/test-bg-selector.php create mode 100644 tests/test-dashboard-widget.php diff --git a/inc/admin.php b/inc/admin.php index d1b37e76..5da317cc 100755 --- a/inc/admin.php +++ b/inc/admin.php @@ -222,7 +222,7 @@ public function check_svg_and_sanitize( $file ) { * * @return bool|int */ - protected function sanitize_svg( $file ) { + public function sanitize_svg( $file ) { // We can ignore the phpcs warning here as we're reading and writing to the Temp file. $dirty = file_get_contents( $file ); // phpcs:ignore diff --git a/inc/dashboard_widget.php b/inc/dashboard_widget.php index 2c7756ad..097e8a8b 100644 --- a/inc/dashboard_widget.php +++ b/inc/dashboard_widget.php @@ -31,7 +31,7 @@ public function init() { * Add the dashboard widget. */ public function add_dashboard_widget() { - if ( ! $this->has_at_least_ten_visits() ) { + if ( ! current_user_can( 'manage_options' ) || ! $this->has_at_least_ten_visits() ) { return; } @@ -42,7 +42,7 @@ public function add_dashboard_widget() { * Enqueue the widget assets. */ public function enqueue_widget() { - if ( ! $this->is_main_dashboard_page() || ! $this->has_at_least_ten_visits() ) { + if ( ! current_user_can( 'manage_options' ) || ! $this->is_main_dashboard_page() || ! $this->has_at_least_ten_visits() ) { return; } @@ -112,7 +112,7 @@ private function get_script_localization() { ], 'skeletonLoader' => $this->get_skeleton_loader(), 'billingURL' => tsdk_translate_link( 'https://dashboard.optimole.com/settings/billing', 'query' ), - 'serviceData' => $this->get_service_data(), + 'serviceData' => $this->get_widget_service_data(), 'assetsURL' => OPTML_URL . 'assets/', 'dashboardMetricsURL' => esc_url( 'https://dashboard.optimole.com/metrics' ), 'dashboardURL' => esc_url( tsdk_translate_link( 'https://dashboard.optimole.com' ) ), @@ -130,6 +130,24 @@ private function get_service_data() { return $settings->get( 'service_data' ); } + /** + * Get the service data fields the widget displays, keeping keys and secrets out of the page. + * + * @return array + */ + private function get_widget_service_data() { + $service_data = $this->get_service_data(); + + if ( ! is_array( $service_data ) ) { + return []; + } + + return array_intersect_key( + $service_data, + array_flip( [ 'visitors', 'visitors_pretty', 'visitors_limit', 'visitors_limit_pretty', 'compression_percentage', 'traffic' ] ) + ); + } + /** * Get the skeleton loader markup. * diff --git a/inc/media_rename/attachment_replace.php b/inc/media_rename/attachment_replace.php index d4b19733..31424def 100644 --- a/inc/media_rename/attachment_replace.php +++ b/inc/media_rename/attachment_replace.php @@ -77,6 +77,12 @@ public function replace() { return new WP_Error( 'file_error', __( 'The uploaded file type does not match the original file type.', 'optimole-wp' ) ); } + if ( $uploaded_filetype['type'] === 'image/svg+xml' ) { + if ( ! Optml_Main::instance()->admin->sanitize_svg( $this->file['tmp_name'] ) ) { + return new WP_Error( 'file_error', __( 'Error uploading file.', 'optimole-wp' ) ); + } + } + global $wp_filesystem; if ( ! $wp_filesystem->move( $this->file['tmp_name'], $original_file, true ) ) { diff --git a/inc/v2/BgOptimizer/Lazyload.php b/inc/v2/BgOptimizer/Lazyload.php index a95dd154..8b52c7ca 100644 --- a/inc/v2/BgOptimizer/Lazyload.php +++ b/inc/v2/BgOptimizer/Lazyload.php @@ -13,6 +13,25 @@ */ class Lazyload { const MARKER = '/* OPTML_VIEWPORT_BG_SELECTORS */'; + /** + * Shape of a plain selector the frontend getUniqueSelector() reports: ids, tags, classes, ' > ' and :nth-of-type(N). + */ + const SAFE_SELECTOR_PATTERN = '/^(?:[\w\-#. >\x{00A0}-\x{10FFFF}]|:nth-of-type\(\d+\))+$/u'; + + /** + * Check whether a client-reported selector is a plain selector safe to embed in the stylesheet as-is. + * + * A selector carrying CSS metacharacters (`(`, `)`, `{`, `;`, `:` beyond nth-of-type, `/`, …) is not + * embedded: the browser already discards the whole rule when it sees one, so treating it as unsafe + * keeps the current behaviour while making CSS injection impossible. + * + * @param mixed $selector The selector to check. + * + * @return bool Whether the selector matches the plain-selector shape. + */ + public static function is_safe_selector( $selector ): bool { + return is_string( $selector ) && $selector !== '' && preg_match( self::SAFE_SELECTOR_PATTERN, $selector ) === 1; + } /** * Get the current personalized CSS for lazy loading. * @@ -41,6 +60,8 @@ public static function get_personalized_css( $data ) { do_action( 'optml_log', 'LCP data: ' . $device . ' ' . print_r( $lcp_data, true ) ); } $css_selectors[ $device ] = []; + // Discard the whole rule if a selector is unsafe. + $device_tainted = false; foreach ( $personalized_selectors as $selector => $above_fold_selectors ) { if ( ! isset( $lazyload_selectors[ $selector ] ) ) { continue; @@ -49,7 +70,11 @@ public static function get_personalized_css( $data ) { $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(.optml-bg-lazyloaded)'; } else { foreach ( $above_fold_selectors as $above_fold_selector => $bg_urls ) { - $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(' . strip_tags( $above_fold_selector ) . '):not(.optml-bg-lazyloaded)'; + if ( ! self::is_safe_selector( $above_fold_selector ) ) { + $device_tainted = true; + break 2; + } + $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(' . $above_fold_selector . '):not(.optml-bg-lazyloaded)'; } } } @@ -58,17 +83,25 @@ public static function get_personalized_css( $data ) { $css_selectors[ $device ] = array_unique( $css_selectors[ $device ] ); if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' ) { if ( ! empty( $lcp_data['bgSelector'] ) ) { - $css_selectors[ $device ] = array_map( - function ( $selector ) use ( $lcp_data ) { - return $selector . ':not(' . strip_tags( $lcp_data['bgSelector'] ) . ')'; - }, - $css_selectors[ $device ] - ); + if ( self::is_safe_selector( $lcp_data['bgSelector'] ) ) { + $css_selectors[ $device ] = array_map( + function ( $selector ) use ( $lcp_data ) { + return $selector . ':not(' . $lcp_data['bgSelector'] . ')'; + }, + $css_selectors[ $device ] + ); + } else { + $device_tainted = true; + } } if ( ! empty( $lcp_data['bgUrls'] ) ) { $preload_urls[ $device ] = array_merge( $preload_urls[ $device ], $lcp_data['bgUrls'] ); } } + + if ( $device_tainted ) { + $css_selectors[ $device ] = []; + } } if ( OPTML_DEBUG ) { do_action( 'optml_log', 'BGCSS selectors: ' . print_r( $css_selectors, true ) ); diff --git a/tests/media_rename/test-attachment-replace.php b/tests/media_rename/test-attachment-replace.php index e643e97f..3d9a5425 100644 --- a/tests/media_rename/test-attachment-replace.php +++ b/tests/media_rename/test-attachment-replace.php @@ -268,6 +268,61 @@ public function chmod( $file, $mode = false, $recursive = false ) { }; } + /** + * Replacing an SVG must run the same sanitizer as a normal upload. + */ + public function test_replace_sanitizes_svg() { + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + + $tmp_file = self::FILESTASH . 'replace-scripted.svg'; + file_put_contents( $tmp_file, '' ); + + $replacer = new Optml_Attachment_Replace( + $id, + [ + 'name' => 'replace-scripted.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $this->assertTrue( $replacer->replace(), 'Replacement operation failed.' ); + + $contents = file_get_contents( $file_path ); + $this->assertStringContainsString( 'assertStringNotContainsString( 'assertStringNotContainsString( 'onload', $contents ); + + wp_delete_post( $id, true ); + } + + /** + * An SVG the sanitizer cannot parse must not replace the original file. + */ + public function test_replace_rejects_unsanitizable_svg() { + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + $original = file_get_contents( $file_path ); + + $tmp_file = self::FILESTASH . 'replace-broken.svg'; + file_put_contents( $tmp_file, 'not 'replace-broken.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $this->assertWPError( $replacer->replace() ); + $this->assertSame( $original, file_get_contents( $file_path ), 'Original SVG was overwritten.' ); + + wp_delete_post( $id, true ); + } + private function do_replace_test( $id_to_replace, $replace_file, $source_scaled, $result_scaled ) { // Removed var_dump diff --git a/tests/test-bg-selector.php b/tests/test-bg-selector.php new file mode 100644 index 00000000..3565e60d --- /dev/null +++ b/tests/test-bg-selector.php @@ -0,0 +1,208 @@ +update( + 'service_data', + [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.com' ], + ] + ); + $settings->update( 'lazyload', 'enabled' ); + $settings->update( 'lazyload_type', 'viewport' ); + $settings->update( 'bg_replacer', 'enabled' ); + Optml_Lazyload_Replacer::instance()->init(); + $this->reset_background_selectors(); + } + + /** + * Clean up the test environment after each test. + */ + public function tearDown(): void { + $this->reset_background_selectors(); + Profile::reset_current_profile(); + parent::tearDown(); + } + + /** + * Selectors produced by the frontend getUniqueSelector() are safe to embed. + * + * @dataProvider safe_selectors + * + * @param string $selector Selector to check. + */ + public function test_is_safe_selector_accepts_generated_selectors( $selector ) { + $this->assertTrue( Lazyload::is_safe_selector( $selector ) ); + } + + /** + * Plain selectors getUniqueSelector() emits. + * + * @return array + */ + public function safe_selectors() { + return [ + 'body' => [ 'body' ], + 'id' => [ '#hero' ], + 'body child' => [ 'body > div.wp-block-cover' ], + 'nested nth' => [ '#main > section.hero.is-dark:nth-of-type(2) > div.inner_wrap' ], + 'non ascii class' => [ 'body > div.café' ], + ]; + } + + /** + * Selectors carrying CSS metacharacters are treated as unsafe. + * + * @dataProvider unsafe_selectors + * + * @param mixed $selector Selector to check. + */ + public function test_is_safe_selector_rejects_metacharacters( $selector ) { + $this->assertFalse( Lazyload::is_safe_selector( $selector ) ); + } + + /** + * Selectors that must not be embedded (injection or CSS-invalid, including Tailwind's `:` and `/`). + * + * @return array + */ + public function unsafe_selectors() { + return [ + 'issue payload' => [ self::PAYLOAD ], + 'tailwind colon' => [ 'body > div.md:flex' ], + 'tailwind slash' => [ 'body > div.w-1/2' ], + 'closing paren' => [ 'div)' ], + 'selector list' => [ 'div, body' ], + 'declaration' => [ 'div;color:red' ], + 'braces' => [ 'div{}' ], + 'attribute' => [ 'div[style]' ], + 'quote' => [ 'a"b' ], + 'newline' => [ "a\nb" ], + 'empty' => [ '' ], + 'array' => [ [ 'div' ] ], + ]; + } + + /** + * An injected above-fold selector voids the device rule instead of leaking CSS. + */ + public function test_personalized_css_drops_injected_selectors() { + $device_data = [ + 'bg' => [ + self::WATCHER => [ + self::PAYLOAD => [], + ], + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + $this->assertSame( '', $css ); + } + + /** + * An injected LCP selector voids the device rule. + */ + public function test_personalized_css_drops_injected_lcp_selector() { + $device_data = [ + 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], + 'lcp' => [ + 'type' => 'bg', + 'bgSelector' => self::PAYLOAD, + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertSame( '', $css ); + } + + /** + * A page with only plain selectors still renders exactly as before. + */ + public function test_personalized_css_keeps_plain_selectors() { + $device_data = [ + 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], + 'lcp' => [ + 'type' => 'bg', + 'bgSelector' => 'body > div.hero:nth-of-type(1)', + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringContainsString( 'html ' . self::WATCHER . ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + $this->assertStringContainsString( ':not(body > div.hero:nth-of-type(1))', $css ); + $this->assertStringContainsString( '{ background-image: none !important; }', $css ); + } + + /** + * A watcher with no above-fold elements still hides all its matches (unchanged behaviour). + */ + public function test_personalized_css_hides_all_when_no_above_fold() { + $device_data = [ + 'bg' => [ self::WATCHER => [] ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertSame( 'html ' . self::WATCHER . ':not(.optml-bg-lazyloaded) { background-image: none !important; }', $css ); + } + + /** + * Reset the cached background lazyload selectors. + */ + private function reset_background_selectors() { + $property = new ReflectionProperty( Optml_Lazyload_Replacer::class, 'background_lazyload_selectors' ); + $property->setAccessible( true ); + $property->setValue( null, null ); + } +} diff --git a/tests/test-dashboard-widget.php b/tests/test-dashboard-widget.php new file mode 100644 index 00000000..1f86d260 --- /dev/null +++ b/tests/test-dashboard-widget.php @@ -0,0 +1,101 @@ +update( + 'service_data', + [ + 'cdn_key' => 'test-cdn-key', + 'cdn_secret' => 'test-cdn-secret', + 'api_key' => 'test-api-key', + 'visitors' => 20, + 'visitors_pretty' => '20', + 'visitors_limit' => 5000, + 'visitors_limit_pretty' => '5k', + 'compression_percentage' => 42.5, + 'traffic' => 12.3, + ] + ); + set_current_screen( 'dashboard' ); + } + + /** + * Clean up the test environment after each test. + */ + public function tearDown(): void { + global $wp_meta_boxes; + unset( $wp_meta_boxes['dashboard'] ); + wp_deregister_script( self::HANDLE ); + set_current_screen( 'front' ); + wp_set_current_user( 0 ); + parent::tearDown(); + } + + /** + * Subscribers get neither the widget nor its localized account data. + */ + public function test_widget_hidden_from_subscriber() { + global $pagenow, $wp_meta_boxes; + wp_set_current_user( self::factory()->user->create( [ 'role' => 'subscriber' ] ) ); + + $widget = new Optml_Dashboard_Widget(); + $widget->add_dashboard_widget(); + + $previous_pagenow = $pagenow; + $pagenow = 'index.php'; + $widget->enqueue_widget(); + $pagenow = $previous_pagenow; + + $this->assertFalse( isset( $wp_meta_boxes['dashboard']['normal']['core'][ self::HANDLE ] ) ); + $this->assertFalse( wp_script_is( self::HANDLE, 'registered' ) ); + } + + /** + * Administrators still get the widget. + */ + public function test_widget_shown_to_administrator() { + global $wp_meta_boxes; + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + + ( new Optml_Dashboard_Widget() )->add_dashboard_widget(); + + $this->assertTrue( isset( $wp_meta_boxes['dashboard']['normal']['core'][ self::HANDLE ] ) ); + } + + /** + * The localized service data only holds the stats the widget displays. + */ + public function test_localized_service_data_excludes_credentials() { + $method = new ReflectionMethod( Optml_Dashboard_Widget::class, 'get_script_localization' ); + $method->setAccessible( true ); + + $service_data = $method->invoke( new Optml_Dashboard_Widget() )['serviceData']; + + $this->assertEqualsCanonicalizing( + [ 'visitors', 'visitors_pretty', 'visitors_limit', 'visitors_limit_pretty', 'compression_percentage', 'traffic' ], + array_keys( $service_data ) + ); + $this->assertSame( 20, $service_data['visitors'] ); + } +} From eb8a69b2ce6b98e151c15e8a8015415df5ce4818 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 14:14:31 +0530 Subject: [PATCH 2/7] fix: handle failed sanitizer writes --- inc/admin.php | 7 ++-- .../media_rename/test-attachment-replace.php | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/inc/admin.php b/inc/admin.php index 5da317cc..c35fa6c5 100755 --- a/inc/admin.php +++ b/inc/admin.php @@ -253,10 +253,11 @@ public function sanitize_svg( $file ) { $clean = gzencode( $clean ); } - // We can ignore the phpcs warning here as we're reading and writing to the Temp file. - file_put_contents( $file, $clean ); // phpcs:ignore + // We handle the write result below; silence the warning on I/O failure. Reading/writing the temp file is intended. + $written = @file_put_contents( $file, $clean ); // phpcs:ignore WordPress.WP.AlternativeFunctions, WordPress.PHP.NoSilencedErrors - return true; + // A failed write leaves the dirty upload in place, so report it as unsanitized. + return false !== $written; } /** diff --git a/tests/media_rename/test-attachment-replace.php b/tests/media_rename/test-attachment-replace.php index 3d9a5425..b5007b14 100644 --- a/tests/media_rename/test-attachment-replace.php +++ b/tests/media_rename/test-attachment-replace.php @@ -323,6 +323,43 @@ public function test_replace_rejects_unsanitizable_svg() { wp_delete_post( $id, true ); } + /** + * A failed sanitizer write must abort the replace and leave the original SVG untouched. + */ + public function test_replace_aborts_when_sanitizer_write_fails() { + if ( 0 === posix_getuid() ) { + $this->markTestSkipped( 'Running as root bypasses the file permission that forces the write to fail.' ); + } + + $id = self::factory()->attachment->create_upload_object( OPTML_PATH . 'tests/assets/sample.svg' ); + $file_path = ( new Optml_Attachment_Model( $id ) )->get_source_file_path(); + $original = file_get_contents( $file_path ); + + // Read-only tmp file: sanitize_svg() can read it but its file_put_contents() fails. + $tmp_file = self::FILESTASH . 'replace-readonly.svg'; + file_put_contents( $tmp_file, '' ); + chmod( $tmp_file, 0444 ); + + $replacer = new Optml_Attachment_Replace( + $id, + [ + 'name' => 'replace-readonly.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => $tmp_file, + ] + ); + + $result = $replacer->replace(); + + chmod( $tmp_file, 0644 ); + + $this->assertWPError( $result, 'A failed sanitizer write must abort the replace.' ); + $this->assertSame( $original, file_get_contents( $file_path ), 'Original SVG was overwritten despite a failed sanitize.' ); + $this->assertStringNotContainsString( ' Date: Thu, 24 Sep 2026 14:48:00 +0530 Subject: [PATCH 3/7] fix: file write validation --- inc/admin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/admin.php b/inc/admin.php index c35fa6c5..b1d1d291 100755 --- a/inc/admin.php +++ b/inc/admin.php @@ -257,7 +257,7 @@ public function sanitize_svg( $file ) { $written = @file_put_contents( $file, $clean ); // phpcs:ignore WordPress.WP.AlternativeFunctions, WordPress.PHP.NoSilencedErrors // A failed write leaves the dirty upload in place, so report it as unsanitized. - return false !== $written; + return is_string( $clean ) && strlen( $clean ) === $written; } /** From 63a51b5774012539cb5a9faecfebc87cd12659c9 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 15:55:41 +0530 Subject: [PATCH 4/7] fix: enhance personalized CSS handling for unsafe selectors --- inc/v2/BgOptimizer/Lazyload.php | 109 ++++++++++++++++++++------------ tests/test-bg-selector.php | 44 +++++++++++++ 2 files changed, 111 insertions(+), 42 deletions(-) diff --git a/inc/v2/BgOptimizer/Lazyload.php b/inc/v2/BgOptimizer/Lazyload.php index 8b52c7ca..73edb7fd 100644 --- a/inc/v2/BgOptimizer/Lazyload.php +++ b/inc/v2/BgOptimizer/Lazyload.php @@ -52,55 +52,26 @@ public static function get_personalized_css( $data ) { $lazyload_selectors = array_fill_keys( $lazyload_selectors, true ); $css_selectors = []; $preload_urls = []; + $tainted = []; foreach ( Profile::get_active_devices() as $device ) { $personalized_selectors = $data[ $device ]['bg'] ?? []; $lcp_data = $data[ $device ]['lcp'] ?? []; + // Guard against malformed profile shapes (e.g. a non-normalizing custom storage backend). + $personalized_selectors = is_array( $personalized_selectors ) ? $personalized_selectors : []; + $lcp_data = is_array( $lcp_data ) ? $lcp_data : []; if ( OPTML_DEBUG ) { do_action( 'optml_log', 'personalized_selectors: ' . $device . ' ' . print_r( $personalized_selectors, true ) ); do_action( 'optml_log', 'LCP data: ' . $device . ' ' . print_r( $lcp_data, true ) ); } - $css_selectors[ $device ] = []; - // Discard the whole rule if a selector is unsafe. - $device_tainted = false; - foreach ( $personalized_selectors as $selector => $above_fold_selectors ) { - if ( ! isset( $lazyload_selectors[ $selector ] ) ) { - continue; - } - if ( empty( $above_fold_selectors ) ) { - $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(.optml-bg-lazyloaded)'; - } else { - foreach ( $above_fold_selectors as $above_fold_selector => $bg_urls ) { - if ( ! self::is_safe_selector( $above_fold_selector ) ) { - $device_tainted = true; - break 2; - } - $css_selectors[ $device ][] = 'html ' . strip_tags( $selector ) . ':not(' . $above_fold_selector . '):not(.optml-bg-lazyloaded)'; - } - } - } - $preload_urls[ $device ] = []; - $css_selectors[ $device ] = array_unique( $css_selectors[ $device ] ); - if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' ) { - if ( ! empty( $lcp_data['bgSelector'] ) ) { - if ( self::is_safe_selector( $lcp_data['bgSelector'] ) ) { - $css_selectors[ $device ] = array_map( - function ( $selector ) use ( $lcp_data ) { - return $selector . ':not(' . $lcp_data['bgSelector'] . ')'; - }, - $css_selectors[ $device ] - ); - } else { - $device_tainted = true; - } - } - if ( ! empty( $lcp_data['bgUrls'] ) ) { - $preload_urls[ $device ] = array_merge( $preload_urls[ $device ], $lcp_data['bgUrls'] ); - } - } + $selectors = self::collect_device_selectors( $personalized_selectors, $lcp_data, $lazyload_selectors ); + // null means a client selector was unsafe, so the whole device rule is voided. + $tainted[ $device ] = null === $selectors; + $css_selectors[ $device ] = $tainted[ $device ] ? [] : $selectors; - if ( $device_tainted ) { - $css_selectors[ $device ] = []; + $preload_urls[ $device ] = []; + if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' && ! empty( $lcp_data['bgUrls'] ) ) { + $preload_urls[ $device ] = array_merge( $preload_urls[ $device ], $lcp_data['bgUrls'] ); } } if ( OPTML_DEBUG ) { @@ -113,8 +84,19 @@ function ( $selector ) use ( $lcp_data ) { } $hide_rule = ' { background-image: none !important; }'; - $mobile_selectors = implode( ',', $css_selectors[ Profile::DEVICE_TYPE_MOBILE ] ); - $desktop_selectors = implode( ',', $css_selectors[ Profile::DEVICE_TYPE_DESKTOP ] ); + $mobile = $css_selectors[ Profile::DEVICE_TYPE_MOBILE ]; + $desktop = $css_selectors[ Profile::DEVICE_TYPE_DESKTOP ]; + + // Keep the surviving rule scoped when the other device was voided. + if ( $tainted[ Profile::DEVICE_TYPE_MOBILE ] !== $tainted[ Profile::DEVICE_TYPE_DESKTOP ] ) { + if ( $tainted[ Profile::DEVICE_TYPE_MOBILE ] ) { + return empty( $desktop ) ? '' : '@media (min-width: 600px) { ' . implode( ',', $desktop ) . $hide_rule . ' }'; + } + return empty( $mobile ) ? '' : '@media (max-width: 600px) { ' . implode( ',', $mobile ) . $hide_rule . ' }'; + } + + $mobile_selectors = implode( ',', $mobile ); + $desktop_selectors = implode( ',', $desktop ); if ( $mobile_selectors === $desktop_selectors ) { return empty( $mobile_selectors ) ? '' : $mobile_selectors . $hide_rule; @@ -131,4 +113,47 @@ function ( $selector ) use ( $lcp_data ) { $media_query = '@media (max-width: 600px) { ' . $mobile_selectors . $hide_rule . ' } @media (min-width: 600px) { ' . $desktop_selectors . $hide_rule . ' }'; return $media_query; } + /** + * Build the background-hide selectors for one device. + * + * @param array> $personalized_selectors Stored bg data: watcher => [ above-fold selector => urls ]. + * @param array $lcp_data Stored LCP data for the device. + * @param array $lazyload_selectors Allowed watcher selectors as keys. + * + * @return array|null Selector strings, or null when a client selector is unsafe (the device rule is voided). + */ + private static function collect_device_selectors( array $personalized_selectors, array $lcp_data, array $lazyload_selectors ) { + $css_selectors = []; + foreach ( $personalized_selectors as $selector => $above_fold_selectors ) { + if ( ! isset( $lazyload_selectors[ $selector ] ) ) { + continue; + } + if ( empty( $above_fold_selectors ) ) { + $css_selectors[] = 'html ' . strip_tags( $selector ) . ':not(.optml-bg-lazyloaded)'; + continue; + } + foreach ( $above_fold_selectors as $above_fold_selector => $bg_urls ) { + if ( ! self::is_safe_selector( $above_fold_selector ) ) { + return null; + } + $css_selectors[] = 'html ' . strip_tags( $selector ) . ':not(' . $above_fold_selector . '):not(.optml-bg-lazyloaded)'; + } + } + + $css_selectors = array_unique( $css_selectors ); + + if ( isset( $lcp_data['type'] ) && $lcp_data['type'] === 'bg' && ! empty( $lcp_data['bgSelector'] ) ) { + if ( ! self::is_safe_selector( $lcp_data['bgSelector'] ) ) { + return null; + } + $css_selectors = array_map( + function ( $selector ) use ( $lcp_data ) { + return $selector . ':not(' . $lcp_data['bgSelector'] . ')'; + }, + $css_selectors + ); + } + + return array_values( $css_selectors ); + } } diff --git a/tests/test-bg-selector.php b/tests/test-bg-selector.php index 3565e60d..606c70c4 100644 --- a/tests/test-bg-selector.php +++ b/tests/test-bg-selector.php @@ -179,6 +179,50 @@ public function test_personalized_css_keeps_plain_selectors() { $this->assertStringContainsString( '{ background-image: none !important; }', $css ); } + /** + * A safe selector followed by an unsafe one still voids the whole device rule. + */ + public function test_personalized_css_drops_rule_when_any_selector_unsafe() { + $device_data = [ + 'bg' => [ + self::WATCHER => [ + '#hero' => [], + self::PAYLOAD => [], + ], + ], + ]; + + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => $device_data, + Profile::DEVICE_TYPE_DESKTOP => $device_data, + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertSame( '', $css ); + } + + /** + * When only one device is voided, the surviving device stays scoped to its own media query + * instead of leaking a media-query-less rule onto the voided device's viewport. + */ + public function test_personalized_css_scopes_surviving_device_to_media_query() { + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], + Profile::DEVICE_TYPE_DESKTOP => [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ] ], + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + // The surviving desktop rule must stay desktop-scoped and not apply on phones. + $this->assertStringStartsWith( '@media (min-width: 600px) {', $css ); + $this->assertStringNotContainsString( '@media (max-width: 600px)', $css ); + $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + } + /** * A watcher with no above-fold elements still hides all its matches (unchanged behaviour). */ From 8e213ab40416b10b73cc92bfa3fc5299f4f66213 Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 16:26:37 +0530 Subject: [PATCH 5/7] fix: add test for mobile device scoping --- tests/test-bg-selector.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test-bg-selector.php b/tests/test-bg-selector.php index 606c70c4..e994c916 100644 --- a/tests/test-bg-selector.php +++ b/tests/test-bg-selector.php @@ -223,6 +223,25 @@ public function test_personalized_css_scopes_surviving_device_to_media_query() { $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); } + /** + * The mirror case: when desktop is voided, the surviving mobile rule stays mobile-scoped. + */ + public function test_personalized_css_scopes_surviving_mobile_device_to_media_query() { + $css = Lazyload::get_personalized_css( + [ + Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ] ], + Profile::DEVICE_TYPE_DESKTOP => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], + ] + ); + + $this->assertStringNotContainsString( 'background:red', $css ); + $this->assertStringNotContainsString( self::PAYLOAD, $css ); + // The surviving mobile rule must stay mobile-scoped and not apply on desktop. + $this->assertStringStartsWith( '@media (max-width: 600px) {', $css ); + $this->assertStringNotContainsString( 'min-width', $css ); + $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + } + /** * A watcher with no above-fold elements still hides all its matches (unchanged behaviour). */ From dc13197f4e5ccb8a228b87dacb987a0df111f8af Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 16:27:03 +0530 Subject: [PATCH 6/7] fix: remove redundant tests --- tests/test-bg-selector.php | 52 +++++++------------------------------- 1 file changed, 9 insertions(+), 43 deletions(-) diff --git a/tests/test-bg-selector.php b/tests/test-bg-selector.php index e994c916..cb7c3403 100644 --- a/tests/test-bg-selector.php +++ b/tests/test-bg-selector.php @@ -155,30 +155,6 @@ public function test_personalized_css_drops_injected_lcp_selector() { $this->assertSame( '', $css ); } - /** - * A page with only plain selectors still renders exactly as before. - */ - public function test_personalized_css_keeps_plain_selectors() { - $device_data = [ - 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], - 'lcp' => [ - 'type' => 'bg', - 'bgSelector' => 'body > div.hero:nth-of-type(1)', - ], - ]; - - $css = Lazyload::get_personalized_css( - [ - Profile::DEVICE_TYPE_MOBILE => $device_data, - Profile::DEVICE_TYPE_DESKTOP => $device_data, - ] - ); - - $this->assertStringContainsString( 'html ' . self::WATCHER . ':not(#hero):not(.optml-bg-lazyloaded)', $css ); - $this->assertStringContainsString( ':not(body > div.hero:nth-of-type(1))', $css ); - $this->assertStringContainsString( '{ background-image: none !important; }', $css ); - } - /** * A safe selector followed by an unsafe one still voids the whole device rule. */ @@ -211,7 +187,13 @@ public function test_personalized_css_scopes_surviving_device_to_media_query() { $css = Lazyload::get_personalized_css( [ Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], - Profile::DEVICE_TYPE_DESKTOP => [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ] ], + Profile::DEVICE_TYPE_DESKTOP => [ + 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], + 'lcp' => [ + 'type' => 'bg', + 'bgSelector' => 'body > div.hero:nth-of-type(1)', + ], + ], ] ); @@ -220,7 +202,9 @@ public function test_personalized_css_scopes_surviving_device_to_media_query() { // The surviving desktop rule must stay desktop-scoped and not apply on phones. $this->assertStringStartsWith( '@media (min-width: 600px) {', $css ); $this->assertStringNotContainsString( '@media (max-width: 600px)', $css ); + // The safe above-fold and LCP selectors are still rendered (the gating must not over-reject them). $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); + $this->assertStringContainsString( ':not(body > div.hero:nth-of-type(1))', $css ); } /** @@ -242,24 +226,6 @@ public function test_personalized_css_scopes_surviving_mobile_device_to_media_qu $this->assertStringContainsString( ':not(#hero):not(.optml-bg-lazyloaded)', $css ); } - /** - * A watcher with no above-fold elements still hides all its matches (unchanged behaviour). - */ - public function test_personalized_css_hides_all_when_no_above_fold() { - $device_data = [ - 'bg' => [ self::WATCHER => [] ], - ]; - - $css = Lazyload::get_personalized_css( - [ - Profile::DEVICE_TYPE_MOBILE => $device_data, - Profile::DEVICE_TYPE_DESKTOP => $device_data, - ] - ); - - $this->assertSame( 'html ' . self::WATCHER . ':not(.optml-bg-lazyloaded) { background-image: none !important; }', $css ); - } - /** * Reset the cached background lazyload selectors. */ From 1ea0feaa0601b21ecfa8c224769486730b0bf75b Mon Sep 17 00:00:00 2001 From: girishpanchal30 Date: Thu, 24 Sep 2026 16:44:59 +0530 Subject: [PATCH 7/7] fix: return type declarations --- tests/test-bg-selector.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test-bg-selector.php b/tests/test-bg-selector.php index cb7c3403..b9ca916d 100644 --- a/tests/test-bg-selector.php +++ b/tests/test-bg-selector.php @@ -56,7 +56,7 @@ public function tearDown(): void { * * @param string $selector Selector to check. */ - public function test_is_safe_selector_accepts_generated_selectors( $selector ) { + public function test_is_safe_selector_accepts_generated_selectors( $selector ): void { $this->assertTrue( Lazyload::is_safe_selector( $selector ) ); } @@ -65,7 +65,7 @@ public function test_is_safe_selector_accepts_generated_selectors( $selector ) { * * @return array */ - public function safe_selectors() { + public function safe_selectors(): array { return [ 'body' => [ 'body' ], 'id' => [ '#hero' ], @@ -82,7 +82,7 @@ public function safe_selectors() { * * @param mixed $selector Selector to check. */ - public function test_is_safe_selector_rejects_metacharacters( $selector ) { + public function test_is_safe_selector_rejects_metacharacters( $selector ): void { $this->assertFalse( Lazyload::is_safe_selector( $selector ) ); } @@ -91,7 +91,7 @@ public function test_is_safe_selector_rejects_metacharacters( $selector ) { * * @return array */ - public function unsafe_selectors() { + public function unsafe_selectors(): array { return [ 'issue payload' => [ self::PAYLOAD ], 'tailwind colon' => [ 'body > div.md:flex' ], @@ -111,7 +111,7 @@ public function unsafe_selectors() { /** * An injected above-fold selector voids the device rule instead of leaking CSS. */ - public function test_personalized_css_drops_injected_selectors() { + public function test_personalized_css_drops_injected_selectors(): void { $device_data = [ 'bg' => [ self::WATCHER => [ @@ -135,7 +135,7 @@ public function test_personalized_css_drops_injected_selectors() { /** * An injected LCP selector voids the device rule. */ - public function test_personalized_css_drops_injected_lcp_selector() { + public function test_personalized_css_drops_injected_lcp_selector(): void { $device_data = [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ], 'lcp' => [ @@ -158,7 +158,7 @@ public function test_personalized_css_drops_injected_lcp_selector() { /** * A safe selector followed by an unsafe one still voids the whole device rule. */ - public function test_personalized_css_drops_rule_when_any_selector_unsafe() { + public function test_personalized_css_drops_rule_when_any_selector_unsafe(): void { $device_data = [ 'bg' => [ self::WATCHER => [ @@ -183,7 +183,7 @@ public function test_personalized_css_drops_rule_when_any_selector_unsafe() { * When only one device is voided, the surviving device stays scoped to its own media query * instead of leaking a media-query-less rule onto the voided device's viewport. */ - public function test_personalized_css_scopes_surviving_device_to_media_query() { + public function test_personalized_css_scopes_surviving_device_to_media_query(): void { $css = Lazyload::get_personalized_css( [ Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ self::PAYLOAD => [] ] ] ], @@ -210,7 +210,7 @@ public function test_personalized_css_scopes_surviving_device_to_media_query() { /** * The mirror case: when desktop is voided, the surviving mobile rule stays mobile-scoped. */ - public function test_personalized_css_scopes_surviving_mobile_device_to_media_query() { + public function test_personalized_css_scopes_surviving_mobile_device_to_media_query(): void { $css = Lazyload::get_personalized_css( [ Profile::DEVICE_TYPE_MOBILE => [ 'bg' => [ self::WATCHER => [ '#hero' => [] ] ] ], @@ -229,7 +229,7 @@ public function test_personalized_css_scopes_surviving_mobile_device_to_media_qu /** * Reset the cached background lazyload selectors. */ - private function reset_background_selectors() { + private function reset_background_selectors(): void { $property = new ReflectionProperty( Optml_Lazyload_Replacer::class, 'background_lazyload_selectors' ); $property->setAccessible( true ); $property->setValue( null, null );