diff --git a/assets/js/single-attachment.js b/assets/js/single-attachment.js index f2fc68255..d30a9adc6 100644 --- a/assets/js/single-attachment.js +++ b/assets/js/single-attachment.js @@ -125,6 +125,7 @@ jQuery(document).ready(function($) { function uploadFile() { var formData = new FormData(); formData.append("action", "optml_replace_file"); + formData.append("optml_replace_nonce", OMAttachmentEdit.nonce); formData.append("attachment_id", OMAttachmentEdit.attachmentId); formData.append("file", $("#optml-replace-file-field")[0].files[0]); @@ -138,11 +139,12 @@ jQuery(document).ready(function($) { contentType: false, success: function(response) { $(".optml-svg-loader").hide(); - if(response.success) { + if (response && response.success) { window.location.reload(); } else { + var msg = (response && (response.data || response.message)) || OMAttachmentEdit.i18n.replaceFileError; $(".optml-replace-file-error").removeClass("hidden"); - $(".optml-replace-file-error").text(response.message); + $(".optml-replace-file-error").text(msg); } }, error: function(response) { diff --git a/inc/media_rename/attachment_edit.php b/inc/media_rename/attachment_edit.php index 0663df3bf..86bc4bff1 100644 --- a/inc/media_rename/attachment_edit.php +++ b/inc/media_rename/attachment_edit.php @@ -92,6 +92,7 @@ public function enqueue_scripts( $hook ) { 'maxFileSize' => $max_file_size, 'attachmentId' => $id, 'mimeType' => $mime_type, + 'nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), 'i18n' => [ 'maxFileSizeError' => $max_file_size_error, 'replaceFileError' => __( 'Error replacing file', 'optimole-wp' ), @@ -329,7 +330,19 @@ public function save_attachment_filename( $post_id ) { * Replace the file */ public function replace_file() { - $id = (int) sanitize_text_field( $_POST['attachment_id'] ); + if ( ! check_ajax_referer( 'optml_replace_media_nonce', 'optml_replace_nonce', false ) ) { + wp_send_json_error( __( 'Security check failed', 'optimole-wp' ) ); + } + + $id = absint( $_POST['attachment_id'] ?? 0 ); + + if ( ! $id ) { + wp_send_json_error( __( 'Invalid attachment ID', 'optimole-wp' ) ); + } + + if ( get_post_type( $id ) !== 'attachment' ) { + wp_send_json_error( __( 'Invalid attachment ID', 'optimole-wp' ) ); + } if ( ! current_user_can( 'edit_post', $id ) ) { wp_send_json_error( __( 'You are not allowed to replace this file', 'optimole-wp' ) ); @@ -339,6 +352,17 @@ public function replace_file() { wp_send_json_error( __( 'No file uploaded', 'optimole-wp' ) ); } + $file_info = wp_check_filetype_and_ext( $_FILES['file']['tmp_name'], $_FILES['file']['name'] ); + + if ( empty( $file_info['type'] ) ) { + wp_send_json_error( __( 'Could not determine uploaded file type', 'optimole-wp' ) ); + } + + $original_mime = get_post_mime_type( $id ); + if ( $file_info['type'] !== $original_mime ) { + wp_send_json_error( __( 'The uploaded file type does not match the original file type.', 'optimole-wp' ) ); + } + $replacer = new Optml_Attachment_Replace( $id, $_FILES['file'] ); $replaced = $replacer->replace(); diff --git a/tests/media_rename/test-attachment-edit.php b/tests/media_rename/test-attachment-edit.php index 87e43bcc7..46a6e0e5d 100644 --- a/tests/media_rename/test-attachment-edit.php +++ b/tests/media_rename/test-attachment-edit.php @@ -5,8 +5,14 @@ /** * Class Test_Attachment_Edit. + * + * Extends WP_Ajax_UnitTestCase so that: + * - DOING_AJAX is true, ensuring wp_send_json_* calls wp_die() rather than die() + * - wp_die() is overridden to throw WPAjaxDieContinueException / WPAjaxDieStopException + * after storing the response in $this->_last_response + * - _handleAjax() dispatches wp_ajax_* actions and captures JSON output cleanly */ -class Test_Attachment_Edit extends WP_UnitTestCase { +class Test_Attachment_Edit extends WP_Ajax_UnitTestCase { /** * Test instance * @@ -14,12 +20,50 @@ class Test_Attachment_Edit extends WP_UnitTestCase { */ private $instance; + /** + * A real JPEG attachment (file on disk) shared across replace_file tests. + * + * @var int + */ + private static $jpeg_attachment_id; + + /** + * Create a real JPEG attachment once for the whole class. + */ + public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { + self::$jpeg_attachment_id = $factory->attachment->create_upload_object( + OPTML_PATH . 'tests/assets/sample-test.jpg' + ); + } + + /** + * Clean up the shared attachment after all tests in this class run. + */ + public static function tear_down_after_class() { + wp_delete_post( self::$jpeg_attachment_id, true ); + parent::tear_down_after_class(); + } + /** * Setup test */ public function setUp(): void { parent::setUp(); + wp_set_current_user( 1 ); + $this->instance = new Optml_Attachment_Edit(); + + // Register the wp_ajax_optml_replace_file action so _handleAjax() can + // invoke replace_file() through the normal WordPress AJAX dispatch path. + $this->instance->init(); + } + + /** + * Reset $_FILES after each test (WP_Ajax_UnitTestCase resets $_POST/$_GET). + */ + public function tearDown(): void { + $_FILES = []; + parent::tearDown(); } /** @@ -49,4 +93,196 @@ public function test_prepare_attachment_filename() { $this->assertEquals( 'test-file', get_post_meta( $attachment->ID, '_optml_pending_rename', true ) ); } + + /** + * Dispatch the optml_replace_file AJAX action with the given POST data and + * $_FILES, catch the wp_die() exception, and return the decoded JSON. + * + * _handleAjax() starts its own output buffer and the die handler stores + * the captured JSON in $this->_last_response before throwing, so no manual + * ob_start() is needed here. + * + * @param array $post Contents for $_POST (nonce and attachment_id). + * @param array $files Contents for $_FILES['file'], or empty to omit. + * @return array Decoded JSON response array, or empty array. + */ + private function call_replace_file( array $post, array $files = [] ): array { + $_POST = $post; + $_FILES = $files; + + try { + $this->_handleAjax( 'optml_replace_file' ); + } catch ( WPAjaxDieContinueException $e ) { + // Normal path: wp_send_json_* echoed JSON then called wp_die(). + } catch ( WPAjaxDieStopException $e ) { + // Fallback: wp_die() called with a plain string (e.g. '-1' from a + // nonce failure when $die = true). Treat as a failed response. + } + + return ! empty( $this->_last_response ) + ? ( json_decode( $this->_last_response, true ) ?? [] ) + : []; + } + + /** + * A request with no nonce must be rejected with a security error. + */ + public function test_replace_file_no_nonce() { + $response = $this->call_replace_file( [ 'attachment_id' => (string) self::$jpeg_attachment_id ] ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'Security check', $response['data'] ); + } + + /** + * A request with a syntactically valid but wrong nonce must be rejected. + */ + public function test_replace_file_bad_nonce() { + $response = $this->call_replace_file( [ + 'attachment_id' => (string) self::$jpeg_attachment_id, + 'optml_replace_nonce' => 'not_a_real_nonce', + ] ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'Security check', $response['data'] ); + } + + /** + * attachment_id = 0 must be rejected before any file or capability check. + */ + public function test_replace_file_zero_id() { + $response = $this->call_replace_file( [ + 'attachment_id' => '0', + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ] ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'Invalid attachment ID', $response['data'] ); + } + + /** + * An ID that belongs to a non-attachment post type must be rejected. + */ + public function test_replace_file_non_attachment_post() { + $post_id = self::factory()->post->create( [ 'post_type' => 'post' ] ); + + $response = $this->call_replace_file( [ + 'attachment_id' => (string) $post_id, + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ] ); + + wp_delete_post( $post_id, true ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'Invalid attachment ID', $response['data'] ); + } + + /** + * A request with no $_FILES['file'] entry must be rejected. + */ + public function test_replace_file_no_file_uploaded() { + $response = $this->call_replace_file( [ + 'attachment_id' => (string) self::$jpeg_attachment_id, + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ] ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'No file uploaded', $response['data'] ); + } + + /** + * A binary blob named .jpg whose content is not a real image must be + * rejected: wp_check_filetype_and_ext() uses getimagesize() for image/* + * types and returns an empty type when the magic bytes do not match. + */ + public function test_replace_file_unrecognizable_mime() { + $tmp = tempnam( sys_get_temp_dir(), 'optml_test_' ); + file_put_contents( $tmp, 'this is not image data @@##$$%%' ); + + $response = $this->call_replace_file( + [ + 'attachment_id' => (string) self::$jpeg_attachment_id, + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ], + [ + 'file' => [ + 'name' => 'fake.jpg', + 'type' => 'image/jpeg', + 'tmp_name' => $tmp, + 'error' => 0, + 'size' => filesize( $tmp ), + ], + ] + ); + + @unlink( $tmp ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'determine', $response['data'] ); + } + + /** + * Uploading a file whose real MIME type differs from the original attachment + * (SVG against a JPEG attachment) must be rejected by the MIME-match check. + */ + public function test_replace_file_mime_mismatch() { + $response = $this->call_replace_file( + [ + 'attachment_id' => (string) self::$jpeg_attachment_id, + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ], + [ + 'file' => [ + 'name' => 'sample.svg', + 'type' => 'image/svg+xml', + 'tmp_name' => OPTML_PATH . 'tests/assets/sample.svg', + 'error' => 0, + 'size' => filesize( OPTML_PATH . 'tests/assets/sample.svg' ), + ], + ] + ); + + $this->assertFalse( $response['success'] ); + $this->assertStringContainsString( 'does not match', $response['data'] ); + } + + /** + * A valid nonce, valid attachment ID, and a replacement whose MIME type + * matches the original must pass all validation and succeed. + * + * A fresh attachment is created so the shared fixture is not consumed by + * the actual file-move that Optml_Attachment_Replace performs. + */ + public function test_replace_file_valid_jpeg_replacement() { + $attachment_id = self::factory()->attachment->create_upload_object( + OPTML_PATH . 'tests/assets/sample-test.jpg' + ); + + // Optml_Attachment_Replace moves (not copies) the tmp file. + $tmp = tempnam( sys_get_temp_dir(), 'optml_repl_' ) . '.jpg'; + copy( OPTML_PATH . 'tests/assets/small-1.jpg', $tmp ); + + $response = $this->call_replace_file( + [ + 'attachment_id' => (string) $attachment_id, + 'optml_replace_nonce' => wp_create_nonce( 'optml_replace_media_nonce' ), + ], + [ + 'file' => [ + 'name' => 'small-1.jpg', + 'type' => 'image/jpeg', + 'tmp_name' => $tmp, + 'error' => 0, + 'size' => filesize( $tmp ), + ], + ] + ); + + if ( file_exists( $tmp ) ) { + @unlink( $tmp ); + } + wp_delete_post( $attachment_id, true ); + + $this->assertTrue( $response['success'] ); + } }