diff --git a/classes/helpers/FrmAppHelper.php b/classes/helpers/FrmAppHelper.php index 7d1532ea75..22f54185c0 100644 --- a/classes/helpers/FrmAppHelper.php +++ b/classes/helpers/FrmAppHelper.php @@ -10,7 +10,7 @@ class FrmAppHelper { * * @var int */ - public static $db_version = 106; + public static $db_version = 107; /** * Used by the API add-on. diff --git a/classes/models/FrmCreateFile.php b/classes/models/FrmCreateFile.php index 0b41d4fa64..9a30a678a2 100644 --- a/classes/models/FrmCreateFile.php +++ b/classes/models/FrmCreateFile.php @@ -76,13 +76,15 @@ private function set_new_file_path( $atts ) { } /** + * @since x.x Returns whether the file was written. Previously returned nothing. + * * @param string $file_content * - * @return void + * @return bool True if the file was written to disk. */ public function create_file( $file_content ) { if ( ! $this->has_permission ) { - return; + return false; } $dirs_exist = true; @@ -92,11 +94,11 @@ public function create_file( $file_content ) { // Only write the file if the folders exist. if ( ! $dirs_exist ) { - return; + return false; } global $wp_filesystem; - $wp_filesystem->put_contents( $this->new_file_path, $file_content, $this->chmod_file ); + return (bool) $wp_filesystem->put_contents( $this->new_file_path, $file_content, $this->chmod_file ); } /** diff --git a/classes/models/FrmMigrate.php b/classes/models/FrmMigrate.php index cee115c6da..47df93b54f 100644 --- a/classes/models/FrmMigrate.php +++ b/classes/models/FrmMigrate.php @@ -405,7 +405,7 @@ private function migrate_data( $old_db_version ) { return; } - $migrations = array( 16, 11, 16, 17, 23, 25, 86, 90, 97, 98, 101, 104, 105 ); + $migrations = array( 16, 11, 16, 17, 23, 25, 86, 90, 97, 98, 101, 104, 105, 107 ); foreach ( $migrations as $migration ) { if ( FrmAppHelper::$db_version < $migration || $old_db_version >= $migration ) { @@ -484,6 +484,26 @@ public function uninstall() { return true; } + /** + * Discard the legacy generated-stylesheet cache-busting version. + * + * Versions before this stored frm_last_style_update as gmdate( 'njGi' ), which could repeat + * across different dates and every year, so a third-party CSS cache keyed on the enqueued + * stylesheet URL could keep serving a copy generated from superseded content. + * + * Removing the stored value makes FrmStylesController::get_css_version() fall back to the + * plugin version, which guarantees the enqueued URL changes on this upgrade even when the + * post-upgrade style regeneration is skipped. The content-derived value is written by the + * next FrmStyle::save_settings() call. + * + * @since x.x + * + * @return void + */ + private function migrate_to_107() { + delete_option( 'frm_last_style_update' ); + } + /** * In older versions of Lite, it's possible we've saved the wrong location ID. * So force it to get valid values again. diff --git a/classes/models/FrmStyle.php b/classes/models/FrmStyle.php index 7cb8e3bb13..7402a18235 100644 --- a/classes/models/FrmStyle.php +++ b/classes/models/FrmStyle.php @@ -407,7 +407,6 @@ public function get_color_settings() { */ public function save_settings() { $filename = FrmAppHelper::plugin_path() . '/css/custom_theme.css.php'; - update_option( 'frm_last_style_update', gmdate( 'njGi' ) ); if ( ! is_file( $filename ) ) { return; @@ -415,12 +414,52 @@ public function save_settings() { $this->clear_cache(); - $css = $this->get_css_content( $filename ); - $create_file = new FrmCreateFile( self::get_create_style_file_args() ); - $create_file->create_file( $css ); + $css = $this->get_css_content( $filename ); + $create_file = new FrmCreateFile( self::get_create_style_file_args() ); + $file_written = $create_file->create_file( $css ); update_option( 'frmpro_css', $css, false ); set_transient( 'frmpro_css', $css, MONTH_IN_SECONDS ); + + if ( $file_written ) { + self::update_css_version( $css ); + } + } + + /** + * Store the cache-busting version for the generated stylesheet. + * + * The value is derived from the stylesheet contents rather than the clock, so it changes if + * and only if the generated bytes change. It is read back by + * FrmStylesController::get_css_version() and appended to the enqueued stylesheet URL. + * + * This previously used gmdate( 'njGi' ), which omitted the year and concatenated unpadded + * month, day and hour values. Distinct dates therefore produced identical version strings + * (1 Jan 10:59, 11 Jan 00:59 and 1 Nov 00:59 all produced "111059"), the value repeated + * every year, and two saves within the same minute were indistinguishable. Third-party CSS + * caches keyed on the enqueued URL could keep serving a copy generated from superseded + * content. + * + * Only called once FrmCreateFile::create_file() confirms the bytes reached disk, and only + * after the frmpro_css option and transient are stored, so the version never advertises + * content that is not being served. This matters because the value is content-derived and + * therefore idempotent: publishing a hash for a write that silently failed would pin a + * third-party cache to the superseded file permanently, since every later save of the same + * content would reproduce the same hash and the same URL. + * + * @since x.x + * + * @param string $css Generated stylesheet contents. + * + * @return void + */ + private static function update_css_version( $css ) { + // skipcq: PHP-A1004 -- md5() here is a content-derived cache-busting token, not a + // password hash. It must be deterministic so the same stylesheet always yields the + // same URL; password_hash() is salted and non-deterministic and would defeat the + // whole mechanism. Matches the existing md5()-for-cache-key calls in FrmAddon, + // FrmAntiSpam, FrmFormApi and FrmStyleApi. + update_option( 'frm_last_style_update', substr( md5( $css ), 0, 12 ) ); } /** diff --git a/phpcs.xml b/phpcs.xml index 00d2870cfe..b0787717c8 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -202,6 +202,16 @@ test_FrmCSVExportHelper.php + test_FrmCreateFile.php + test_FrmStyle.php + + + test_FrmCreateFile.php + test_FrmStyle.php + + + test_FrmCreateFile.php + test_FrmStyle.php FrmEntriesController.php diff --git a/tests/phpunit/database/test_FrmMigrate.php b/tests/phpunit/database/test_FrmMigrate.php index 8721970356..d41922dced 100644 --- a/tests/phpunit/database/test_FrmMigrate.php +++ b/tests/phpunit/database/test_FrmMigrate.php @@ -297,6 +297,44 @@ public function test_migrate_to_97() { } } + /** + * Make sure migrate_to_107 is actually reached by the migration dispatch (migrate_data()) + * when upgrading from db_version 106, not just that the method works when called directly. + * A migration that is registered but never dispatched is the failure mode this guards + * against. + * + * This deliberately calls migrate_data() directly rather than the public upgrade(), because + * upgrade() unconditionally regenerates the default style at the end of every call + * ( $frm_style->update( 'default' ) ), which would overwrite frm_last_style_update with a + * fresh value regardless of whether migrate_to_107 actually ran, masking the result. + * + * @covers FrmMigrate::migrate_data + * @covers FrmMigrate::migrate_to_107 + */ + public function test_migrate_to_107_runs_on_dispatch_from_106() { + update_option( 'frm_last_style_update', '111059' ); + + $frmdb = new FrmMigrate(); + $this->run_private_method( array( $frmdb, 'migrate_data' ), array( 106 ) ); + + $this->assertFalse( get_option( 'frm_last_style_update' ), 'migrate_to_107 should have been dispatched and deleted the legacy frm_last_style_update option.' ); + } + + /** + * A site already on db_version 107 (or later) must not have migrate_to_107 run again. + * + * @covers FrmMigrate::migrate_data + * @covers FrmMigrate::migrate_to_107 + */ + public function test_migrate_to_107_does_not_run_again_once_applied() { + update_option( 'frm_last_style_update', 'abcdef123456' ); + + $frmdb = new FrmMigrate(); + $this->run_private_method( array( $frmdb, 'migrate_data' ), array( 107 ) ); + + $this->assertSame( 'abcdef123456', get_option( 'frm_last_style_update' ), 'migrate_to_107 must not re-run once already at db_version 107.' ); + } + /** * @covers FrmMigrate::collation */ diff --git a/tests/phpunit/misc/test_FrmCreateFile.php b/tests/phpunit/misc/test_FrmCreateFile.php new file mode 100644 index 0000000000..746307384b --- /dev/null +++ b/tests/phpunit/misc/test_FrmCreateFile.php @@ -0,0 +1,160 @@ + + */ + private $paths_to_clean_up = array(); + + public function tearDown(): void { + foreach ( $this->paths_to_clean_up as $path ) { + if ( is_dir( $path ) ) { + @rmdir( $path ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged + } elseif ( file_exists( $path ) ) { + @unlink( $path ); // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged + } + } + $this->paths_to_clean_up = array(); + + parent::tearDown(); + } + + /** + * @covers FrmCreateFile::create_file + */ + public function test_create_file_returns_true_on_successful_write() { + $uploads = wp_upload_dir(); + $folder = 'frm-test-create-file-' . wp_generate_password( 8, false ); + $file_name = 'test.css'; + + $create_file = new FrmCreateFile( + array( + 'file_name' => $file_name, + 'folder_name' => $folder, + ) + ); + + $content = 'body{color:#123456}'; + $result = $create_file->create_file( $content ); + $written_path = $uploads['basedir'] . '/' . $folder . '/' . $file_name; + $this->paths_to_clean_up[] = $written_path; + $this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder . '/index.php'; + $this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder; + + $this->assertTrue( $result, 'create_file() should return true when the file is actually written to disk.' ); + $this->assertSame( $content, file_get_contents( $written_path ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + } + + /** + * The write-permission early return (`! $this->has_permission`) can only be reached in this + * harness by forcing the private flag directly: the local/direct filesystem method always + * succeeds here, and there is no practical, non-flaky way to make WP_Filesystem's credential + * check fail deterministically in an automated integration run. Reflection on the private + * property is used only for this one guard clause, not for the method under test itself. + * + * @covers FrmCreateFile::create_file + */ + public function test_create_file_returns_false_without_permission() { + $create_file = new FrmCreateFile( + array( + 'file_name' => 'no-permission.css', + 'folder_name' => 'frm-test-no-permission', + ) + ); + + $permission_property = $this->get_accessible_property( $create_file, 'has_permission' ); + $permission_property->setValue( $create_file, false ); + + $result = $create_file->create_file( 'body{color:#000}' ); + + $this->assertFalse( $result, 'create_file() should return false when there is no filesystem permission.' ); + } + + /** + * Forces the directory-creation early return (`! $dirs_exist`) with a real filesystem + * collision rather than a mock: a plain file is created where FrmCreateFile needs to create a + * directory of the same name, so both mkdir() and the is_dir() fallback genuinely fail. + * + * @covers FrmCreateFile::create_file + */ + public function test_create_file_returns_false_when_directory_cannot_be_created() { + $blocking = 'frm-test-blocking-' . wp_generate_password( 8, false ); + $blocked_path = wp_upload_dir()['basedir'] . '/' . $blocking; + + $this->assertNotFalse( + file_put_contents( $blocked_path, 'not a directory' ), // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents + 'Test setup: failed to create the blocking file.' + ); + $this->paths_to_clean_up[] = $blocked_path; + + $create_file = new FrmCreateFile( + array( + 'file_name' => 'blocked.css', + 'folder_name' => $blocking, + ) + ); + + $result = $create_file->create_file( 'body{color:#000}' ); + + $this->assertFalse( $result, 'create_file() should return false when its target directory cannot be created.' ); + } + + /** + * The widened create_file() return type (void -> bool) must not affect its two internal + * callers: both discard the return value and remain void themselves. + * + * @covers FrmCreateFile::append_file + * @covers FrmCreateFile::combine_files + */ + public function test_append_file_and_combine_files_are_unaffected_by_the_widened_return_type() { + $uploads = wp_upload_dir(); + $folder = 'frm-test-callers-' . wp_generate_password( 8, false ); + + $append_target = new FrmCreateFile( + array( + 'file_name' => 'append.css', + 'folder_name' => $folder, + ) + ); + + $append_result = $append_target->append_file( 'first-part;' ); + $this->assertNull( $append_result, 'append_file() must remain void regardless of create_file()\'s widened return type.' ); + + $append_result_2 = $append_target->append_file( 'second-part;' ); + $this->assertNull( $append_result_2 ); + + $appended_path = $uploads['basedir'] . '/' . $folder . '/append.css'; + $this->paths_to_clean_up[] = $appended_path; + $this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder . '/index.php'; + $this->paths_to_clean_up[] = $uploads['basedir'] . '/' . $folder; + + $this->assertSame( 'first-part;second-part;', file_get_contents( $appended_path ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + + $source_a = $uploads['basedir'] . '/' . $folder . '/source-a.css'; + $source_b = $uploads['basedir'] . '/' . $folder . '/source-b.css'; + file_put_contents( $source_a, 'a{color:#111}' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents + file_put_contents( $source_b, 'b{color:#222}' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents + $this->paths_to_clean_up[] = $source_a; + $this->paths_to_clean_up[] = $source_b; + + $combine_target = new FrmCreateFile( + array( + 'file_name' => 'combined.css', + 'folder_name' => $folder, + ) + ); + + $combine_result = $combine_target->combine_files( array( $source_a, $source_b ) ); + $this->assertNull( $combine_result, 'combine_files() must remain void regardless of create_file()\'s widened return type.' ); + + $combined_path = $uploads['basedir'] . '/' . $folder . '/combined.css'; + $this->paths_to_clean_up[] = $combined_path; + + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + $this->assertSame( "a{color:#111}\nb{color:#222}\n", file_get_contents( $combined_path ) ); + } +} diff --git a/tests/phpunit/styles/test_FrmStyle.php b/tests/phpunit/styles/test_FrmStyle.php index 5c8bb689b8..ebbd729aec 100644 --- a/tests/phpunit/styles/test_FrmStyle.php +++ b/tests/phpunit/styles/test_FrmStyle.php @@ -156,4 +156,305 @@ private function trim_braces( $value ) { $frm_style = new FrmStyle(); return $this->run_private_method( array( $frm_style, 'trim_braces' ), array( $value ) ); } + + /** + * Regression test for the bug this fix addresses: the legacy `gmdate( 'njGi' )` cache-busting + * version omitted the year and concatenated unpadded month/day/hour, so distinct dates could + * produce an identical version string. This is documented directly (no clock mocking needed, + * since gmdate() accepts an explicit timestamp), then the new content-derived version is shown + * to both distinguish and, when content really is unchanged, correctly match across those same + * colliding moments -- proving the new value depends on content, not the clock. + * + * @covers FrmStyle::update_css_version + */ + public function test_update_css_version_does_not_collide_across_previously_colliding_dates() { + // 2026-01-01 10:59 UTC, 2026-01-11 00:59 UTC and 2026-11-01 00:59 UTC. + $colliding_timestamps = array( + gmmktime( 10, 59, 0, 1, 1, 2026 ), + gmmktime( 0, 59, 0, 1, 11, 2026 ), + gmmktime( 0, 59, 0, 11, 1, 2026 ), + ); + + $legacy_versions = array(); + + foreach ( $colliding_timestamps as $timestamp ) { + $legacy_versions[] = gmdate( 'njGi', $timestamp ); + } + + // Document the bug being fixed: the legacy format collides across all three dates. + $this->assertSame( + array( '111059', '111059', '111059' ), + $legacy_versions, + 'Sanity check that these three dates are the ones known to collide under the legacy gmdate( "njGi" ) format.' + ); + + delete_option( 'frm_last_style_update' ); + + // Distinct content "saved" at each of those colliding moments must produce distinct + // versions. This is the property the legacy format could not provide. + $distinct_versions = array(); + + foreach ( $colliding_timestamps as $index => $timestamp ) { + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( 'body{--collision-check:' . $index . '}' ) ); + $distinct_versions[] = get_option( 'frm_last_style_update' ); + } + + $this->assertCount( 3, array_unique( $distinct_versions ), 'The content-derived version must not collide across the three previously-colliding dates.' ); + + // Identical content "saved" at each of those same moments must produce the SAME version, + // proving the derivation is clock-independent rather than merely higher resolution. + $stable_versions = array(); + + foreach ( $colliding_timestamps as $timestamp ) { + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( 'body{color:#123456}' ) ); + $stable_versions[] = get_option( 'frm_last_style_update' ); + } + + $this->assertCount( 1, array_unique( $stable_versions ), 'Identical content must resolve to the same version regardless of when it is saved.' ); + } + + /** + * @covers FrmStyle::update_css_version + */ + public function test_update_css_version_is_sensitive_to_content_in_both_directions() { + delete_option( 'frm_last_style_update' ); + + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( 'body{color:#111111}' ) ); + $version_a = get_option( 'frm_last_style_update' ); + $this->assertNotEmpty( $version_a ); + + // Different CSS must produce a different version. + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( 'body{color:#222222}' ) ); + $version_b = get_option( 'frm_last_style_update' ); + $this->assertNotSame( $version_a, $version_b, 'Different CSS content must produce a different version.' ); + + // Identical CSS must produce the identical version (this is why a hash was chosen over + // time() -- an unchanged save should not needlessly invalidate downstream caches). + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( 'body{color:#111111}' ) ); + $version_a_again = get_option( 'frm_last_style_update' ); + $this->assertSame( $version_a, $version_a_again, 'Identical CSS content must produce the identical version.' ); + } + + /** + * Two consecutive saves in the same PHP process with different content must produce different + * versions, and each version must be derived from that content alone. This fails by + * construction against the legacy gmdate( 'njGi' ) implementation, which is minute-granularity + * and would store the identical value for both saves. + * + * The clock-independence claim is asserted against the content, not against the wall clock. An + * earlier form of this test first sanity-checked that both saves landed in the same legacy + * gmdate( 'njGi' ) bucket, which fails whenever two consecutive calls happen to straddle a + * minute boundary -- a red build for correct product behaviour, on a premise the assertions + * below do not actually need. Pinning each version to the hash of its own content proves the + * property outright rather than relying on the run being lucky. + * + * @covers FrmStyle::update_css_version + */ + public function test_consecutive_saves_with_different_content_produce_content_derived_versions() { + delete_option( 'frm_last_style_update' ); + + $css_1 = 'body{color:#aaaaaa}'; + $css_2 = 'body{color:#bbbbbb}'; + + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( $css_1 ) ); + $version_1 = get_option( 'frm_last_style_update' ); + + $this->run_private_method( array( 'FrmStyle', 'update_css_version' ), array( $css_2 ) ); + $version_2 = get_option( 'frm_last_style_update' ); + + $this->assertNotEmpty( $version_1 ); + $this->assertNotEmpty( $version_2 ); + $this->assertNotSame( $version_1, $version_2, 'Two consecutive saves with different content must produce different versions.' ); + + // Each version is the hash of its own content and nothing else. No minute-granularity + // value -- or any other clock-derived one -- can satisfy both of these assertions, which + // is the reversion this test exists to catch. + $this->assertSame( substr( md5( $css_1 ), 0, 12 ), $version_1, 'The version must be derived from the stylesheet content alone.' ); + $this->assertSame( substr( md5( $css_2 ), 0, 12 ), $version_2, 'The version must be derived from the stylesheet content alone.' ); + } + + /** + * Anti-reversion guard: the legacy date-based version must not creep back into save_settings(). + * + * @covers FrmStyle::save_settings + */ + public function test_save_settings_does_not_reintroduce_legacy_date_based_version() { + $method = new ReflectionMethod( 'FrmStyle', 'save_settings' ); + $lines = file( $method->getFileName() ); + $body = implode( '', array_slice( $lines, $method->getStartLine() - 1, $method->getEndLine() - $method->getStartLine() + 1 ) ); + + $this->assertStringNotContainsString( 'njGi', $body, 'save_settings() must not reintroduce the legacy gmdate( "njGi" ) cache-busting version.' ); + } + + /** + * Regression guard: because the version is a content hash, publishing it for a write that + * silently failed is unrecoverable (every later save of the same content reproduces the same + * hash and URL, permanently pinning a downstream cache to the stale file). This forces a real + * write failure -- via a genuine filesystem directory collision, not a mock -- and asserts + * frm_last_style_update is left completely untouched: neither overwritten nor deleted. + * + * The sentinel assertion on its own would also hold if save_settings() never got as far as + * FrmCreateFile::create_file() -- it returns early when css/custom_theme.css.php is missing, + * which would make this test pass without exercising the failed write at all. Two guards + * close that off: the source stylesheet is asserted to exist before the call, and frmpro_css + * is asserted to have been populated afterwards. The option is only written after + * create_file() has returned, so a populated frmpro_css alongside an untouched version proves + * the write was attempted, reported failure, and the version write was skipped for that + * reason. + * + * @covers FrmStyle::save_settings + */ + public function test_save_settings_leaves_version_untouched_when_file_write_fails() { + add_filter( 'frm_add_css_to_uploads_dir', '__return_true' ); + + $blocked_path = wp_upload_dir()['basedir'] . '/formidable'; + + // A previous save (in this test or another) may have already created this as a real + // directory. It only holds generated/cache data, so it is safe to clear it to guarantee a + // deterministic collision below. + $this->rmdir_recursive( $blocked_path ); + + try { + $this->assertNotFalse( + file_put_contents( $blocked_path, 'not a directory' ), // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_read_file_put_contents + 'Test setup: failed to create the blocking file.' + ); + $this->assertTrue( + is_file( $blocked_path ), + 'Test setup assumption: the blocking path must be a plain file, not a directory,' + . ' so FrmCreateFile cannot create its "formidable" subdirectory there.' + ); + $this->assertTrue( + is_file( FrmAppHelper::plugin_path() . '/css/custom_theme.css.php' ), + 'Test setup assumption: the source stylesheet must exist, or save_settings() returns before FrmCreateFile::create_file() and this test asserts nothing.' + ); + + update_option( 'frm_last_style_update', 'sentinel-untouched' ); + delete_option( 'frmpro_css' ); + + $frm_style = new FrmStyle( 'default' ); + $frm_style->save_settings(); + + $this->assertNotEmpty( + get_option( 'frmpro_css' ), + 'The generated CSS must have been stored, which only happens once FrmCreateFile::create_file() has been called -- otherwise the failed write was never exercised.' + ); + $this->assertSame( + 'sentinel-untouched', + get_option( 'frm_last_style_update' ), + 'frm_last_style_update must be left untouched (not updated, not deleted)' + . ' when the CSS file write fails.' + ); + } finally { + remove_filter( 'frm_add_css_to_uploads_dir', '__return_true' ); + + if ( is_file( $blocked_path ) ) { + unlink( $blocked_path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink + } + } + } + + /** + * @param string $path + * + * @return void + */ + private function rmdir_recursive( $path ) { + if ( is_file( $path ) ) { + unlink( $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink + return; + } + + if ( ! is_dir( $path ) ) { + return; + } + + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged, WordPress.WP.AlternativeFunctions.file_system_operations_scandir + foreach ( array_diff( scandir( $path ), array( '.', '..' ) ) as $item ) { + $this->rmdir_recursive( $path . '/' . $item ); + } + + rmdir( $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_rmdir + } + + /** + * The version must never advertise content the AJAX fallback ( frmpro_css option/transient ) + * is not already serving. Whenever save_settings() advances the version, frmpro_css must + * already be populated with exactly the content that hash was derived from. + * + * This is an ordering claim, so it is asserted at the ordering boundary rather than after the + * fact: a pre_update_option_frm_last_style_update filter records what frmpro_css held at the + * instant the version write was attempted. Checking only the end state would let a regression + * that advances the version first and stores the CSS afterwards pass, since the final values + * agree either way. The end-state assertions are kept as well, so both the ordering and the + * resulting consistency are covered. + * + * The invariant is asserted for every observed write rather than for a single expected one. + * save_settings() can legitimately run more than once per call: get_css_content() renders + * custom_theme.css.php, which reads FrmStyle::get_all(), which creates a default style and + * calls update( 'default' ) -- and therefore save_settings() again -- when no style rows exist + * yet. How many writes happen is incidental and depends on the state the suite leaves behind; + * that each one is ordered after its own frmpro_css write is the property that matters. + * + * @covers FrmStyle::save_settings + */ + public function test_frmpro_css_is_populated_before_version_advances() { + delete_option( 'frm_last_style_update' ); + delete_option( 'frmpro_css' ); + delete_transient( 'frmpro_css' ); + + $observed = array(); + $observer = function ( $value ) use ( &$observed ) { + $observed[] = array( + 'version' => $value, + 'option' => get_option( 'frmpro_css' ), + 'transient' => get_transient( 'frmpro_css' ), + ); + + return $value; + }; + + add_filter( 'pre_update_option_frm_last_style_update', $observer ); + + try { + $frm_style = new FrmStyle( 'default' ); + $frm_style->save_settings(); + } finally { + remove_filter( 'pre_update_option_frm_last_style_update', $observer ); + } + + $version = get_option( 'frm_last_style_update' ); + $this->assertNotEmpty( $version, 'Test setup assumption: the write should succeed and the version should advance in this environment.' ); + + $this->assertNotEmpty( $observed, 'The version write must have been observed, or this test asserts nothing about the ordering.' ); + + foreach ( $observed as $index => $at_write ) { + $where = ' (version write ' . ( $index + 1 ) . ' of ' . count( $observed ) . ')'; + + $this->assertNotEmpty( $at_write['option'], 'The frmpro_css option must already be populated at the moment the version is written, not afterwards.' . $where ); + $this->assertNotEmpty( $at_write['transient'], 'The frmpro_css transient must already be populated at the moment the version is written, not afterwards.' . $where ); + $this->assertSame( + $at_write['version'], + substr( md5( $at_write['option'] ), 0, 12 ), + 'The version being written must hash the CSS already in frmpro_css, so the URL never advertises content the fallback is not serving.' . $where + ); + $this->assertSame( + $at_write['option'], + $at_write['transient'], + 'The frmpro_css option and transient must already agree at the moment the version is written.' . $where + ); + } + + $stored_css = get_option( 'frmpro_css' ); + $transient_css = get_transient( 'frmpro_css' ); + + $this->assertNotEmpty( $stored_css, 'The frmpro_css option must be populated whenever the version advances.' ); + $this->assertNotEmpty( $transient_css, 'The frmpro_css transient must be populated whenever the version advances.' ); + $this->assertSame( $stored_css, $transient_css, 'The frmpro_css option and transient must agree.' ); + $this->assertSame( + $version, + substr( md5( $stored_css ), 0, 12 ), + 'The advanced version must correspond exactly to the frmpro_css content already stored, so the enqueued URL and the AJAX fallback can never disagree.' + ); + } } diff --git a/tests/phpunit/styles/test_FrmStylesController.php b/tests/phpunit/styles/test_FrmStylesController.php index d4061d4bdb..b6e08d5bee 100644 --- a/tests/phpunit/styles/test_FrmStylesController.php +++ b/tests/phpunit/styles/test_FrmStylesController.php @@ -80,4 +80,66 @@ public function test_save() { $updated_style = $frm_style->get_one(); $this->assertSame( $style->post_title . ' Updated', $updated_style->post_title ); } + + /** + * Integration test: saving a style, reading the enqueued `formidable` handle's version, + * changing a colour and saving again must change the enqueued version. This is what actually + * busts a third-party cache keyed on the enqueued stylesheet URL. + * + * @covers FrmStylesController::get_css_version + * @covers FrmStylesController::enqueue_css + * @covers FrmStyle::save_settings + */ + public function test_css_version_changes_when_style_content_changes_after_save() { + $this->set_current_user_to_1(); + $this->set_front_end(); + + $frm_style = new FrmStyle( 'default' ); + $style = $frm_style->get_one(); + + $_POST = array( + 'ID' => $style->ID, + 'style_name' => $style->post_name, + 'frm_style' => wp_create_nonce( 'frm_style_nonce' ), + 'frm_action' => 'save', + 'frm_style_setting' => array( + 'post_title' => $style->post_title, + 'post_content' => array_merge( $style->post_content, array( 'submit_bg_color' => '112233' ) ), + ), + ); + + FrmStylesController::save_style(); + $version_1 = $this->get_registered_formidable_css_version(); + $this->assertNotEmpty( $version_1 ); + + // Change a colour and save again. + $_POST['frm_style_setting']['post_content'] = array_merge( $style->post_content, array( 'submit_bg_color' => '445566' ) ); + FrmStylesController::save_style(); + $version_2 = $this->get_registered_formidable_css_version(); + + $this->assertNotEmpty( $version_2 ); + $this->assertNotSame( $version_1, $version_2, 'The enqueued stylesheet version should change when the generated CSS content changes.' ); + } + + /** + * Force the `formidable` style handle to be re-registered so its version reflects the + * current `frm_last_style_update` option, then return that version. + * + * @return string + */ + private function get_registered_formidable_css_version() { + global $wp_styles, $frm_vars; + + $frm_vars['css_loaded'] = false; + + if ( isset( $wp_styles->registered['formidable'] ) ) { + wp_deregister_style( 'formidable' ); + } + + FrmStylesController::enqueue_css( 'register', true ); + + $this->assertArrayHasKey( 'formidable', $wp_styles->registered, 'The formidable stylesheet was not registered' ); + + return $wp_styles->registered['formidable']->ver; + } }