diff --git a/README.md b/README.md index 76601f34..8da99532 100644 --- a/README.md +++ b/README.md @@ -345,3 +345,11 @@ add_filter('optml_dont_replace_url', function( $old, $url ) { }, 10, 2); + += Can I keep the sizes attribute I wrote for an image ? = + +Yes. Add the `data-optml-keep-sizes` attribute to the image tag and the page profiler won't add its own entries to the image's `sizes` attribute. Images lazy loaded by Optimole don't keep `srcset` or `sizes`, since the lazy loader doesn't need them. To keep the existing `sizes` attribute on every image, add this snippet: + + +add_filter( 'optml_keep_existing_sizes', '__return_true' ); + diff --git a/composer.lock b/composer.lock index 5dadbdc8..047a51e6 100644 --- a/composer.lock +++ b/composer.lock @@ -64,16 +64,16 @@ }, { "name": "codeinwp/themeisle-sdk", - "version": "3.3.67", + "version": "3.3.68", "source": { "type": "git", "url": "https://github.com/Codeinwp/themeisle-sdk.git", - "reference": "e1dda44811660a65c3da6994349c054a46686d69" + "reference": "8477d5100ffdddd316a3e51129d775d8854d8a77" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Codeinwp/themeisle-sdk/zipball/e1dda44811660a65c3da6994349c054a46686d69", - "reference": "e1dda44811660a65c3da6994349c054a46686d69", + "url": "https://api.github.com/repos/Codeinwp/themeisle-sdk/zipball/8477d5100ffdddd316a3e51129d775d8854d8a77", + "reference": "8477d5100ffdddd316a3e51129d775d8854d8a77", "shasum": "" }, "require-dev": { @@ -99,22 +99,22 @@ ], "support": { "issues": "https://github.com/Codeinwp/themeisle-sdk/issues", - "source": "https://github.com/Codeinwp/themeisle-sdk/tree/v3.3.67" + "source": "https://github.com/Codeinwp/themeisle-sdk/tree/v3.3.68" }, - "time": "2026-10-13T12:16:34+00:00" + "time": "2026-10-27T13:38:50+00:00" }, { "name": "enshrined/svg-sanitize", - "version": "1.0.0", + "version": "1.1.1", "source": { "type": "git", "url": "https://github.com/darylldoyle/svg-sanitizer.git", - "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff" + "reference": "1976ff93c8d8a234cdd3ff99626bd43ecf11bf49" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", - "reference": "f3300fcd1bbf67d205b52217c75d0f7d6a8c47ff", + "url": "https://api.github.com/repos/darylldoyle/svg-sanitizer/zipball/1976ff93c8d8a234cdd3ff99626bd43ecf11bf49", + "reference": "1976ff93c8d8a234cdd3ff99626bd43ecf11bf49", "shasum": "" }, "require": { @@ -144,9 +144,9 @@ "description": "An SVG sanitizer for PHP", "support": { "issues": "https://github.com/darylldoyle/svg-sanitizer/issues", - "source": "https://github.com/darylldoyle/svg-sanitizer/tree/1.0.0" + "source": "https://github.com/darylldoyle/svg-sanitizer/tree/1.1.1" }, - "time": "2026-09-01T09:35:47+00:00" + "time": "2026-10-20T09:45:10+00:00" }, { "name": "symfony/polyfill-php80", diff --git a/inc/attachment_cache.php b/inc/attachment_cache.php index 64fa4a62..392caa9c 100644 --- a/inc/attachment_cache.php +++ b/inc/attachment_cache.php @@ -5,6 +5,13 @@ */ class Optml_Attachment_Cache { const CACHE_GROUP = 'om_att'; + /** + * Most URLs kept in the in-memory map during one request. + * + * A page with many distinct image URLs would otherwise grow the map for the whole request. + * When it is full, the older half is dropped; those URLs are still in the persistent cache. + */ + const MAX_MEMORY_ENTRIES = 2000; /** * Local cache map. * @@ -32,10 +39,10 @@ public static function get_cached_attachment_id( $url ) { return self::$cache_map[ $cache_key ]; } - $value = wp_using_ext_object_cache() + $value = wp_using_ext_object_cache() ? wp_cache_get( $cache_key, self::CACHE_GROUP ) : get_transient( self::CACHE_GROUP . $cache_key ); - self::$cache_map[ $cache_key ] = $value; + self::remember( $cache_key, $value ); return $value; } @@ -51,7 +58,7 @@ public static function get_cached_attachment_id( $url ) { public static function set_cached_attachment_id( $url, $id ) { $cache_key = self::get_cache_key( $url ); // We cache also in memory to avoid calling DB every time when not using Object Cache. - self::$cache_map[ $cache_key ] = $id; + self::remember( $cache_key, $id ); // If the ID is not found we cache for 10 minutes, otherwise for a week. // We try to reduce the cache time when is not found to // avoid caching for situation when this might be temporary. @@ -61,6 +68,21 @@ public static function set_cached_attachment_id( $url, $id ) { : set_transient( self::CACHE_GROUP . $cache_key, $id, $expiration ); } + /** + * Keep a value in the in-memory map, dropping the older half when it is full. + * + * @param string $cache_key The cache key. + * @param int|numeric-string|false $value The attachment ID (a numeric string when read back from a transient), or false when it is not cached. + * + * @return void + */ + private static function remember( $cache_key, $value ) { + if ( ! isset( self::$cache_map[ $cache_key ] ) && count( self::$cache_map ) >= self::MAX_MEMORY_ENTRIES ) { + self::$cache_map = array_slice( self::$cache_map, (int) ( self::MAX_MEMORY_ENTRIES / 2 ), null, true ); + } + self::$cache_map[ $cache_key ] = $value; + } + /** * Generate cache key for URL. * diff --git a/inc/manager.php b/inc/manager.php index 587fe756..dc5dce2c 100644 --- a/inc/manager.php +++ b/inc/manager.php @@ -142,6 +142,13 @@ final class Optml_Manager { */ private static $ob_processed = false; + /** + * Reads the last PHP error; tests swap it to simulate a fatal error. + * + * @var callable(): (array{type: int, message: string, file: string, line: int}|null) + */ + private static $error_reader = 'error_get_last'; + /** * Class instance method. * @@ -698,7 +705,9 @@ public static function parse_images_from_html( $content ) { if ( OPTML_DEBUG ) { do_action( 'optml_log', 'Content to parse images from: ' . $content ); } - $regex = '/(?:]+?href=["|\'](?P[^\s]+?)["|\'][^>]*?>\s*)?(?P(?:\s*)?]*?\s?(?:' . implode( '|', array_merge( [ 'src' ], Optml_Tag_Replacer::possible_src_attributes() ) ) . ')=["\'\\\\]*?(?P[' . Optml_Config::$chars . ']{10,}).*?>(?:\s*<\/noscript\s*>)?){1}(?:\s*<\/a>)?/ismu'; + // A lazy-load attribute (e.g. data-src) holding a data: placeholder is skipped, so the real src is used. + // A data: src is still matched: the image then belongs to another lazy-load script. + $regex = '/(?:]+?href=["|\'](?P[^\s]+?)["|\'][^>]*?>\s*)?(?P(?:\s*)?]*?(?:\ssrc=["\'\\\\]*?|\s?(?:' . implode( '|', array_merge( [ 'src' ], Optml_Tag_Replacer::possible_src_attributes() ) ) . ')=["\'\\\\]*?(?!data:))(?P[' . Optml_Config::$chars . ']{10,}).*?>(?:\s*<\/noscript\s*>)?){1}(?:\s*<\/a>)?/ismu'; if ( preg_match_all( $regex, $content, $images, PREG_OFFSET_CAPTURE ) ) { if ( OPTML_DEBUG ) { @@ -983,6 +992,9 @@ public function handle_buffer_fallback( $content, $phase = 0 ) { if ( self::$ob_processed || $content === '' ) { return $content; } + if ( self::request_had_fatal_error() ) { + return $content; + } if ( ! $this->captures_at_shutdown() ) { try { return $this->replace_content( $content, self::is_ajax_request() ); @@ -1049,6 +1061,36 @@ public function close_final_buffer() { $this->capture_and_process_buffer( false ); } + /** + * Whether the request already died with a fatal error. + * + * A fatal error (memory exhausted, time limit) that hits while MySQL is still + * sending a query's rows leaves the connection out of sync, so every later + * query in the request fails with "Commands out of sync". The page is then + * passed through unprocessed, without the profiler and cache reads that would + * each log that error. + * + * @param array|null $error The last error, from error_get_last(). + * + * @return bool + */ + public static function is_fatal_error( $error ) { + if ( ! is_array( $error ) || ! isset( $error['type'] ) ) { + return false; + } + + return in_array( (int) $error['type'], [ E_ERROR, E_PARSE, E_CORE_ERROR, E_COMPILE_ERROR ], true ); + } + + /** + * Whether the current request already hit a fatal error. + * + * @return bool + */ + private static function request_had_fatal_error() { + return self::is_fatal_error( call_user_func( self::$error_reader ) ); + } + /** * Capture our buffer, process it outside the display-handler context and echo the result. * @@ -1085,6 +1127,10 @@ private function capture_and_process_buffer( $is_page = true ) { */ $html = apply_filters( 'optml_captured_page_html', $html ); } + if ( self::request_had_fatal_error() ) { + echo $html; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- full page HTML, passed through unprocessed. + return true; + } echo $this->replace_content( $html, self::is_ajax_request() ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- full page HTML, escaping would break the page. } return true; diff --git a/inc/media_offload.php b/inc/media_offload.php index c08ab51d..c0080bcb 100644 --- a/inc/media_offload.php +++ b/inc/media_offload.php @@ -57,6 +57,11 @@ class Optml_Media_Offload extends Optml_App_Replacer { const POST_ROLLBACK_FLAG = 'optimole_rollback_post'; const RETRYABLE_META_COUNTER = '_optimole_retryable_errors'; + /** + * Post meta holding the file a rollback moved into uploads before the attachment was updated to it. + */ + const ROLLBACK_PENDING_FILE = '_optimole_rollback_pending_file'; + /** * Transient name for the transfer lock. */ @@ -67,6 +72,17 @@ class Optml_Media_Offload extends Optml_App_Replacer { */ const TRANSFER_LOCK_TTL = 600; + /** + * Option counting the failed attempts of the current transfer step, and the steps skipped so far. + * It is an option rather than a transient so an object cache eviction can't reset the count. + */ + const STEP_FAILURES_OPTION = 'optml_transfer_step_failures'; + + /** + * How many times a failing transfer step is retried before the chain moves on. + */ + const MAX_STEP_RETRIES = 5; + /** * Flag used inside wp_get_attachment url filter. * @@ -874,6 +890,8 @@ public function rollback_and_update_images( $image_ids ) { if ( ! isset( $current_meta['file'] ) || ! self::is_uploaded_image( $current_meta['file'] ) ) { delete_post_meta( $id, self::META_KEYS['offloaded'] ); delete_post_meta( $id, self::OM_OFFLOADED_FLAG ); + // A rollback that died after WordPress saved the local metadata ends here on retry. + delete_post_meta( $id, self::ROLLBACK_PENDING_FILE ); ++$success_back; continue; } @@ -966,8 +984,21 @@ public function rollback_and_update_images( $image_ids ) { continue; } - // Move the temporary file into the uploads directory. - $results = wp_handle_sideload( $file, $overrides, get_the_date( 'Y/m', $id ) ); + if ( ! function_exists( 'update_attached_file' ) ) { + include_once ABSPATH . '/wp-admin/includes/post.php'; + } + if ( ! function_exists( 'update_attached_file' ) ) { + update_post_meta( $id, self::META_KEYS['rollback_error'], 'true' ); + continue; + } + + // A file left behind by an interrupted attempt is replaced in place: a new sideload would get a + // suffixed name, and deleting it first would leave the attachment without a file if the move fails. + $pending = self::get_pending_rollback_file( $id ); + $results = null !== $pending + ? self::replace_pending_rollback_file( $pending, $temp_file, $type ) + // Move the temporary file into the uploads directory. + : wp_handle_sideload( $file, $overrides, get_the_date( 'Y/m', $id ) ); if ( ! empty( $results['error'] ) ) { if ( OPTML_DEBUG_MEDIA ) { do_action( 'optml_log', ' wp_handle_sideload error' ); @@ -978,6 +1009,13 @@ public function rollback_and_update_images( $image_ids ) { continue; } + // Remember the moved file until the attachment points to it, so a retry can clean it up. + update_post_meta( $id, self::ROLLBACK_PENDING_FILE, $results['file'] ); + + // Point the attachment at the restored file right away: generating the sub-sizes is the + // slow part, and a request that dies there must not leave the stored path behind. + update_attached_file( $id, $results['file'] ); + if ( ! function_exists( 'wp_create_image_subsizes' ) ) { include_once ABSPATH . '/wp-admin/includes/image.php'; } @@ -1003,14 +1041,7 @@ public function rollback_and_update_images( $image_ids ) { wp_update_attachment_metadata( $id, $meta ); } - if ( ! function_exists( 'update_attached_file' ) ) { - include_once ABSPATH . '/wp-admin/includes/post.php'; - } - if ( ! function_exists( 'update_attached_file' ) ) { - update_post_meta( $id, self::META_KEYS['rollback_error'], 'true' ); - continue; - } - update_attached_file( $id, $results['file'] ); + delete_post_meta( $id, self::ROLLBACK_PENDING_FILE ); $duplicated_images = apply_filters( 'optml_offload_duplicated_images', [], $id ); if ( is_array( $duplicated_images ) && ! empty( $duplicated_images ) ) { @@ -1052,6 +1083,75 @@ public function rollback_and_update_images( $image_ids ) { return $success_back; } + /** + * Get the file an interrupted rollback of this attachment moved into uploads. + * + * Only an existing file inside the uploads directory is returned, so a corrupted meta value can + * never point the rollback at anything else. + * + * @param int $id The attachment ID. + * + * @return string|null The absolute path, or null when there is no usable file. + */ + public static function get_pending_rollback_file( int $id ): ?string { + $pending = get_post_meta( $id, self::ROLLBACK_PENDING_FILE, true ); + if ( empty( $pending ) || ! is_string( $pending ) ) { + return null; + } + + $uploads = wp_get_upload_dir(); + $base_dir = realpath( $uploads['basedir'] ); + $file = realpath( $pending ); + if ( false === $base_dir || false === $file || ! is_file( $file ) ) { + return null; + } + + $base_dir = trailingslashit( wp_normalize_path( $base_dir ) ); + if ( strpos( wp_normalize_path( $file ), $base_dir ) !== 0 ) { + return null; + } + + // The stored path, not the resolved one: WordPress stores attached files relative to the uploads path as configured. + return $pending; + } + + /** + * Put the downloaded image in place of the file an interrupted rollback left behind. + * + * The image is copied next to the old file first and then renamed over it, so a copy that fails + * partway (full disk, file size limit) leaves the old file intact. + * + * @param string $pending The file left behind, from {@see self::get_pending_rollback_file()}. + * @param string $temp_file The downloaded image. + * @param string $type The image MIME type. + * + * @return array{file?: string, url?: string, type?: string, error?: string} The same shape as wp_handle_sideload(). + */ + public static function replace_pending_rollback_file( string $pending, string $temp_file, string $type ): array { + $partial = $pending . '.optml-partial'; + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged -- a failed move is reported through the return value. + if ( ! @copy( $temp_file, $partial ) || ! @rename( $partial, $pending ) ) { + wp_delete_file( $partial ); + + return [ 'error' => 'The restored file could not be written to ' . $pending ]; + } + wp_delete_file( $temp_file ); + + // Same permissions as wp_handle_sideload() gives a new file. + $stat = stat( dirname( $pending ) ); + if ( false !== $stat ) { + chmod( $pending, $stat['mode'] & 0000666 ); + } + + $uploads = wp_get_upload_dir(); + + return [ + 'file' => $pending, + 'url' => str_replace( wp_normalize_path( $uploads['basedir'] ), $uploads['baseurl'], wp_normalize_path( $pending ) ), + 'type' => $type, + ]; + } + /** * Handle the bulk actions. * @@ -1988,6 +2088,7 @@ public static function move_images( $action, $refresh ) { $lock_token = self::acquire_transfer_lock( $action ); if ( false !== $lock_token ) { + delete_option( self::STEP_FAILURES_OPTION ); $total = ceil( $count / $batch ); self::schedule_action( time(), @@ -2069,25 +2170,79 @@ public static function is_scheduled( $hook ) { * @return string|false The lock token on success, false if another worker already holds the lock. */ public static function acquire_transfer_lock( $action ) { + // Reading the transient also removes it once it has expired, so an expired lock can be taken again. $lock = get_transient( self::TRANSFER_LOCK_TRANSIENT ); if ( false !== $lock ) { return false; } $token = wp_generate_uuid4(); + $value = [ + 'token' => $token, + 'action' => $action, + ]; - set_transient( - self::TRANSFER_LOCK_TRANSIENT, - [ - 'token' => $token, - 'action' => $action, - ], - self::TRANSFER_LOCK_TTL - ); + if ( ! self::add_transfer_lock( $value ) ) { + return false; + } return $token; } + /** + * Store the transfer lock only if no other request stored it first. + * + * A read followed by a write lets two requests that start at the same moment both take the lock. + * The write here fails when the lock already exists: wp_cache_add() with a persistent object + * cache, and an INSERT IGNORE on the transient row otherwise. Both use the same storage as + * set_transient(), so the rest of the lock code reads it as before. + * + * @param array{token: string, action: string} $value The lock value. + * + * @return bool True if this request stored the lock. + */ + private static function add_transfer_lock( array $value ): bool { + if ( wp_using_ext_object_cache() ) { + return wp_cache_add( self::TRANSFER_LOCK_TRANSIENT, $value, 'transient', self::TRANSFER_LOCK_TTL ); + } + + global $wpdb; + + // The expiry is written first, so a request that dies right after the insert cannot leave a + // lock that never expires. A request that loses the race only rewrites the winner's expiry to + // the same moment. + update_option( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, time() + self::TRANSFER_LOCK_TTL, false ); + + $option = '_transient_' . self::TRANSFER_LOCK_TRANSIENT; + $added = $wpdb->query( + $wpdb->prepare( + "INSERT IGNORE INTO `{$wpdb->options}` (`option_name`, `option_value`, `autoload`) VALUES (%s, %s, %s)", + $option, + maybe_serialize( $value ), + 'no' + ) + ); + + // The read above cached the transient as missing; forget that so the next read sees the new row. + $notoptions = wp_cache_get( 'notoptions', 'options' ); + if ( is_array( $notoptions ) && isset( $notoptions[ $option ] ) ) { + unset( $notoptions[ $option ] ); + wp_cache_set( 'notoptions', $notoptions, 'options' ); + } + wp_cache_delete( $option, 'options' ); + + if ( 1 !== $added ) { + return false; + } + + // Written again, in case the previous holder's release removed it between the two writes. The + // cached copy is dropped first: it still holds the first write, so update_option() would skip the write. + wp_cache_delete( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, 'options' ); + update_option( '_transient_timeout_' . self::TRANSFER_LOCK_TRANSIENT, time() + self::TRANSFER_LOCK_TTL, false ); + + return true; + } + /** * Renew the transfer lock if we still own it, extending its expiration. * @@ -2274,9 +2429,16 @@ public function start_processing_images( $action, $batch, $page, $total, $step, self::$instance->settings->update( $option, 'disabled' ); - self::$instance->settings->update( 'show_offload_finish_notice', $type ); + // A transfer that skipped failing steps left images behind, so it does not report success. + $skipped = self::get_skipped_steps( $lock_token ); + delete_option( self::STEP_FAILURES_OPTION ); + if ( $skipped > 0 ) { + self::$instance->logger->add_log( $type, $skipped . ' step(s) were skipped after failing ' . self::MAX_STEP_RETRIES . ' times. Start the transfer again to move the images left behind.' ); + } else { + self::$instance->settings->update( 'show_offload_finish_notice', $type ); + } - // Transfer completed successfully: release the lock. + // Transfer finished: release the lock. self::release_transfer_lock( $lock_token ); return; @@ -2322,12 +2484,22 @@ public function start_processing_images( $action, $batch, $page, $total, $step, $lock_token, ] ); + self::clear_step_failures( $lock_token ); } catch ( Exception $e ) { // Reschedule the cron to run again after a delay. Sometimes memory limit is exausted. // This is a retryable error, so the lock is kept rather than released. $delay_in_seconds = 10; self::$instance->logger->add_log( $type, $e->getMessage() ); + // A step that keeps failing would be retried every few seconds forever, with the transfer + // shown as running. After a few attempts the chain moves on, so the transfer can finish + // and the images left behind can be retried. + if ( self::count_step_failure( $lock_token, $action, $step ) >= self::MAX_STEP_RETRIES ) { + self::record_skipped_step( $lock_token ); + self::$instance->logger->add_log( $type, 'Step ' . $step . ' failed ' . self::MAX_STEP_RETRIES . ' times, moving on to the next one.' ); + $step = $step + 1; + } + self::schedule_action( time() + $delay_in_seconds, 'optml_start_processing_images', @@ -2345,6 +2517,98 @@ public function start_processing_images( $action, $batch, $page, $total, $step, } } + /** + * Read the step failures record of a transfer chain. + * + * @param string $lock_token The transfer lock token of the chain. + * + * @return array{token: string, action: string, step: int, count: int, skipped: int} The record, empty for another chain. + */ + private static function get_step_failures( string $lock_token ): array { + $failures = get_option( self::STEP_FAILURES_OPTION ); + $record = [ + 'token' => $lock_token, + 'action' => '', + 'step' => 0, + 'count' => 0, + 'skipped' => 0, + ]; + if ( ! is_array( $failures ) || ! isset( $failures['token'] ) || $failures['token'] !== $lock_token ) { + return $record; + } + + return [ + 'token' => $lock_token, + 'action' => isset( $failures['action'] ) ? (string) $failures['action'] : '', + 'step' => isset( $failures['step'] ) ? (int) $failures['step'] : 0, + 'count' => isset( $failures['count'] ) ? (int) $failures['count'] : 0, + 'skipped' => isset( $failures['skipped'] ) ? (int) $failures['skipped'] : 0, + ]; + } + + /** + * Count a failed attempt of a transfer step. + * + * @param string $lock_token The transfer lock token of the chain. + * @param string $action The transfer action. + * @param int $step The step that failed. + * + * @return int How many times in a row this step has failed. + */ + private static function count_step_failure( string $lock_token, string $action, int $step ): int { + $record = self::get_step_failures( $lock_token ); + if ( $record['action'] !== $action || $record['step'] !== $step ) { + $record['count'] = 0; + } + $record['action'] = $action; + $record['step'] = $step; + ++$record['count']; + update_option( self::STEP_FAILURES_OPTION, $record, false ); + + return $record['count']; + } + + /** + * Remember that the chain skipped a step that kept failing. + * + * @param string $lock_token The transfer lock token of the chain. + * + * @return void + */ + private static function record_skipped_step( string $lock_token ): void { + $record = self::get_step_failures( $lock_token ); + $record['count'] = 0; + ++$record['skipped']; + update_option( self::STEP_FAILURES_OPTION, $record, false ); + } + + /** + * Clear the failed attempts after a step succeeds, keeping the count of skipped steps. + * + * @param string $lock_token The transfer lock token of the chain. + * + * @return void + */ + private static function clear_step_failures( string $lock_token ): void { + $record = self::get_step_failures( $lock_token ); + if ( 0 === $record['count'] ) { + return; + } + $record['count'] = 0; + update_option( self::STEP_FAILURES_OPTION, $record, false ); + } + + /** + * How many steps the chain skipped after they kept failing. + * + * @param string $lock_token The transfer lock token of the chain. + * + * @return int + */ + public static function get_skipped_steps( string $lock_token ): int { + return self::get_step_failures( $lock_token )['skipped']; + } + /** * Alter attachment image src for offloaded images. * diff --git a/inc/tag_replacer.php b/inc/tag_replacer.php index 3330edb2..0df7d581 100644 --- a/inc/tag_replacer.php +++ b/inc/tag_replacer.php @@ -13,6 +13,11 @@ final class Optml_Tag_Replacer extends Optml_App_Replacer { use Optml_Validator; use Optml_Dam_Offload_Utils; + /** + * Attribute an image uses to keep its own sizes attribute. + */ + const KEEP_SIZES_ATTRIBUTE = 'data-optml-keep-sizes'; + /** * Cached object instance. * @@ -550,11 +555,16 @@ public function add_missing_srcset_attributes( $tag, $missing_srcsets, $new_url, $tag = $this->insert_after_attribute( $tag, 'src', $srcset_attr ); } - // Handle sizes attribute - skip if existing sizes contains calc() or complex formulas + // Handle sizes attribute - skip if the image asks to keep its existing sizes, or they contain calc() or complex formulas if ( ! empty( $new_sizes_entries ) ) { $should_skip_sizes = false; - if ( $has_existing_sizes ) { + if ( $has_existing_sizes && $this->keeps_existing_sizes( $tag ) ) { + $should_skip_sizes = true; + if ( OPTML_DEBUG ) { + do_action( 'optml_log', 'Keeping the existing sizes attribute: ' . $new_url ); + } + } elseif ( $has_existing_sizes ) { // Check if existing sizes contains calc() or complex formulas $sizes = $this->find_quoted_attribute( $tag, 'sizes' ); if ( null !== $sizes ) { @@ -596,9 +606,9 @@ function ( $a, $b ) { } } } else { - // Log that sizes were skipped due to complex formulas + // Log that sizes were skipped (kept on request, or complex formula) if ( OPTML_DEBUG ) { - do_action( 'optml_log', 'Skipped adding sizes due to complex formula detection' ); + do_action( 'optml_log', 'Skipped adding sizes' ); } } } @@ -834,6 +844,28 @@ private function has_attribute( string $tag, string $name ): bool { return in_array( $name, array_column( $this->get_attributes( $tag ) ?? [], 'name' ), true ); } + /** + * Check whether an image's own sizes attribute must be left as it is. + * + * An image opts out with the data-optml-keep-sizes attribute. The optml_keep_existing_sizes filter + * does it for every image, or for the ones it picks, with no change to the markup. + * + * @param string $tag The image tag. + * + * @return bool True if the existing sizes attribute is kept. + */ + private function keeps_existing_sizes( string $tag ): bool { + $keep = $this->has_attribute( $tag, self::KEEP_SIZES_ATTRIBUTE ); + + /** + * Filters whether Optimole leaves an image's existing sizes attribute unchanged. + * + * @param bool $keep Whether the existing sizes attribute is kept. True when the image has data-optml-keep-sizes. + * @param string $tag The image tag. + */ + return (bool) apply_filters( 'optml_keep_existing_sizes', $keep, $tag ); + } + /** * Check whether the tag's src or srcset is a data: URI placeholder. * diff --git a/inc/url_replacer.php b/inc/url_replacer.php index af827f3c..e1095c9d 100644 --- a/inc/url_replacer.php +++ b/inc/url_replacer.php @@ -324,13 +324,18 @@ private function normalize_image( $url, $original_url, $args, $is_uploaded = fal /** * Get the active cache booster. * + * Individual tokens are stored as the API returned them, which can be a number, + * so the token is cast to the string the SDK expects. + * * @param string $url The URL. - * @param string $main_cache_buster The default value. + * @param mixed $main_cache_buster The default value. * * @return string */ public static function get_active_cache_booster( $url, $main_cache_buster ) { - return ( get_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY ) ?: [] )[ crc32( wp_basename( $url ) ) ] ?? $main_cache_buster; + $token = ( get_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY ) ?: [] )[ crc32( wp_basename( $url ) ) ] ?? $main_cache_buster; + + return is_scalar( $token ) ? (string) $token : ''; } /** * Throw error on object clone diff --git a/readme.txt b/readme.txt index 89ca3fab..da7c72b0 100755 --- a/readme.txt +++ b/readme.txt @@ -345,3 +345,11 @@ add_filter('optml_dont_replace_url', function( $old, $url ) { }, 10, 2); + += Can I keep the sizes attribute I wrote for an image ? = + +Yes. Add the `data-optml-keep-sizes` attribute to the image tag and the page profiler won't add its own entries to the image's `sizes` attribute. Images lazy loaded by Optimole don't keep `srcset` or `sizes`, since the lazy loader doesn't need them. To keep the existing `sizes` attribute on every image, add this snippet: + + +add_filter( 'optml_keep_existing_sizes', '__return_true' ); + diff --git a/tests/test-attachment-cache.php b/tests/test-attachment-cache.php new file mode 100644 index 00000000..67ce1f29 --- /dev/null +++ b/tests/test-attachment-cache.php @@ -0,0 +1,82 @@ + + */ + private function memory_map() { + $property = new ReflectionProperty( Optml_Attachment_Cache::class, 'cache_map' ); + $property->setAccessible( true ); + + return $property->getValue(); + } + + /** + * A page with many distinct uncached image URLs must not grow the in-memory map for the whole request. + */ + public function test_memory_map_is_bounded_on_cache_misses() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i <= $limit; $i++ ) { + Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/miss-' . $i . '.jpg' ); + } + + $this->assertCount( (int) ( $limit / 2 ) + 1, $this->memory_map() ); + } + + /** + * The newest URLs stay in memory when the older half is dropped, and the dropped ones are + * still found in the persistent cache. + */ + public function test_dropped_entries_are_still_found_in_the_persistent_cache() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i <= $limit; $i++ ) { + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-' . $i . '.jpg', $i + 1 ); + } + + $map = $this->memory_map(); + $this->assertCount( (int) ( $limit / 2 ) + 1, $map ); + $this->assertContains( $limit + 1, $map ); + $this->assertNotContains( 1, $map ); + + $this->assertSame( 1, (int) Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg' ) ); + } + + /** + * Updating a URL that is already in memory never drops other entries. + */ + public function test_updating_a_known_url_keeps_the_map() { + $limit = Optml_Attachment_Cache::MAX_MEMORY_ENTRIES; + + for ( $i = 0; $i < $limit; $i++ ) { + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-' . $i . '.jpg', $i + 1 ); + } + Optml_Attachment_Cache::set_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg', 99 ); + + $this->assertCount( $limit, $this->memory_map() ); + $this->assertSame( 99, Optml_Attachment_Cache::get_cached_attachment_id( 'https://example.org/wp-content/uploads/image-0.jpg' ) ); + } +} diff --git a/tests/test-dimension-replacement.php b/tests/test-dimension-replacement.php index 32640ad4..6d389b03 100644 --- a/tests/test-dimension-replacement.php +++ b/tests/test-dimension-replacement.php @@ -117,6 +117,17 @@ public function test_existing_attributes_preserved() { } + /** + * A data: placeholder in data-src before src does not stop the real image from being processed. + */ + public function test_dimensions_injected_when_data_src_placeholder_comes_first() { + $html_input = 'Test image'; + + $result = $this->tag_replacer->process_image_tags( $html_input, $this->extract_images_from_html( $html_input ) ); + + $this->assertMatchesRegularExpression( '/width="1920".*height="1080".*data-opt-id=' . $this->tag_replacer->get_id_by_url( self::IMAGE_URL ) . '/', $result ); + } + /** * Helper method to extract images from HTML for testing. * Uses the actual manager method to ensure correct structure. diff --git a/tests/test-fatal-error-passthrough.php b/tests/test-fatal-error-passthrough.php new file mode 100644 index 00000000..ef05c13e --- /dev/null +++ b/tests/test-fatal-error-passthrough.php @@ -0,0 +1,87 @@ +assertTrue( Optml_Manager::is_fatal_error( [ 'type' => $type, 'message' => 'Allowed memory size exhausted' ] ) ); + } + + /** + * @return array + */ + public function fatal_error_types(): array { + return [ + 'E_ERROR' => [ E_ERROR ], + 'E_PARSE' => [ E_PARSE ], + 'E_CORE_ERROR' => [ E_CORE_ERROR ], + 'E_COMPILE_ERROR' => [ E_COMPILE_ERROR ], + ]; + } + + /** + * Warnings, notices and deprecations don't stop the page from being processed. + * + * @dataProvider non_fatal_errors + * + * @param array|null $error The last error. + */ + public function test_other_errors_are_not_fatal( $error ): void { + $this->assertFalse( Optml_Manager::is_fatal_error( $error ) ); + } + + /** + * @return array|null}> + */ + public function non_fatal_errors(): array { + return [ + 'no error' => [ null ], + 'E_WARNING' => [ [ 'type' => E_WARNING ] ], + 'E_NOTICE' => [ [ 'type' => E_NOTICE ] ], + 'E_DEPRECATED' => [ [ 'type' => E_DEPRECATED ] ], + 'no type' => [ [ 'message' => 'x' ] ], + ]; + } + + /** + * A warning earlier in the request still lets the output handler process the page. + */ + public function test_output_handler_processes_page_after_a_warning(): void { + $settings = new Optml_Settings(); + $settings->update( + 'service_data', + [ + 'cdn_key' => 'test123', + 'cdn_secret' => '12345', + 'whitelist' => [ 'example.org' ], + ] + ); + $settings->update( 'lazyload', 'disabled' ); + Optml_Url_Replacer::instance()->init(); + Optml_Tag_Replacer::instance()->init(); + Optml_Manager::instance()->init(); + + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- sets error_get_last() to a warning. + @trigger_error( 'earlier warning', E_USER_WARNING ); + $html = ''; + + $result = Optml_Manager::instance()->handle_buffer_fallback( $html ); + + $this->assertStringContainsString( 'i.optimole.com', $result ); + } +} diff --git a/tests/test-lazyload.php b/tests/test-lazyload.php index db132d0a..a7bded32 100644 --- a/tests/test-lazyload.php +++ b/tests/test-lazyload.php @@ -553,4 +553,34 @@ public function test_dam_lazyloading_no_wh_attributes() { $this->assertStringContainsString( $svg, $replaced_content ); remove_filter('optml_lazyload_images_skip','__return_zero'); } + + /** + * A tag whose data-src holds a data: placeholder is lazy-loaded from its real URL. + */ + public function test_lazyload_tag_with_data_uri_data_src() { + $settings = new Optml_Settings(); + $settings->update( 'skip_lazyload_images', 0 ); + // The skip limit is cached in a static on first use, so an earlier test may have cached another value. + $skip_limit = new ReflectionProperty( Optml_Lazyload_Replacer::class, 'skip_lazyload_images' ); + $skip_limit->setAccessible( true ); + $skip_limit->setValue( null, null ); + Optml_Tag_Replacer::$lazyload_skipped_images = 0; + Optml_Manager::instance()->init(); + + $placeholder = 'data:image/gif;base64,R0lGODlhAQABAIAAAP///////yH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='; + $tags = [ + 'Test', + 'Test', + ]; + + foreach ( $tags as $tag ) { + $replaced_content = Optml_Manager::instance()->replace_content( $tag ); + + $this->assertStringContainsString( 'data-opt-src="https://test123.i.optimole.com/', $replaced_content, $tag ); + $this->assertStringContainsString( '/https://example.com/wp-content/uploads/2026/10/photo.jpg"', $replaced_content, $tag ); + $this->assertStringContainsString( 'data-src="' . $placeholder . '"', $replaced_content, $tag ); + } + + $skip_limit->setValue( null, null ); + } } diff --git a/tests/test-media.php b/tests/test-media.php index f8a9eaa1..78d6f37a 100644 --- a/tests/test-media.php +++ b/tests/test-media.php @@ -543,6 +543,139 @@ public function test_background_step_does_nothing_while_another_run_moves_images $this->assert_moving_images_lock_is_free(); } + /** + * A step that throws every time is retried a few times, then the chain moves on to the next step + * instead of retrying it every few seconds forever. + */ + public function test_failing_step_moves_on_after_retries(): void { + ( new Optml_Settings() )->update( 'rollback_status', 'enabled' ); + delete_option( Optml_Media_Offload::STEP_FAILURES_OPTION ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'rollback_images' ); + $fail = function () { + throw new Exception( 'cloud lookup failed' ); + }; + add_action( 'pre_get_posts', $fail ); + + $steps = []; + for ( $attempt = 1; $attempt <= Optml_Media_Offload::MAX_STEP_RETRIES; $attempt++ ) { + _set_cron_array( [] ); + Optml_Media_Offload::instance()->start_processing_images( 'rollback_images', 10, 1, 5, 1, $token ); + $steps[] = $this->scheduled_transfer_step(); + } + remove_action( 'pre_get_posts', $fail ); + + $this->assertSame( array_merge( array_fill( 0, Optml_Media_Offload::MAX_STEP_RETRIES - 1, 1 ), [ 2 ] ), $steps ); + $this->assertSame( 1, Optml_Media_Offload::get_skipped_steps( $token ) ); + $this->assertSame( 0, get_option( Optml_Media_Offload::STEP_FAILURES_OPTION )['count'] ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + } + + /** + * A step that succeeds clears the failures of the one before it, but the skipped steps are kept. + */ + public function test_successful_step_clears_failures(): void { + ( new Optml_Settings() )->update( 'rollback_status', 'enabled' ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'rollback_images' ); + update_option( Optml_Media_Offload::STEP_FAILURES_OPTION, $this->step_failures( $token, Optml_Media_Offload::MAX_STEP_RETRIES - 1, 1 ), false ); + + _set_cron_array( [] ); + Optml_Media_Offload::instance()->start_processing_images( 'rollback_images', 10, 1, 5, 1, $token ); + + $this->assertSame( 2, $this->scheduled_transfer_step() ); + $this->assertSame( 0, get_option( Optml_Media_Offload::STEP_FAILURES_OPTION )['count'] ); + $this->assertSame( 1, Optml_Media_Offload::get_skipped_steps( $token ) ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + } + + /** + * Failures left by an earlier transfer don't count toward the retries of a new one. + */ + public function test_failures_of_another_transfer_are_ignored(): void { + ( new Optml_Settings() )->update( 'rollback_status', 'enabled' ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'rollback_images' ); + update_option( Optml_Media_Offload::STEP_FAILURES_OPTION, $this->step_failures( 'old-transfer', Optml_Media_Offload::MAX_STEP_RETRIES - 1, 2 ), false ); + $fail = function () { + throw new Exception( 'cloud lookup failed' ); + }; + add_action( 'pre_get_posts', $fail ); + + _set_cron_array( [] ); + Optml_Media_Offload::instance()->start_processing_images( 'rollback_images', 10, 1, 5, 1, $token ); + remove_action( 'pre_get_posts', $fail ); + + $this->assertSame( 1, $this->scheduled_transfer_step() ); + $this->assertSame( 1, get_option( Optml_Media_Offload::STEP_FAILURES_OPTION )['count'] ); + $this->assertSame( 0, Optml_Media_Offload::get_skipped_steps( $token ) ); + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + } + + /** + * A transfer that skipped steps does not show the "Transfer Complete" notice. + */ + public function test_transfer_with_skipped_steps_does_not_report_success(): void { + $settings = new Optml_Settings(); + $settings->update( 'rollback_status', 'enabled' ); + $settings->update( 'show_offload_finish_notice', '' ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'rollback_images' ); + update_option( Optml_Media_Offload::STEP_FAILURES_OPTION, $this->step_failures( $token, 0, 1 ), false ); + + Optml_Media_Offload::instance()->start_processing_images( 'rollback_images', 10, 6, 5, 6, $token ); + + $settings = new Optml_Settings(); + $this->assertSame( 'disabled', $settings->get( 'rollback_status' ) ); + $this->assertSame( '', $settings->get( 'show_offload_finish_notice' ) ); + $this->assertFalse( get_option( Optml_Media_Offload::STEP_FAILURES_OPTION ) ); + } + + /** + * A transfer that ran every step shows the "Transfer Complete" notice. + */ + public function test_transfer_without_skipped_steps_reports_success(): void { + $settings = new Optml_Settings(); + $settings->update( 'rollback_status', 'enabled' ); + $settings->update( 'show_offload_finish_notice', '' ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'rollback_images' ); + update_option( Optml_Media_Offload::STEP_FAILURES_OPTION, $this->step_failures( 'old-transfer', 0, 3 ), false ); + + Optml_Media_Offload::instance()->start_processing_images( 'rollback_images', 10, 6, 5, 6, $token ); + + $this->assertSame( 'rollback', ( new Optml_Settings() )->get( 'show_offload_finish_notice' ) ); + } + + /** + * Build a step failures record. + * + * @param string $token The transfer lock token. + * @param int $count The failed attempts of step 1. + * @param int $skipped The skipped steps. + * + * @return array{token: string, action: string, step: int, count: int, skipped: int} + */ + private function step_failures( string $token, int $count, int $skipped ): array { + return [ + 'token' => $token, + 'action' => 'rollback_images', + 'step' => 1, + 'count' => $count, + 'skipped' => $skipped, + ]; + } + + /** + * @return int|null The step of the background transfer step waiting in WP-Cron. + */ + private function scheduled_transfer_step(): ?int { + foreach ( (array) _get_cron_array() as $hooks ) { + if ( isset( $hooks['optml_start_processing_images'] ) ) { + $event = reset( $hooks['optml_start_processing_images'] ); + + return (int) $event['args'][4]; + } + } + + return null; + } + /** * @return int The background transfer steps waiting in WP-Cron. */ @@ -691,6 +824,117 @@ public function test_image_rollback() { $this->assertStringContainsString( '-150x150', $image_meta['sizes']['thumbnail']['file'] ); } + /** + * A rollback that died after moving the file into uploads leaves that file behind. The retry + * must write over it, so the restored image keeps its name instead of getting a "-1" suffix. + */ + public function test_rollback_retry_replaces_file_left_by_interrupted_attempt() { + $attachment_id = self::$sample_attachement; + $uploads = wp_upload_dir( get_the_date( 'Y/m', $attachment_id ) ); + $orphan = $uploads['path'] . '/' . self::$files[0] . '.jpg'; + wp_mkdir_p( $uploads['path'] ); + file_put_contents( $orphan, 'left by an interrupted rollback' ); + update_post_meta( $attachment_id, Optml_Media_Offload::ROLLBACK_PENDING_FILE, $orphan ); + + Optml_Media_Offload::instance()->rollback_images( 100 ); + + $attached = get_attached_file( $attachment_id ); + $this->assertSame( $orphan, $attached ); + $this->assertTrue( file_exists( $attached ) ); + $this->assertNotSame( 'left by an interrupted rollback', file_get_contents( $attached ) ); + $this->assertEmpty( get_post_meta( $attachment_id, Optml_Media_Offload::ROLLBACK_PENDING_FILE, true ) ); + } + + /** + * A request that dies while the sub-sizes are generated has already pointed the attachment at + * the restored file, and remembers that file so a retry can clean it up. + */ + public function test_rollback_dying_during_subsizes_keeps_attachment_on_restored_file() { + // Another file already uses the name, so the restored file gets a suffixed one. + $uploads = wp_upload_dir( get_the_date( 'Y/m', self::$sample_attachement ) ); + $taken = $uploads['path'] . '/' . self::$files[0] . '.jpg'; + wp_mkdir_p( $uploads['path'] ); + file_put_contents( $taken, 'another file' ); + + $died = function () { + throw new RuntimeException( 'request died' ); + }; + add_filter( 'intermediate_image_sizes_advanced', $died ); + + try { + Optml_Media_Offload::instance()->rollback_and_update_images( [ self::$sample_attachement ] ); + $this->fail( 'The rollback was expected to stop.' ); + } catch ( RuntimeException $e ) { + $this->assertSame( 'request died', $e->getMessage() ); + } finally { + remove_filter( 'intermediate_image_sizes_advanced', $died ); + } + + $restored = get_post_meta( self::$sample_attachement, Optml_Media_Offload::ROLLBACK_PENDING_FILE, true ); + $this->assertNotEmpty( $restored ); + $this->assertTrue( file_exists( $restored ) ); + $this->assertNotSame( $taken, $restored ); + $this->assertSame( $restored, get_attached_file( self::$sample_attachement, true ) ); + unlink( $taken ); + } + + /** + * Only a file inside the uploads directory is used, whatever the meta says. + */ + public function test_pending_rollback_file_outside_uploads_is_ignored() { + $attachment_id = self::factory()->post->create( [ 'post_type' => 'attachment' ] ); + $outside = get_temp_dir() . 'optml-outside-uploads.jpg'; + file_put_contents( $outside, 'not ours' ); + update_post_meta( $attachment_id, Optml_Media_Offload::ROLLBACK_PENDING_FILE, $outside ); + + $this->assertNull( Optml_Media_Offload::get_pending_rollback_file( $attachment_id ) ); + $this->assertSame( 'not ours', file_get_contents( $outside ) ); + unlink( $outside ); + } + + /** + * A retry whose copy fails partway (full disk, file size limit) keeps the file the attachment points to. + */ + public function test_failed_copy_keeps_the_pending_file() { + if ( ! in_array( 'optml-failing-read', stream_get_wrappers(), true ) ) { + stream_wrapper_register( 'optml-failing-read', Optml_Test_Failing_Read_Stream::class ); + } + $uploads = wp_get_upload_dir(); + $pending = $uploads['basedir'] . '/optml-pending-copy.jpg'; + file_put_contents( $pending, 'restored image' ); + + $result = Optml_Media_Offload::replace_pending_rollback_file( $pending, 'optml-failing-read://download', 'image/jpeg' ); + + $this->assertArrayHasKey( 'error', $result ); + $this->assertSame( 'restored image', file_get_contents( $pending ) ); + $this->assertFileDoesNotExist( $pending . '.optml-partial' ); + unlink( $pending ); + } + + /** + * A retry that ends in the branch for attachments whose local metadata was already saved + * clears the pending file too. + */ + public function test_retry_after_local_metadata_was_saved_clears_pending_file() { + $attachment_id = self::factory()->post->create( [ 'post_type' => 'attachment' ] ); + wp_update_attachment_metadata( $attachment_id, [ 'file' => '2026/10/restored.jpg' ] ); + update_post_meta( $attachment_id, Optml_Media_Offload::ROLLBACK_PENDING_FILE, '/some/where/restored.jpg' ); + + Optml_Media_Offload::instance()->rollback_and_update_images( [ $attachment_id ] ); + + $this->assertEmpty( get_post_meta( $attachment_id, Optml_Media_Offload::ROLLBACK_PENDING_FILE, true ) ); + } + + /** + * A rollback that completes leaves no pending file behind, so a later retry deletes nothing. + */ + public function test_completed_rollback_clears_pending_file() { + Optml_Media_Offload::instance()->rollback_images( 100 ); + + $this->assertTrue( file_exists( get_attached_file( self::$sample_attachement ) ) ); + $this->assertEmpty( get_post_meta( self::$sample_attachement, Optml_Media_Offload::ROLLBACK_PENDING_FILE, true ) ); + } + public function test_rollback_retry_clears_errors() { // Use a dedicated attachment with fully controlled meta so the assertions do @@ -989,6 +1233,63 @@ public function test_acquire_transfer_lock_blocks_concurrent_start() { delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); } + /** + * Two requests that both read the lock as free must not both take it: the one whose write comes + * second gets false, and the lock keeps the first one's token and an expiry. + */ + public function test_acquire_transfer_lock_fails_when_another_request_took_it_after_the_read() { + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + global $wpdb; + + // This request reads the lock as missing, then another request stores it before our write. + $this->assertFalse( get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ) ); + $wpdb->insert( + $wpdb->options, + [ + 'option_name' => '_transient_' . Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT, + 'option_value' => maybe_serialize( [ 'token' => 'other-request', 'action' => 'offload_images' ] ), + 'autoload' => 'no', + ] + ); + + $this->assertFalse( Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ) ); + + $lock = get_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + $this->assertSame( 'other-request', $lock['token'] ); + $this->assertGreaterThan( time(), (int) get_option( '_transient_timeout_' . Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ) ); + + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + } + + /** + * Another request's release can delete the expiry row between our first write and the insert. + * The lock we then store must still get an expiry, or a dead worker would block transfers forever. + */ + public function test_acquire_transfer_lock_restores_expiry_removed_before_the_insert() { + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + global $wpdb; + $timeout = '_transient_timeout_' . Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT; + + $release = function ( $query ) use ( &$release, $wpdb, $timeout ) { + if ( false !== strpos( $query, 'INSERT IGNORE' ) ) { + remove_filter( 'query', $release ); + $wpdb->delete( $wpdb->options, [ 'option_name' => $timeout ] ); + } + + return $query; + }; + add_filter( 'query', $release ); + $token = Optml_Media_Offload::acquire_transfer_lock( 'offload_images' ); + remove_filter( 'query', $release ); + + $this->assertNotFalse( $token ); + $stored = $wpdb->get_var( $wpdb->prepare( "SELECT option_value FROM {$wpdb->options} WHERE option_name = %s", $timeout ) ); + $this->assertNotNull( $stored ); + $this->assertGreaterThan( time(), (int) $stored ); + + delete_transient( Optml_Media_Offload::TRANSFER_LOCK_TRANSIENT ); + } + /** * A lock left behind by a worker that died mid-batch (fatal error, killed process) must * eventually become reclaimable, otherwise a crashed transfer would be stuck forever. @@ -1225,3 +1526,79 @@ public function transfer_action_provider(): array { ]; } } + +/** + * A stream that returns some bytes, then fails, like a copy that runs out of disk space partway. + */ +class Optml_Test_Failing_Read_Stream { + /** + * Stream context, set by PHP. + * + * @var resource|null + */ + public $context; + + /** + * Reads done so far. + * + * @var int + */ + private int $reads = 0; + + /** + * Open the stream. + * + * @param string $path The URL. + * @param string $mode The mode. + * @param int $options The options. + * @param string|null $opened_path The opened path. + * + * @return bool + */ + public function stream_open( string $path, string $mode, int $options, ?string &$opened_path ): bool { + return true; + } + + /** + * Return a few bytes the first time, then fail. + * + * @param int $count The bytes asked for. + * + * @return string|false + */ + public function stream_read( int $count ) { + ++$this->reads; + + return 1 === $this->reads ? 'partial' : false; + } + + /** + * The stream never reaches its end. + * + * @return bool + */ + public function stream_eof(): bool { + return false; + } + + /** + * Stream stats. + * + * @return array + */ + public function stream_stat(): array { + return []; + } + + /** + * URL stats. + * + * @param string $path The URL. + * @param int $flags The flags. + * + * @return array + */ + public function url_stat( string $path, int $flags ): array { + return [ 'size' => 100 ]; + } +} diff --git a/tests/test-replacer.php b/tests/test-replacer.php index e42be6a3..701acfbc 100644 --- a/tests/test-replacer.php +++ b/tests/test-replacer.php @@ -849,4 +849,73 @@ public function test_dam_flag_removal() { $this->assertStringNotContainsString('dam:1', $replaced_content); $this->assertStringContainsString('q:mauto', $replaced_content); } -} \ No newline at end of file + + /** + * A data: placeholder in a lazy-load attribute before src is not taken as the image URL. + */ + public function test_parse_images_skips_data_uri_in_lazy_attribute() { + $placeholder = 'data:image/gif;base64,R0lGODlhAQABAIAAAP///////yH5BAEKAAEALAAAAAABAAEAAAICTAEAOw=='; + $real_src = 'https://example.com/wp-content/uploads/2026/10/photo.jpg'; + + $tags = [ + 'Test', + "Test", + 'Test', + 'Test', + '', + ]; + + foreach ( $tags as $tag ) { + $images = Optml_Manager::parse_images_from_html( $tag ); + $this->assertSame( [ $real_src ], $images['img_url'], $tag ); + $this->assertSame( [ $tag ], $images['img_tag'], $tag ); + } + + $placeholder_only = 'Test'; + $this->assertSame( [], Optml_Manager::parse_images_from_html( $placeholder_only ) ); + + $placeholder_and_lazy_src = 'Test'; + $this->assertSame( [ $real_src ], Optml_Manager::parse_images_from_html( $placeholder_and_lazy_src )['img_url'] ); + } + + /** + * A data: src is still matched, so an image owned by another lazy-load script is handled as before. + */ + public function test_parse_images_keeps_data_uri_src() { + $tag = ''; + $images = Optml_Manager::parse_images_from_html( $tag ); + + $this->assertSame( [ 'data:image/gif' ], $images['img_url'] ); + } + + /** + * Individual cache tokens are stored as the API returned them, which can be a number. + * The SDK only accepts a string cache buster. + */ + public function test_build_url_with_numeric_individual_cache_token() { + $url = 'http://example.org/wp-content/uploads/2018/05/brands.png'; + set_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY, [ crc32( wp_basename( $url ) ) => 12345 ], HOUR_IN_SECONDS ); + + $result = Optml_Url_Replacer::instance()->build_url( $url ); + + delete_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY ); + + $this->assertStringContainsString( '/cb:12345/', $result ); + } + + /** + * The active cache token is always a string, whatever type was stored. + */ + public function test_get_active_cache_booster_returns_string() { + $url = 'http://example.org/wp-content/uploads/2018/05/brands.png'; + + $this->assertSame( 'eFRn.20eff', Optml_Url_Replacer::get_active_cache_booster( $url, 'eFRn.20eff' ) ); + $this->assertSame( '987', Optml_Url_Replacer::get_active_cache_booster( $url, 987 ) ); + $this->assertSame( '', Optml_Url_Replacer::get_active_cache_booster( $url, null ) ); + + set_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY, [ crc32( wp_basename( $url ) ) => 12345 ], HOUR_IN_SECONDS ); + $this->assertSame( '12345', Optml_Url_Replacer::get_active_cache_booster( $url, 'eFRn.20eff' ) ); + $this->assertSame( 'eFRn.20eff', Optml_Url_Replacer::get_active_cache_booster( 'http://example.org/other.png', 'eFRn.20eff' ) ); + delete_transient( Optml_Settings::INDIVIDUAL_CACHE_TOKENS_KEY ); + } +} diff --git a/tests/test-srcset.php b/tests/test-srcset.php index 5b90d7d6..3f8c6b3d 100644 --- a/tests/test-srcset.php +++ b/tests/test-srcset.php @@ -168,6 +168,65 @@ public function test_add_missing_srcset_attributes_with_existing_sizes() { $this->assertStringContainsString( '(max-width: 768px) 400px', $result ); } + /** + * An image with data-optml-keep-sizes keeps its hand-written sizes, and still gets the new srcset entries. + */ + public function test_keep_sizes_attribute_preserves_existing_sizes() { + $tag_replacer = new Optml_Tag_Replacer(); + + $tag = 'Test'; + $missing_srcsets = [ + [ 'w' => 200, 'h' => 150, 'd' => 1, 's' => '200w', 'b' => 320 ], + [ 'w' => 400, 'h' => 300, 'd' => 1, 's' => '400w', 'b' => 768 ], + ]; + + $result = $tag_replacer->add_missing_srcset_attributes( $tag, $missing_srcsets, 'https://example.com/image.jpg', false ); + + $this->assertStringContainsString( 'sizes="(max-width: 480px) 300px"', $result ); + $this->assertStringNotContainsString( '(max-width: 320px) 200px', $result ); + $this->assertStringContainsString( '200w', $result ); + } + + /** + * The optml_keep_existing_sizes filter keeps existing sizes site-wide, with no change to the markup. + */ + public function test_keep_existing_sizes_filter() { + $tag_replacer = new Optml_Tag_Replacer(); + + $tag = 'Test'; + $missing_srcsets = [ + [ 'w' => 200, 'h' => 150, 'd' => 1, 's' => '200w', 'b' => 320 ], + ]; + + add_filter( 'optml_keep_existing_sizes', '__return_true' ); + $result = $tag_replacer->add_missing_srcset_attributes( $tag, $missing_srcsets, 'https://example.com/image.jpg', false ); + remove_filter( 'optml_keep_existing_sizes', '__return_true' ); + + $this->assertStringContainsString( 'sizes="(max-width: 480px) 300px"', $result ); + $this->assertStringNotContainsString( '(max-width: 320px) 200px', $result ); + } + + /** + * A filter returning false overrides the attribute, so the existing sizes are enhanced again. + */ + public function test_keep_existing_sizes_filter_can_override_attribute() { + $tag_replacer = new Optml_Tag_Replacer(); + + $tag = 'Test'; + $missing_srcsets = [ + [ 'w' => 200, 'h' => 150, 'd' => 1, 's' => '200w', 'b' => 320 ], + ]; + + $kept = $tag_replacer->add_missing_srcset_attributes( $tag, $missing_srcsets, 'https://example.com/image.jpg', false ); + $this->assertStringNotContainsString( '(max-width: 320px) 200px', $kept ); + + add_filter( 'optml_keep_existing_sizes', '__return_false' ); + $result = $tag_replacer->add_missing_srcset_attributes( $tag, $missing_srcsets, 'https://example.com/image.jpg', false ); + remove_filter( 'optml_keep_existing_sizes', '__return_false' ); + + $this->assertStringContainsString( '(max-width: 320px) 200px', $result ); + } + /** * Test retina images setting respect */ diff --git a/tests/test-zz-buffer.php b/tests/test-zz-buffer.php index a7f8e2fa..b6b595a8 100644 --- a/tests/test-zz-buffer.php +++ b/tests/test-zz-buffer.php @@ -56,6 +56,7 @@ public function tearDown(): void { } } $this->reset_buffer_state(); + $this->set_error_reader( 'error_get_last' ); remove_filter( 'optml_capture_at_shutdown', '__return_true' ); remove_filter( 'optml_capture_at_shutdown', '__return_false' ); parent::tearDown(); @@ -75,6 +76,70 @@ private function reset_buffer_state( $processed = false ) { } } + /** + * Swap how Optml_Manager reads the last PHP error. + * + * @param callable(): (array{type: int, message: string, file: string, line: int}|null) $reader Returns what error_get_last() would. + */ + private function set_error_reader( $reader ) { + $prop = new ReflectionProperty( Optml_Manager::class, 'error_reader' ); + $prop->setAccessible( true ); + $prop->setValue( null, $reader ); + } + + /** + * Simulate a request that already died with a fatal error. + */ + private function simulate_fatal_error() { + $this->set_error_reader( + function () { + return [ + 'type' => E_ERROR, + 'message' => 'Allowed memory size of 8388608 bytes exhausted', + 'file' => __FILE__, + 'line' => __LINE__, + ]; + } + ); + } + + /** + * After a fatal error the default in-handler mode passes the page through unprocessed. + */ + public function test_default_mode_passes_page_through_after_fatal_error() { + remove_filter( 'optml_capture_at_shutdown', '__return_true' ); + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $this->simulate_fatal_error(); + $manager->close_buffer(); + $manager->close_final_buffer(); + $out = ob_get_clean(); + + $this->assertSame( self::IMG_TAGS, $out ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + + /** + * After a fatal error the shutdown-capture mode echoes the page once, unprocessed, + * and late shutdown output is passed through too. + */ + public function test_capture_mode_passes_page_through_after_fatal_error() { + $manager = Optml_Manager::instance(); + ob_start(); + $manager->process_template_redirect_content(); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $this->simulate_fatal_error(); + $manager->close_buffer(); + echo self::IMG_TAGS; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped + $manager->close_final_buffer(); + $out = ob_get_clean(); + + $this->assertSame( self::IMG_TAGS . self::IMG_TAGS, $out ); + $this->assertSame( $this->base_level, ob_get_level() ); + } + /** * Callbacks on our filters may use output buffering without fataling. *